From 4d7ae9eb58fa2feb6264d7526bbffae904a52b7f Mon Sep 17 00:00:00 2001 From: greatSumini Date: Tue, 31 Aug 2021 01:26:51 +0900 Subject: [PATCH 1/7] enhance: update useAnalyticsPageView to be called only once --- src/hooks/useAnalyticsPageView.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hooks/useAnalyticsPageView.ts b/src/hooks/useAnalyticsPageView.ts index 8f884b45..ae50d681 100644 --- a/src/hooks/useAnalyticsPageView.ts +++ b/src/hooks/useAnalyticsPageView.ts @@ -7,5 +7,6 @@ export const useAnalyticsPageView = (params: UnknownRecord) => { React.useEffect(() => { analytics.onPageView(params); - }, [analytics, params]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [analytics]); }; From 8808dae3aa9c4354f101b80638ba3cd15bd5adaa Mon Sep 17 00:00:00 2001 From: greatSumini Date: Tue, 31 Aug 2021 01:42:41 +0900 Subject: [PATCH 2/7] enhance: update useAnalyticsPageView to accept callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 이제 callback 함수를 입력 받을 수 있습니다. - regular function으로 변경하고 overloading을 추가했습니다. --- src/hooks/useAnalyticsPageView.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/hooks/useAnalyticsPageView.ts b/src/hooks/useAnalyticsPageView.ts index ae50d681..ca37e3b3 100644 --- a/src/hooks/useAnalyticsPageView.ts +++ b/src/hooks/useAnalyticsPageView.ts @@ -2,11 +2,19 @@ import React from 'react'; import {useAnalyticsContext} from '../contexts/useAnalyticsContext'; import {UnknownRecord} from '../types/common'; -export const useAnalyticsPageView = (params: UnknownRecord) => { +export function useAnalyticsPageView(params: UnknownRecord): void; +export function useAnalyticsPageView(callback: () => UnknownRecord): void; +export function useAnalyticsPageView(callback: () => Promise): void; +export function useAnalyticsPageView(paramsOrCallback: UnknownRecord | (() => Promise | UnknownRecord)) { const analytics = useAnalyticsContext(); - React.useEffect(() => { + const pageView = async () => { + const params = typeof paramsOrCallback === 'function' ? await paramsOrCallback() : paramsOrCallback; analytics.onPageView(params); + }; + + React.useEffect(() => { + pageView(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [analytics]); -}; +} From 74d43d074e9825c84d7533a468f18c286fc9301c Mon Sep 17 00:00:00 2001 From: greatSumini Date: Tue, 31 Aug 2021 02:35:20 +0900 Subject: [PATCH 3/7] test: add useAnalyticsPageView test --- tests/hooks/useAnalyticsPageView.test.ts | 66 ++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 tests/hooks/useAnalyticsPageView.test.ts diff --git a/tests/hooks/useAnalyticsPageView.test.ts b/tests/hooks/useAnalyticsPageView.test.ts new file mode 100644 index 00000000..52724cd3 --- /dev/null +++ b/tests/hooks/useAnalyticsPageView.test.ts @@ -0,0 +1,66 @@ +import React from 'react'; +import * as faker from 'faker'; + +import * as contextModule from '../../src/contexts/useAnalyticsContext'; +import {useAnalyticsPageView} from '../../src/hooks/useAnalyticsPageView'; + +describe('useAnalyticsPageView', () => { + const setUp = () => { + const params = {value: faker.lorem.word()}; + const callback = () => params; + const asyncCallback = async () => params; + + const onPageView = jest.fn(); + + const useEffectSpy = jest.spyOn(React, 'useEffect').mockImplementationOnce(cb => cb()); + const useContextSpy = jest.spyOn(contextModule, 'useAnalyticsContext').mockImplementationOnce( + () => + ({ + onPageView, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any), + ); + + return { + params, + callback, + asyncCallback, + onPageView, + useEffectSpy, + useContextSpy, + }; + }; + + test('should call analytics.onPageView with params', async () => { + const {params, onPageView, useContextSpy, useEffectSpy} = setUp(); + + useAnalyticsPageView(params); + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(useContextSpy).toHaveBeenCalled(); + expect(useEffectSpy).toHaveBeenCalled(); + expect(onPageView).toHaveBeenCalledWith(params); + }); + + test('should call analytics.onPageView with params', async () => { + const {params, callback, onPageView, useContextSpy, useEffectSpy} = setUp(); + + useAnalyticsPageView(callback); + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(useContextSpy).toHaveBeenCalled(); + expect(useEffectSpy).toHaveBeenCalled(); + expect(onPageView).toHaveBeenCalledWith(params); + }); + + test('should call analytics.onPageView with params', async () => { + const {params, asyncCallback, onPageView, useContextSpy, useEffectSpy} = setUp(); + + useAnalyticsPageView(asyncCallback); + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(useContextSpy).toHaveBeenCalled(); + expect(useEffectSpy).toHaveBeenCalled(); + expect(onPageView).toHaveBeenCalledWith(params); + }); +}); From a3c8a187f7dc489eaace4bc34edc0aa958710307 Mon Sep 17 00:00:00 2001 From: greatSumini Date: Tue, 31 Aug 2021 02:42:21 +0900 Subject: [PATCH 4/7] test: remove waiting time in useAnalyticsPageView tests 100ms -> 0mx --- tests/hooks/useAnalyticsPageView.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/hooks/useAnalyticsPageView.test.ts b/tests/hooks/useAnalyticsPageView.test.ts index 52724cd3..b4af8f80 100644 --- a/tests/hooks/useAnalyticsPageView.test.ts +++ b/tests/hooks/useAnalyticsPageView.test.ts @@ -35,7 +35,7 @@ describe('useAnalyticsPageView', () => { const {params, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(params); - await new Promise(resolve => setTimeout(resolve, 100)); + await new Promise(resolve => setTimeout(resolve, 0)); expect(useContextSpy).toHaveBeenCalled(); expect(useEffectSpy).toHaveBeenCalled(); @@ -46,7 +46,7 @@ describe('useAnalyticsPageView', () => { const {params, callback, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(callback); - await new Promise(resolve => setTimeout(resolve, 100)); + await new Promise(resolve => setTimeout(resolve, 0)); expect(useContextSpy).toHaveBeenCalled(); expect(useEffectSpy).toHaveBeenCalled(); @@ -57,7 +57,7 @@ describe('useAnalyticsPageView', () => { const {params, asyncCallback, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(asyncCallback); - await new Promise(resolve => setTimeout(resolve, 100)); + await new Promise(resolve => setTimeout(resolve, 0)); expect(useContextSpy).toHaveBeenCalled(); expect(useEffectSpy).toHaveBeenCalled(); From 88e229704219a66a12b5a355a2a63266ec1b356d Mon Sep 17 00:00:00 2001 From: greatSumini Date: Tue, 31 Aug 2021 13:06:45 +0900 Subject: [PATCH 5/7] chore: fix typo --- tests/hooks/useAnalyticsPageView.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/hooks/useAnalyticsPageView.test.ts b/tests/hooks/useAnalyticsPageView.test.ts index b4af8f80..958677e1 100644 --- a/tests/hooks/useAnalyticsPageView.test.ts +++ b/tests/hooks/useAnalyticsPageView.test.ts @@ -42,7 +42,7 @@ describe('useAnalyticsPageView', () => { expect(onPageView).toHaveBeenCalledWith(params); }); - test('should call analytics.onPageView with params', async () => { + test('should call analytics.onPageView with callback', async () => { const {params, callback, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(callback); @@ -53,7 +53,7 @@ describe('useAnalyticsPageView', () => { expect(onPageView).toHaveBeenCalledWith(params); }); - test('should call analytics.onPageView with params', async () => { + test('should call analytics.onPageView with asyncCallback', async () => { const {params, asyncCallback, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(asyncCallback); From 9948e5f844878700db67beab759c43ccd652ec10 Mon Sep 17 00:00:00 2001 From: greatSumini Date: Wed, 1 Sep 2021 22:18:59 +0900 Subject: [PATCH 6/7] test: add waitForAsync for remove duplicates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 비동기 대기 인라인 코드를 함수로 만들어 중복을 제거했습니다. --- tests/hooks/useAnalyticsPageView.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/hooks/useAnalyticsPageView.test.ts b/tests/hooks/useAnalyticsPageView.test.ts index 958677e1..dbb27b0e 100644 --- a/tests/hooks/useAnalyticsPageView.test.ts +++ b/tests/hooks/useAnalyticsPageView.test.ts @@ -31,11 +31,13 @@ describe('useAnalyticsPageView', () => { }; }; + const waitForAsync = () => new Promise(resolve => setTimeout(resolve, 0)); + test('should call analytics.onPageView with params', async () => { const {params, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(params); - await new Promise(resolve => setTimeout(resolve, 0)); + await waitForAsync(); expect(useContextSpy).toHaveBeenCalled(); expect(useEffectSpy).toHaveBeenCalled(); @@ -46,7 +48,7 @@ describe('useAnalyticsPageView', () => { const {params, callback, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(callback); - await new Promise(resolve => setTimeout(resolve, 0)); + await waitForAsync(); expect(useContextSpy).toHaveBeenCalled(); expect(useEffectSpy).toHaveBeenCalled(); @@ -57,7 +59,7 @@ describe('useAnalyticsPageView', () => { const {params, asyncCallback, onPageView, useContextSpy, useEffectSpy} = setUp(); useAnalyticsPageView(asyncCallback); - await new Promise(resolve => setTimeout(resolve, 0)); + await waitForAsync(); expect(useContextSpy).toHaveBeenCalled(); expect(useEffectSpy).toHaveBeenCalled(); From 66d5b8f2057450fedbb12334249aa387139cb4ac Mon Sep 17 00:00:00 2001 From: greatSumini Date: Wed, 1 Sep 2021 22:20:15 +0900 Subject: [PATCH 7/7] test: mute logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit console.info를 mocking하여 불필요한 log 발생을 없앴습니다. --- tests/hooks/useAnalyticsPageView.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/hooks/useAnalyticsPageView.test.ts b/tests/hooks/useAnalyticsPageView.test.ts index dbb27b0e..057a63c2 100644 --- a/tests/hooks/useAnalyticsPageView.test.ts +++ b/tests/hooks/useAnalyticsPageView.test.ts @@ -20,6 +20,7 @@ describe('useAnalyticsPageView', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any), ); + jest.spyOn(console, 'info').mockImplementation(() => null); return { params,