fix: DataLoaderProvider handle data return from useDataLoader (#150)

Author: DorianMaliszewski

packages/use-dataloader/README.md

@@ -135,3 +135,36 @@ function MyComponent() {
 
 export default MyComponent
 ```
+
+---
+
+## API
+
+### useDataLoader
+
+```js
+const useDataLoader = (
+  key, // A key to save the data fetched in a local cache
+  method, // A method that return a promise (ex: () => new Promise((resolve) => setTimeout(resolve, 2000))
+  {
+    onSuccess, // Callback when a request success
+    onError, // Callback when a error is occured
+    initialData, // Initial data if no one is present in the cache before the request
+    pollingInterval, // Relaunch the request after the last success
+    enabled = true, // Launch request automatically
+    keepPreviousData = true, // Do we need to keep the previous data after reload
+  } = {},
+)
+```
+
+|   Property   |                                                      Description                                                      |
+| :----------: | :-------------------------------------------------------------------------------------------------------------------: |
+|    isIdle    |                                         `true` if the request is not launched                                         |
+|  isLoading   |                                           `true` if the request is launched                                           |
+|  isSuccess   |                                      `true`if the request finished successfully                                       |
+|   isError    |                                         `true` if the request throw an error                                          |
+|  isPolling   | `true` if the request if `enabled` is true, `pollingInterval` is defined and the status is `isLoading` or `isSuccess` |
+| previousData |                             if `keepPreviousData` is true it return the last data fetched                             |
+|     data     |     return the `initialData` if no data is fetched or not present in the cache otherwise return the data fetched      |
+|    error     |                                      return the error occured during the request                                      |
+|    reload    |                            allow you to reload the data (it doesn't clear the actual data)                            |

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

@@ -11,7 +11,7 @@ const initialProps = {
     }),
   config: {
     enabled: true,
-    reloadOnKeyChange: false,
+    keepPreviousData: true,
   },
 }
 // eslint-disable-next-line react/prop-types
@@ -37,40 +37,69 @@ describe('useDataLoader', () => {
     expect(result.current.isLoading).toBe(false)
   })
 
-  test('should render correctly with enabled true', async () => {
-    const { result, waitForNextUpdate, rerender } = renderHook(
+  test('should render correctly without valid key', async () => {
+    const { result, waitForNextUpdate } = renderHook(
+      props => useDataLoader(props.key, props.method),
+      {
+        wrapper,
+        initialProps: {
+          ...initialProps,
+          key: 2,
+        },
+      },
+    )
+    expect(result.current.data).toBe(undefined)
+    expect(result.current.isLoading).toBe(true)
+    await waitForNextUpdate()
+    expect(result.current.data).toBe(undefined)
+    expect(result.current.isSuccess).toBe(true)
+    expect(result.current.isLoading).toBe(false)
+  })
+
+  test('should render correctly without keepPreviousData', async () => {
+    const { result, waitForNextUpdate } = renderHook(
       props => useDataLoader(props.key, props.method, props.config),
       {
         wrapper,
-        initialProps,
+        initialProps: {
+          ...initialProps,
+          config: {
+            keepPreviousData: false,
+          },
+        },
       },
     )
     expect(result.current.data).toBe(undefined)
     expect(result.current.isLoading).toBe(true)
-    rerender()
     await waitForNextUpdate()
     expect(result.current.data).toBe(true)
     expect(result.current.isSuccess).toBe(true)
     expect(result.current.isLoading).toBe(false)
+  })
 
-    act(() => {
-      result.current.reload()
-    })
-    act(() => {
-      result.current.reload()
-    })
-
-    expect(result.current.data).toBe(true)
+  test('should render correctly with result null', async () => {
+    const { result, waitForNextUpdate } = renderHook(
+      props => useDataLoader(props.key, props.method, props.config),
+      {
+        wrapper,
+        initialProps: {
+          ...initialProps,
+          method: () =>
+            new Promise(resolve => setTimeout(() => resolve(null), 100)),
+        },
+      },
+    )
+    expect(result.current.data).toBe(undefined)
     expect(result.current.isLoading).toBe(true)
     await waitForNextUpdate()
-    expect(result.current.data).toBe(true)
+    expect(result.current.data).toBe(undefined)
     expect(result.current.isSuccess).toBe(true)
     expect(result.current.isLoading).toBe(false)
   })
 
-  test('should render correctly with bad key', async () => {
+  test('should render correctly with enabled true', async () => {
     const { result, waitForNextUpdate } = renderHook(
-      props => useDataLoader(undefined, props.method, props.config),
+      props => useDataLoader(props.key, props.method, props.config),
       {
         wrapper,
         initialProps,
@@ -99,35 +128,33 @@ describe('useDataLoader', () => {
   })
 
   test('should render correctly with key update', async () => {
-    let key = 'test'
     const propsToPass = {
       ...initialProps,
-      key,
+      key: 'test',
       config: {
         reloadOnKeyChange: true,
       },
     }
     const { result, waitForNextUpdate, rerender } = renderHook(
-      props => useDataLoader(key, props.method, props.config),
+      () =>
+        useDataLoader(propsToPass.key, propsToPass.method, propsToPass.config),
       {
         wrapper,
-        initialProps: propsToPass,
       },
     )
 
     expect(result.current.data).toBe(undefined)
     expect(result.current.isLoading).toBe(true)
     await waitForNextUpdate()
-    expect(result.current.data).toBe(true)
     expect(result.current.isSuccess).toBe(true)
     expect(result.current.isLoading).toBe(false)
+    expect(result.current.data).toBe(true)
 
-    key = 'new-test'
+    propsToPass.key = 'key-2'
     rerender()
-
-    expect(result.current.data).toBe(undefined)
     expect(result.current.isLoading).toBe(true)
-    key = 'new-new-test'
+    expect(result.current.data).toBe(undefined)
+    propsToPass.key = 'new-new-test'
     rerender()
     expect(result.current.data).toBe(undefined)
     expect(result.current.isLoading).toBe(true)

packages/use-dataloader/src/reducer.js

@@ -14,18 +14,11 @@ export default (state, action) => {
       return {
         ...state,
         error: undefined,
-        data: action.data,
         status: StatusEnum.SUCCESS,
       }
-    case ActionEnum.ON_UPDATE_DATA:
-      return {
-        ...state,
-        data: action.data,
-      }
     case ActionEnum.RESET:
       return {
         status: StatusEnum.IDLE,
-        data: action.data,
         error: undefined,
       }
     case ActionEnum.ON_ERROR:

packages/use-dataloader/src/useDataLoader.js

@@ -4,13 +4,41 @@ import { ActionEnum, StatusEnum } from './constants'
 import reducer from './reducer'
 
 const Actions = {
-  createReset: ({ data }) => ({ type: ActionEnum.RESET, data }),
+  createReset: () => ({ type: ActionEnum.RESET }),
   createOnLoading: () => ({ type: ActionEnum.ON_LOADING }),
-  createOnSuccess: data => ({ type: ActionEnum.ON_SUCCESS, data }),
-  createOnUpdateData: data => ({ type: ActionEnum.ON_UPDATE_DATA, data }),
+  createOnSuccess: () => ({ type: ActionEnum.ON_SUCCESS }),
   createOnError: error => ({ type: ActionEnum.ON_ERROR, error }),
 }
 
+/**
+ * @typedef {Object} useDataLoaderConfig
+ * @property {Function} [onSuccess] callback when a request success
+ * @property {Function} [onError] callback when a error is occured
+ * @property {*} [initialData] initial data if no one is present in the cache before the request
+ * @property {number} [pollingInterval] relaunch the request after the last success
+ * @property {boolean} [enabled=true] launch request automatically (default true)
+ * @property {boolean} [keepPreviousData=true] do we need to keep the previous data after reload (default true)
+ */
+
+/**
+ * @typedef {Object} useDataLoaderResult
+ * @property {boolean} isIdle true if the hook in initial state
+ * @property {boolean} isLoading true if the request is launched
+ * @property {boolean} isSuccess true if the request success
+ * @property {boolean} isError true if the request throw an error
+ * @property {boolean} isPolling true if the request if enabled is true, pollingInterval is defined and the status is isLoading or isSuccess
+ * @property {*} previousData if keepPreviousData is true it return the last data fetched
+ * @property {*} data initialData if no data is fetched or not present in the cache otherwise return the data fetched
+ * @property {string} error the error occured during the request
+ * @property {Function} reload reload the data
+ */
+
+/**
+ * @param {string} key key to save the data fetched in a local cache
+ * @param {() => Promise} method a method that return a promise
+ * @param {useDataLoaderConfig} config hook configuration
+ * @returns {useDataLoaderResult} hook result containing data, request state, and method to reload the data
+ */
 const useDataLoader = (
   key,
   method,
@@ -20,7 +48,6 @@ const useDataLoader = (
     initialData,
     pollingInterval,
     enabled = true,
-    reloadOnKeyChange = true,
     keepPreviousData = true,
   } = {},
 ) => {
@@ -30,14 +57,13 @@ const useDataLoader = (
     clearReload,
     getCachedData,
   } = useDataLoaderContext()
-  const [{ status, data, error }, dispatch] = useReducer(reducer, {
+  const [{ status, error }, dispatch] = useReducer(reducer, {
     status: StatusEnum.IDLE,
-    data: initialData,
     error: undefined,
   })
 
   const previousDataRef = useRef()
-  const keyRef = useRef()
+  const keyRef = useRef(key)
   const methodRef = useRef(method)
   const onSuccessRef = useRef(onSuccess)
   const onErrorRef = useRef(onError)
@@ -46,26 +72,28 @@ const useDataLoader = (
   const isIdle = useMemo(() => status === StatusEnum.IDLE, [status])
   const isSuccess = useMemo(() => status === StatusEnum.SUCCESS, [status])
   const isError = useMemo(() => status === StatusEnum.ERROR, [status])
+
   const isPolling = useMemo(
     () => enabled && pollingInterval && (isSuccess || isLoading),
     [isSuccess, isLoading, enabled, pollingInterval],
   )
 
-  const handleRequest = useRef(async (cacheKey, args) => {
-    const cachedData = getCachedData(cacheKey)
-    if (cacheKey && !data && cachedData) {
-      dispatch(Actions.createOnUpdateData(cachedData))
-    }
+  const handleRequest = useRef(async cacheKey => {
     try {
       dispatch(Actions.createOnLoading())
-      const result = await methodRef.current?.(args)
+      const result = await methodRef.current?.()
 
-      if (result && cacheKey) addCachedData(cacheKey, result)
       if (keyRef.current && cacheKey && cacheKey !== keyRef.current) {
         return
       }
 
-      dispatch(Actions.createOnSuccess(result))
+      if (keepPreviousData) {
+        previousDataRef.current = getCachedData(cacheKey)
+      }
+      if (result !== undefined && result !== null && cacheKey)
+        addCachedData(cacheKey, result)
+
+      dispatch(Actions.createOnSuccess())
 
       await onSuccessRef.current?.(result)
     } catch (err) {
@@ -77,19 +105,15 @@ const useDataLoader = (
   useEffect(() => {
     let handler
     if (enabled) {
-      if (
-        reloadOnKeyChange &&
-        key !== keyRef.current &&
-        status !== StatusEnum.IDLE
-      ) {
+      if (!isIdle && keyRef.current !== key) {
         keyRef.current = key
-        dispatch(Actions.createReset({ data: initialData }))
+        dispatch(Actions.createReset())
       } else {
-        if (status === StatusEnum.IDLE) {
+        if (isIdle) {
           keyRef.current = key
           handleRequest.current(key)
         }
-        if (pollingInterval && status === StatusEnum.SUCCESS) {
+        if (pollingInterval && isSuccess && !handler) {
           handler = setTimeout(
             () => handleRequest.current(key),
             pollingInterval,
@@ -107,38 +131,34 @@ const useDataLoader = (
       }
       if (handler) {
         clearTimeout(handler)
+        handler = undefined
       }
     }
     // Why can't put empty array for componentDidMount, componentWillUnmount ??? No array act like componentDidMount and componentDidUpdate
   }, [
     enabled,
-    pollingInterval,
     key,
     clearReload,
     addReload,
-    status,
-    reloadOnKeyChange,
-    initialData,
+    addCachedData,
+    getCachedData,
+    pollingInterval,
+    isIdle,
+    isSuccess,
   ])
 
   useLayoutEffect(() => {
     methodRef.current = method
   }, [method])
 
-  useLayoutEffect(() => {
-    if (keepPreviousData && data && previousDataRef.current !== data) {
-      previousDataRef.current = data
-    }
-  }, [keepPreviousData, data])
-
   return {
     isLoading,
     isIdle,
     isSuccess,
     isError,
     isPolling,
     previousData: previousDataRef.current,
-    data,
+    data: getCachedData(key) || initialData,
     error,
     reload: args => handleRequest.current(key, args),
   }