From 60cce424a084d3f79a4cb09c9ade746c1416edca Mon Sep 17 00:00:00 2001 From: Emmanuel Chambon Date: Thu, 27 May 2021 21:15:26 +0200 Subject: [PATCH 1/2] fix(use-dataloader): move cacheKeyPrefix logic into hook to trigger reload --- .../use-dataloader/src/DataLoaderProvider.js | 37 ++++++++----------- packages/use-dataloader/src/useDataLoader.js | 17 +++++++-- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/packages/use-dataloader/src/DataLoaderProvider.js b/packages/use-dataloader/src/DataLoaderProvider.js index 50f5e2873..c32d811c4 100644 --- a/packages/use-dataloader/src/DataLoaderProvider.js +++ b/packages/use-dataloader/src/DataLoaderProvider.js @@ -34,11 +34,11 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix }) => { if (key && typeof key === 'string' && newData) { setCachedData(actualCachedData => ({ ...actualCachedData, - [`${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${key}`]: newData, + [key]: newData, })) } }, - [setCachedData, cacheKeyPrefix], + [setCachedData], ) const addReload = useCallback( @@ -76,13 +76,13 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix }) => { if (key && typeof key === 'string') { setCachedData(actualCachedData => { const tmp = actualCachedData - delete tmp[`${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${key}`] + delete tmp[key] return tmp }) } }, - [setCachedData, cacheKeyPrefix], + [setCachedData], ) const clearAllCachedData = useCallback(() => { setCachedData({}) @@ -100,20 +100,13 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix }) => { ) }, []) - const getCachedData = useCallback( - key => { - if (key) { - return ( - cachedData.current[ - `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${key}` - ] || undefined - ) - } + const getCachedData = useCallback(key => { + if (key) { + return cachedData.current[key] || undefined + } - return cachedData.current - }, - [cacheKeyPrefix], - ) + return cachedData.current + }, []) const getReloads = useCallback(key => { if (key) { @@ -127,6 +120,7 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix }) => { () => ({ addCachedData, addReload, + cacheKeyPrefix, clearAllCachedData, clearAllReloads, clearCachedData, @@ -138,14 +132,15 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix }) => { }), [ addCachedData, - clearReload, + addReload, + cacheKeyPrefix, + clearAllCachedData, + clearAllReloads, clearCachedData, + clearReload, getCachedData, getReloads, - addReload, reload, - clearAllCachedData, - clearAllReloads, reloadAll, ], ) diff --git a/packages/use-dataloader/src/useDataLoader.js b/packages/use-dataloader/src/useDataLoader.js index b8ec8a28e..91666c0ff 100644 --- a/packages/use-dataloader/src/useDataLoader.js +++ b/packages/use-dataloader/src/useDataLoader.js @@ -40,7 +40,7 @@ const Actions = { * @returns {useDataLoaderResult} hook result containing data, request state, and method to reload the data */ const useDataLoader = ( - key, + fetchKey, method, { enabled = true, @@ -51,8 +51,13 @@ const useDataLoader = ( pollingInterval, } = {}, ) => { - const { addReload, clearReload, getCachedData, addCachedData } = - useDataLoaderContext() + const { + addReload, + clearReload, + getCachedData, + addCachedData, + cacheKeyPrefix, + } = useDataLoaderContext() const [{ status, error }, dispatch] = useReducer(reducer, { error: undefined, status: StatusEnum.IDLE, @@ -60,6 +65,12 @@ const useDataLoader = ( const addReloadRef = useRef(addReload) const clearReloadRef = useRef(clearReload) + + const key = useMemo( + () => `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${fetchKey}`, + [cacheKeyPrefix, fetchKey], + ) + const previousDataRef = useRef() const isMountedRef = useRef(enabled) const methodRef = useRef(method) From fc733a3f2a7efd13558c2bf01c3a32aa135da645 Mon Sep 17 00:00:00 2001 From: Emmanuel Chambon Date: Thu, 27 May 2021 21:39:36 +0200 Subject: [PATCH 2/2] test: correct tests --- .../src/__tests__/DataLoaderProvider.test.js | 47 ------------------- .../src/__tests__/useDataLoader.test.js | 38 +++++++++++++++ packages/use-dataloader/src/useDataLoader.js | 11 +++-- 3 files changed, 45 insertions(+), 51 deletions(-) diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.js b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.js index b04cb05ed..6e5324333 100644 --- a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.js +++ b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.js @@ -7,10 +7,6 @@ const wrapper = ({ children }) => ( {children} ) -const wrapperWithCacheKey = ({ children }) => ( - {children} -) - describe('DataLoaderProvider', () => { test('should render correctly', async () => { render(Test) @@ -29,21 +25,6 @@ describe('DataLoaderProvider', () => { expect(result.current.getCachedData().test).toBe('test') }) - test('should add cached data with cacheKeyPrefix', async () => { - const { result } = renderHook(useDataLoaderContext, { - wrapper: wrapperWithCacheKey, - }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addCachedData('test', 'test') - result.current.addCachedData(3, 'testWrong') - }) - - expect(Object.values(result.current.getCachedData()).length).toBe(1) - expect(result.current.getCachedData()['sample-test']).toBe('test') - }) - test('should delete cached data', async () => { const { result } = renderHook(useDataLoaderContext, { wrapper }) @@ -70,34 +51,6 @@ describe('DataLoaderProvider', () => { expect(result.current.getCachedData()).toStrictEqual({}) }) - test('should delete cached data with cacheKeyPrefix', async () => { - const { result } = renderHook(useDataLoaderContext, { - wrapper: wrapperWithCacheKey, - }) - - act(() => { - result.current.addCachedData('testA', 'testA') - result.current.addCachedData('testB', 'testB') - result.current.addCachedData('testC', 'testC') - result.current.addCachedData('testD', 'testD') - result.current.addCachedData('testE', 'testE') - }) - expect(result.current.getCachedData('testA')).toBe('testA') - - act(() => { - result.current.clearCachedData() - result.current.clearCachedData('testA') - }) - expect(Object.values(result.current.getCachedData()).length).toBe(4) - expect(result.current.getCachedData().testA).toBe(undefined) - - act(() => { - result.current.clearAllCachedData() - }) - expect(Object.values(result.current.getCachedData()).length).toBe(0) - expect(result.current.getCachedData()).toStrictEqual({}) - }) - test('should get cached data', async () => { const { result } = renderHook(useDataLoaderContext, { wrapper }) expect(result.current.getCachedData()).toStrictEqual({}) diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.js b/packages/use-dataloader/src/__tests__/useDataLoader.test.js index bd40aeada..56093bd82 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.js +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.js @@ -19,6 +19,10 @@ const wrapper = ({ children }) => ( {children} ) +const wrapperWithCacheKey = ({ children }) => ( + {children} +) + describe('useDataLoader', () => { test('should render correctly without options', async () => { const { result, waitForNextUpdate, rerender } = renderHook( @@ -97,6 +101,40 @@ describe('useDataLoader', () => { expect(result.current.isLoading).toBe(false) }) + test('should render and cache correctly with cacheKeyPrefix', async () => { + const { result, waitForNextUpdate } = renderHook( + props => [ + useDataLoader(props.key, props.method, props.config), + useDataLoader(props.key, props.method, { + ...props.config, + enabled: false, + }), + ], + { + initialProps, + wrapper: wrapperWithCacheKey, + }, + ) + + expect(result.current[0].data).toBe(undefined) + expect(result.current[0].isLoading).toBe(true) + expect(result.current[1].data).toBe(undefined) + expect(result.current[1].isIdle).toBe(true) + await waitForNextUpdate() + expect(result.current[0].data).toBe(true) + expect(result.current[0].isSuccess).toBe(true) + + act(() => { + result.current[1].reload() + }) + + expect(result.current[1].data).toBe(true) + expect(result.current[1].isLoading).toBe(true) + + await waitForNextUpdate() + expect(result.current[1].isSuccess).toBe(true) + }) + test('should render correctly with enabled true', async () => { const { result, waitForNextUpdate } = renderHook( props => useDataLoader(props.key, props.method, props.config), diff --git a/packages/use-dataloader/src/useDataLoader.js b/packages/use-dataloader/src/useDataLoader.js index 91666c0ff..3da360b7c 100644 --- a/packages/use-dataloader/src/useDataLoader.js +++ b/packages/use-dataloader/src/useDataLoader.js @@ -66,10 +66,13 @@ const useDataLoader = ( const addReloadRef = useRef(addReload) const clearReloadRef = useRef(clearReload) - const key = useMemo( - () => `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${fetchKey}`, - [cacheKeyPrefix, fetchKey], - ) + const key = useMemo(() => { + if (!fetchKey || typeof fetchKey !== 'string') { + return fetchKey + } + + return `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${fetchKey}` + }, [cacheKeyPrefix, fetchKey]) const previousDataRef = useRef() const isMountedRef = useRef(enabled)