diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.tsx index e0aa069ae..4753b1be3 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.tsx @@ -1,7 +1,19 @@ import { act, renderHook } from '@testing-library/react-hooks' import React from 'react' import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' -import useDataLoader from '../useDataLoader' +import useDataLoader, { + PromiseType, + UseDataLoaderConfig, + UseDataLoaderResult, +} from '../useDataLoader' + +type UseDataLoaderHookProps = { + config: UseDataLoaderConfig + key: string + method: () => Promise +} + +const PROMISE_TIMEOUT = 100 const initialProps = { config: { @@ -11,31 +23,33 @@ const initialProps = { key: 'test', method: () => new Promise(resolve => { - setTimeout(() => resolve(true), 500) + setTimeout(() => resolve(true), PROMISE_TIMEOUT) }), } // eslint-disable-next-line react/prop-types -const wrapper = ({ children }: { children: React.ReactNode }) => ( +const wrapper = ({ children }: { children?: React.ReactNode }) => ( {children} ) -const wrapperWithCacheKey = ({ children }: { children: React.ReactNode }) => ( +const wrapperWithCacheKey = ({ children }: { children?: React.ReactNode }) => ( {children} ) -const wrapperWithOnError = (onError: (err: Error) => void) => ({ children }: { children: React.ReactNode }) => ( - {children} -) +const wrapperWithOnError = + (onError: (err: Error) => void) => + // eslint-disable-next-line react/require-default-props + ({ children }: { children?: React.ReactNode }) => + {children} describe('useDataLoader', () => { test('should render correctly without options', async () => { - const { result, waitForNextUpdate, rerender } = renderHook( - props => useDataLoader(props.key, props.method), - { - initialProps, - wrapper, - }, - ) + const { result, waitForNextUpdate, rerender } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method), { + initialProps, + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) rerender() @@ -46,16 +60,18 @@ describe('useDataLoader', () => { }) test('should render correctly without valid key', async () => { - const { result, waitForNextUpdate } = renderHook( - props => useDataLoader(props.key, props.method), - { - initialProps: { - ...initialProps, - key: 2, - }, - wrapper, + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method), { + initialProps: { + ...initialProps, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error + key: 2, }, - ) + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -65,18 +81,18 @@ describe('useDataLoader', () => { }) test('should render correctly without keepPreviousData', async () => { - const { result, waitForNextUpdate } = renderHook( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - ...initialProps, - config: { - keepPreviousData: false, - }, + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + ...initialProps, + config: { + keepPreviousData: false, }, - wrapper, }, - ) + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -86,17 +102,19 @@ describe('useDataLoader', () => { }) test('should render correctly with result null', async () => { - const { result, waitForNextUpdate } = renderHook( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - ...initialProps, - method: () => - new Promise(resolve => setTimeout(() => resolve(null), 100)), - }, - wrapper, + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + ...initialProps, + method: () => + new Promise(resolve => + setTimeout(() => resolve(null), PROMISE_TIMEOUT), + ), }, - ) + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -106,7 +124,10 @@ describe('useDataLoader', () => { }) test('should render and cache correctly with cacheKeyPrefix', async () => { - const { result, waitForNextUpdate } = renderHook)[]>( + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + ReturnType[] + >( props => [ useDataLoader(props.key, props.method, props.config), useDataLoader(props.key, props.method, { @@ -141,13 +162,13 @@ describe('useDataLoader', () => { }) test('should render correctly with enabled true', async () => { - const { result, waitForNextUpdate } = renderHook>( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps, - wrapper, - }, - ) + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + ReturnType + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps, + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -175,12 +196,12 @@ describe('useDataLoader', () => { test('should render correctly with key update', async () => { const propsToPass = { ...initialProps, - config: { - reloadOnKeyChange: true, - }, key: 'test', } - const { result, waitForNextUpdate, rerender } = renderHook( + const { result, waitForNextUpdate, rerender } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >( () => useDataLoader(propsToPass.key, propsToPass.method, propsToPass.config), { @@ -208,13 +229,13 @@ describe('useDataLoader', () => { test('should render correctly with pooling', async () => { const pollingProps = { config: { - pollingInterval: 500, + pollingInterval: PROMISE_TIMEOUT, }, key: 'test', method: jest.fn( () => new Promise(resolve => { - setTimeout(() => resolve(true), 250) + setTimeout(() => resolve(true), PROMISE_TIMEOUT) }), ), } @@ -222,17 +243,17 @@ describe('useDataLoader', () => { const method2 = jest.fn( () => new Promise(resolve => { - setTimeout(() => resolve(2), 250) + setTimeout(() => resolve(2), PROMISE_TIMEOUT) }), ) - const { result, waitForNextUpdate, rerender } = renderHook>( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: pollingProps, - wrapper, - }, - ) + const { result, waitForNextUpdate, rerender } = renderHook< + UseDataLoaderHookProps, + ReturnType + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: pollingProps, + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isPolling).toBe(true) expect(result.current.isLoading).toBe(true) @@ -274,7 +295,7 @@ describe('useDataLoader', () => { rerender({ ...pollingProps, config: { - pollingInterval: 500, + pollingInterval: PROMISE_TIMEOUT, }, method: method2, }) @@ -292,18 +313,18 @@ describe('useDataLoader', () => { }) test('should render correctly with enabled off', async () => { - const { result, waitForNextUpdate } = renderHook>( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - ...initialProps, - config: { - enabled: false, - }, + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + ReturnType + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + ...initialProps, + config: { + enabled: false, }, - wrapper, }, - ) + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isIdle).toBe(true) @@ -323,18 +344,18 @@ describe('useDataLoader', () => { test('should call onSuccess', async () => { const onSuccess = jest.fn() - const { result, waitForNextUpdate } = renderHook( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - ...initialProps, - config: { - onSuccess, - }, + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + ...initialProps, + config: { + onSuccess, }, - wrapper, }, - ) + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -347,25 +368,25 @@ describe('useDataLoader', () => { const onSuccess = jest.fn() const onError = jest.fn() const error = new Error('Test error') - const { result, waitForNextUpdate } = renderHook( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - config: { - onError, - onSuccess, - }, - key: 'test', - method: () => - new Promise((resolve, reject) => { - setTimeout(() => { - reject(error) - }, 500) - }), + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + config: { + onError, + onSuccess, }, - wrapper, + key: 'test', + method: () => + new Promise((resolve, reject) => { + setTimeout(() => { + reject(error) + }, PROMISE_TIMEOUT) + }), }, - ) + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -383,25 +404,25 @@ describe('useDataLoader', () => { const onError = jest.fn() const error = new Error('Test error') const onErrorProvider = jest.fn() - const { result, waitForNextUpdate } = renderHook( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - config: { - onError, - onSuccess, - }, - key: 'test', - method: () => - new Promise((resolve, reject) => { - setTimeout(() => { - reject(error) - }, 500) - }), + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + config: { + onError, + onSuccess, }, - wrapper: wrapperWithOnError(onErrorProvider), + key: 'test', + method: () => + new Promise((_, reject) => { + setTimeout(() => { + reject(error) + }, PROMISE_TIMEOUT) + }), }, - ) + wrapper: wrapperWithOnError(onErrorProvider), + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -419,24 +440,24 @@ describe('useDataLoader', () => { const onSuccess = jest.fn() const error = new Error('Test error') const onErrorProvider = jest.fn() - const { result, waitForNextUpdate } = renderHook( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - config: { - onSuccess, - }, - key: 'test', - method: () => - new Promise((resolve, reject) => { - setTimeout(() => { - reject(error) - }, 500) - }), + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + config: { + onSuccess, }, - wrapper: wrapperWithOnError(onErrorProvider), + key: 'test', + method: () => + new Promise((_, reject) => { + setTimeout(() => { + reject(error) + }, PROMISE_TIMEOUT) + }), }, - ) + wrapper: wrapperWithOnError(onErrorProvider), + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) @@ -457,29 +478,29 @@ describe('useDataLoader', () => { success = true }) const error = new Error('Test error') - const { result, waitForNextUpdate } = renderHook>( - props => useDataLoader(props.key, props.method, props.config), - { - initialProps: { - config: { - onError, - onSuccess, - }, - key: 'test', - method: () => - new Promise((resolve, reject) => { - setTimeout(() => { - if (success) { - resolve(true) - } else { - reject(error) - } - }, 500) - }), + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >(props => useDataLoader(props.key, props.method, props.config), { + initialProps: { + config: { + onError, + onSuccess, }, - wrapper, + key: 'test', + method: () => + new Promise((resolve, reject) => { + setTimeout(() => { + if (success) { + resolve(true) + } else { + reject(error) + } + }, PROMISE_TIMEOUT) + }), }, - ) + wrapper, + }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -504,7 +525,10 @@ describe('useDataLoader', () => { }) test('should use cached data', async () => { - const { result, waitForNextUpdate } = renderHook[])>( + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult[] + >( props => [ useDataLoader(props.key, props.method, props.config), useDataLoader(props.key, props.method, { @@ -544,10 +568,16 @@ describe('useDataLoader', () => { new Promise(resolve => { setTimeout(() => { resolve(true) - }, 500) + }, PROMISE_TIMEOUT) }), ) - const { result, waitForNextUpdate } = renderHook, ReturnType])>( + const { result, waitForNextUpdate } = renderHook< + UseDataLoaderHookProps, + [ + ReturnType, + ReturnType, + ] + >( props => [ useDataLoader(props.key, props.method, props.config), useDataLoaderContext(), @@ -582,4 +612,73 @@ describe('useDataLoader', () => { expect(result.current[0].isSuccess).toBe(true) expect(mockedFn).toBeCalledTimes(2) }) + + test('should cancel request', async () => { + const onCancel = jest.fn() + const { result, unmount } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >( + props => + useDataLoader( + props.key, + () => { + const promise = new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }) as PromiseType + promise.cancel = onCancel + + return promise + }, + props.config, + ), + { + initialProps, + wrapper, + }, + ) + expect(result.current.data).toBe(undefined) + expect(result.current.isLoading).toBe(true) + unmount() + await act(result.current.reload) + expect(onCancel).toHaveBeenCalledTimes(1) + expect(result.current.data).toBe(undefined) + expect(result.current.isSuccess).toBe(false) + expect(result.current.isError).toBe(false) + }) + + test('should cancel request with Error', async () => { + const onCancel = jest.fn() + const { result, unmount } = renderHook< + UseDataLoaderHookProps, + UseDataLoaderResult + >( + props => + useDataLoader( + props.key, + () => { + const promise = new Promise((_, reject) => { + setTimeout(() => reject(new Error('Test')), PROMISE_TIMEOUT) + }) as PromiseType + promise.cancel = onCancel + + return promise + }, + props.config, + ), + { + initialProps, + wrapper, + }, + ) + expect(result.current.data).toBe(undefined) + expect(result.current.isLoading).toBe(true) + unmount() + await act(result.current.reload) + expect(onCancel).toHaveBeenCalledTimes(1) + expect(result.current.data).toBe(undefined) + expect(result.current.error).toBe(undefined) + expect(result.current.isSuccess).toBe(false) + expect(result.current.isError).toBe(false) + }) }) diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index b66eb880d..3a2a4bc18 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -17,6 +17,10 @@ const Actions = { createReset: () => ({ type: ActionEnum.RESET }), } +export class PromiseType extends Promise { + public cancel?: () => void +} + /** * @typedef {Object} UseDataLoaderConfig * @property {Function} [onSuccess] callback when a request success @@ -26,13 +30,13 @@ const Actions = { * @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) */ -interface UseDataLoaderConfig { - enabled?: boolean, - initialData?: T, - keepPreviousData?: boolean, - onError?: (err: Error) => void| Promise, - onSuccess?: (data: T) => void | Promise, - pollingInterval?: number, +export interface UseDataLoaderConfig { + enabled?: boolean + initialData?: T + keepPreviousData?: boolean + onError?: (err: Error) => void | Promise + onSuccess?: (data: T) => void | Promise + pollingInterval?: number } /** @@ -47,16 +51,16 @@ interface UseDataLoaderConfig { * @property {string} error the error occured during the request * @property {Function} reload reload the data */ -interface UseDataLoaderResult { - data?: T; - error?: Error; - isError: boolean; - isIdle: boolean; - isLoading: boolean; - isPolling: boolean; - isSuccess: boolean; - previousData?: T; - reload: () => Promise; +export interface UseDataLoaderResult { + data?: T + error?: Error + isError: boolean + isIdle: boolean + isLoading: boolean + isPolling: boolean + isSuccess: boolean + previousData?: T + reload: () => Promise } /** @@ -67,7 +71,7 @@ interface UseDataLoaderResult { */ const useDataLoader = ( fetchKey: string, - method: () => Promise, + method: () => PromiseType, { enabled = true, initialData, @@ -92,6 +96,9 @@ const useDataLoader = ( const addReloadRef = useRef(addReload) const clearReloadRef = useRef(clearReload) + const cancelMethodRef = useRef<(() => void) | undefined>(undefined) + const isMountedRef = useRef(false) + const isFetchingRef = useRef(false) const key = useMemo(() => { if (!fetchKey || typeof fetchKey !== 'string') { @@ -107,9 +114,8 @@ 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)) ?? false, + () => !!(enabled && pollingInterval && (isSuccess || isLoading)), [isSuccess, isLoading, enabled, pollingInterval], ) @@ -117,20 +123,26 @@ const useDataLoader = ( async cacheKey => { try { dispatch(Actions.createOnLoading()) - const result = await method() + const promise = method() + cancelMethodRef.current = promise.cancel + const result = await promise.then(res => res) - if (keepPreviousData) { - previousDataRef.current = getCachedData(cacheKey) as T - } - if (result !== undefined && result !== null && cacheKey) - addCachedData(cacheKey, result) + if (isMountedRef.current) { + if (keepPreviousData) { + previousDataRef.current = getCachedData(cacheKey) as T + } + if (result !== undefined && result !== null && cacheKey) + addCachedData(cacheKey, result) - dispatch(Actions.createOnSuccess()) + dispatch(Actions.createOnSuccess()) - await onSuccess?.(result) + await onSuccess?.(result) + } } catch (err) { - dispatch(Actions.createOnError(err as Error)) - await ((onError ?? onErrorProvider)?.(err as Error)) + if (isMountedRef.current) { + dispatch(Actions.createOnError(err as Error)) + await (onError ?? onErrorProvider)?.(err as Error) + } } }, [ @@ -175,9 +187,7 @@ const useDataLoader = ( useLayoutEffect(() => { dispatch(Actions.createReset()) if (key && typeof key === 'string') { - addReloadRef.current?.(key, () => - handleRequestRef.current(key), - ) + addReloadRef.current?.(key, () => handleRequestRef.current(key)) } return () => { @@ -196,6 +206,21 @@ const useDataLoader = ( handleRequestRef.current = handleRequest }, [handleRequest]) + useEffect(() => { + isFetchingRef.current = isLoading || isPolling + }, [isLoading, isPolling]) + + useEffect(() => { + isMountedRef.current = true + + return () => { + isMountedRef.current = false + if (isFetchingRef.current && cancelMethodRef.current) { + cancelMethodRef.current?.() + } + } + }, []) + return { data: (getCachedData(key) || initialData) as T, error,