fix(use-dataloader): move cacheKeyPrefix logic into hook to trigger reload (#203)

* fix(use-dataloader): move cacheKeyPrefix logic into hook to trigger reload

* test: correct tests

Author: chambo-e

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,
     ],
   )

packages/use-dataloader/src/__tests__/DataLoaderProvider.test.js

@@ -7,10 +7,6 @@ const wrapper = ({ children }) => (
   <DataLoaderProvider>{children}</DataLoaderProvider>
 )
 
-const wrapperWithCacheKey = ({ children }) => (
-  <DataLoaderProvider cacheKeyPrefix="sample">{children}</DataLoaderProvider>
-)
-
 describe('DataLoaderProvider', () => {
   test('should render correctly', async () => {
     render(<DataLoaderProvider>Test</DataLoaderProvider>)
@@ -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({})

packages/use-dataloader/src/__tests__/useDataLoader.test.js

@@ -19,6 +19,10 @@ const wrapper = ({ children }) => (
   <DataLoaderProvider>{children}</DataLoaderProvider>
 )
 
+const wrapperWithCacheKey = ({ children }) => (
+  <DataLoaderProvider cacheKeyPrefix="sample">{children}</DataLoaderProvider>
+)
+
 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),

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,15 +51,29 @@ 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,
   })
 
   const addReloadRef = useRef(addReload)
   const clearReloadRef = useRef(clearReload)
+
+  const key = useMemo(() => {
+    if (!fetchKey || typeof fetchKey !== 'string') {
+      return fetchKey
+    }
+
+    return `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${fetchKey}`
+  }, [cacheKeyPrefix, fetchKey])
+
   const previousDataRef = useRef()
   const isMountedRef = useRef(enabled)
   const methodRef = useRef(method)