From ecd4623f1f38d125dee4cf1db686aed118179c1f Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Mon, 3 May 2021 17:55:18 +0200 Subject: [PATCH] fix: it should be good with key changing --- packages/use-dataloader/README.md | 33 +++++++ .../src/__tests__/useDataLoader.test.js | 77 ++++++++++------ packages/use-dataloader/src/reducer.js | 7 -- packages/use-dataloader/src/useDataLoader.js | 88 ++++++++++++------- 4 files changed, 139 insertions(+), 66 deletions(-) diff --git a/packages/use-dataloader/README.md b/packages/use-dataloader/README.md index 3bc3d7976..fca49a960 100644 --- a/packages/use-dataloader/README.md +++ b/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) | diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.js b/packages/use-dataloader/src/__tests__/useDataLoader.test.js index 39ef7d3c1..5e17ef240 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.js +++ b/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) diff --git a/packages/use-dataloader/src/reducer.js b/packages/use-dataloader/src/reducer.js index 3fa8bde93..00b95ffaf 100644 --- a/packages/use-dataloader/src/reducer.js +++ b/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: diff --git a/packages/use-dataloader/src/useDataLoader.js b/packages/use-dataloader/src/useDataLoader.js index 2cc0e0079..767670ab0 100644 --- a/packages/use-dataloader/src/useDataLoader.js +++ b/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,30 +131,26 @@ 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, @@ -138,7 +158,7 @@ const useDataLoader = ( isError, isPolling, previousData: previousDataRef.current, - data, + data: getCachedData(key) || initialData, error, reload: args => handleRequest.current(key, args), }