diff --git a/packages/use-dataloader/README.md b/packages/use-dataloader/README.md index 10b50758c..150fdb07c 100644 --- a/packages/use-dataloader/README.md +++ b/packages/use-dataloader/README.md @@ -63,6 +63,12 @@ ReactDOM.render( ) ``` +#### `maxConcurrentRequests` + +You can specify a `maxConcurrentRequests` which will prevent DataLoader to launch request simultaneously and wait some to finish before start next ones. + +This can be useful if you want to limit the number of concurrent requests. + #### `onError(err: Error): void | Promise` This is a global `onError` handler. It will be overriden if you specify one in `useDataLoader` diff --git a/packages/use-dataloader/src/DataLoaderProvider.tsx b/packages/use-dataloader/src/DataLoaderProvider.tsx index 526838c09..39c25985b 100644 --- a/packages/use-dataloader/src/DataLoaderProvider.tsx +++ b/packages/use-dataloader/src/DataLoaderProvider.tsx @@ -5,21 +5,23 @@ import React, { createContext, useCallback, useContext, + useEffect, useMemo, useRef, - useState, } from 'react' -import { KEY_IS_NOT_STRING_ERROR, StatusEnum } from './constants' +import { + DEFAULT_MAX_CONCURRENT_REQUESTS, + KEY_IS_NOT_STRING_ERROR, + StatusEnum, +} from './constants' import DataLoader from './dataloader' import { OnErrorFn, PromiseType } from './types' -type RequestQueue = Record type CachedData = Record type Reloads = Record Promise> type UseDataLoaderInitializerArgs = { enabled?: boolean - key: string status?: StatusEnum method: () => PromiseType pollingInterval?: number @@ -30,210 +32,179 @@ type UseDataLoaderInitializerArgs = { maxDataLifetime?: number } -interface Context { - addCachedData: (key: string, newData: unknown) => void - addReload: (key: string, method: () => Promise) => void +type GetCachedDataFn = { + (): CachedData + (key?: string): unknown | undefined +} + +type GetReloadsFn = { + (): Reloads + (key?: string): (() => Promise) | undefined +} + +export interface IDataLoaderContext { addRequest: (key: string, args: UseDataLoaderInitializerArgs) => DataLoader - cacheKeyPrefix: string + getOrAddRequest: ( + key: string, + args: UseDataLoaderInitializerArgs, + ) => DataLoader + cacheKeyPrefix?: string onError?: (error: Error) => void | Promise clearAllCachedData: () => void - clearAllReloads: () => void - clearCachedData: (key?: string) => void - clearReload: (key?: string) => void - getCachedData: (key?: string) => unknown | CachedData - getReloads: (key?: string) => (() => Promise) | Reloads - getRequest: (key: string) => DataLoader | undefined + clearCachedData: (key: string) => void + getCachedData: GetCachedDataFn + getReloads: GetReloadsFn + getRequest: (key: string) => DataLoader reload: (key?: string) => Promise reloadAll: () => Promise } // @ts-expect-error we force the context to undefined, should be corrected with default values -export const DataLoaderContext = createContext(undefined) +export const DataLoaderContext = createContext(undefined) const DataLoaderProvider = ({ children, cacheKeyPrefix, onError, + maxConcurrentRequests, }: { children: ReactNode cacheKeyPrefix: string onError: OnErrorFn + maxConcurrentRequests?: number }): ReactElement => { - const [requestQueue, setRequestQueue] = useState({} as RequestQueue) - const [cachedData, setCachedDataPrivate] = useState({}) - const reloads = useRef({}) - + const requestsRef = useRef>({}) const computeKey = useCallback( (key: string) => `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${key}`, [cacheKeyPrefix], ) - const setCachedData = useCallback( - (compute: CachedData | ((data: CachedData) => CachedData)) => { - if (typeof compute === 'function') { - setCachedDataPrivate(current => compute(current)) - } else { - setCachedDataPrivate(compute) - } - }, - [], - ) - - const setReloads = useCallback( - (compute: Reloads | ((data: Reloads) => Reloads)) => { - if (typeof compute === 'function') { - reloads.current = compute(reloads.current) - } else { - reloads.current = compute - } - }, - [], - ) - - const addCachedData = useCallback( - (key: string, newData: unknown) => { - if (newData) { - if (key && typeof key === 'string') { - setCachedData(actualCachedData => ({ - ...actualCachedData, - [computeKey(key)]: newData, - })) - } else throw new Error(KEY_IS_NOT_STRING_ERROR) - } - }, - [setCachedData, computeKey], - ) - - const addReload = useCallback( - (key: string, method: () => Promise) => { - if (method) { - if (key && typeof key === 'string') { - setReloads(actualReloads => ({ - ...actualReloads, - [computeKey(key)]: method, - })) - } else throw new Error(KEY_IS_NOT_STRING_ERROR) - } - }, - [setReloads, computeKey], + const getRequest = useCallback( + (key: string) => requestsRef.current[computeKey(key)], + [computeKey], ) const addRequest = useCallback( (key: string, args: UseDataLoaderInitializerArgs) => { + if (DataLoader.maxConcurrent !== maxConcurrentRequests) { + DataLoader.maxConcurrent = maxConcurrentRequests as number + } if (key && typeof key === 'string') { - const notifyChanges = (updatedRequest: DataLoader) => { - setRequestQueue(current => ({ - ...current, - [computeKey(updatedRequest.key)]: updatedRequest, - })) - } - const newRequest = new DataLoader({ ...args, notify: notifyChanges }) - newRequest.addOnSuccessListener(result => { - if (result !== undefined && result !== null) - addCachedData(key, result) + const newRequest = new DataLoader({ + ...args, + key: computeKey(key), }) - setRequestQueue(current => ({ - ...current, - [computeKey(key)]: newRequest, - })) - addReload(key, () => newRequest.load(true)) + requestsRef.current[newRequest.key] = newRequest return newRequest } throw new Error(KEY_IS_NOT_STRING_ERROR) }, - [computeKey, addCachedData, addReload], + [computeKey, maxConcurrentRequests], ) - const getRequest = useCallback( - (key: string) => requestQueue[computeKey(key)], - [computeKey, requestQueue], - ) - - const clearReload = useCallback( - (key?: string) => { - if (key && typeof key === 'string') { - setReloads(actualReloads => { - const tmp = actualReloads - delete tmp[computeKey(key)] + const getOrAddRequest = useCallback( + (key: string, args: UseDataLoaderInitializerArgs) => { + const requestFound = getRequest(key) + if (!requestFound) { + return addRequest(key, args) + } - return tmp - }) - } else throw new Error(KEY_IS_NOT_STRING_ERROR) + return requestFound }, - [setReloads, computeKey], + [addRequest, getRequest], ) - const clearAllReloads = useCallback(() => { - setReloads({}) - }, [setReloads]) - const clearCachedData = useCallback( - (key?: string) => { - if (key && typeof key === 'string') { - setCachedData(actualCachedData => { - const tmp = actualCachedData - delete tmp[computeKey(key)] - - return tmp - }) + (key: string) => { + if (typeof key === 'string') { + if (requestsRef.current[computeKey(key)]) { + requestsRef.current[computeKey(key)].clearData() + } } else throw new Error(KEY_IS_NOT_STRING_ERROR) }, - [setCachedData, computeKey], + [computeKey], ) const clearAllCachedData = useCallback(() => { - setCachedData({}) - }, [setCachedData]) + Object.values(requestsRef.current).forEach(request => { + request.clearData() + }) + }, []) const reload = useCallback( async (key?: string) => { if (key && typeof key === 'string') { - await reloads.current[computeKey(key)]?.() + await getRequest(key)?.load(true) } else throw new Error(KEY_IS_NOT_STRING_ERROR) }, - [computeKey], + [getRequest], ) const reloadAll = useCallback(async () => { await Promise.all( - Object.values(reloads.current).map(reloadFn => reloadFn()), + Object.values(requestsRef.current).map(request => request.load(true)), ) }, []) const getCachedData = useCallback( (key?: string) => { if (key) { - return cachedData[computeKey(key)] + return getRequest(key)?.getData() } - return cachedData + return Object.values(requestsRef.current).reduce( + (acc, request) => ({ + ...acc, + [request.key]: request.getData(), + }), + {} as CachedData, + ) }, - [computeKey, cachedData], + [getRequest], ) const getReloads = useCallback( (key?: string) => { if (key) { - return reloads.current[computeKey(key)] + return getRequest(key) ? () => getRequest(key).load(true) : undefined } - return reloads.current + return Object.entries(requestsRef.current).reduce( + (acc, [requestKey, { load }]) => ({ + ...acc, + [requestKey]: () => load(true), + }), + {} as Reloads, + ) }, - [computeKey], + [getRequest], ) + useEffect(() => { + const cleanRequest = () => { + setTimeout(() => { + Object.keys(requestsRef.current).forEach(key => { + if (requestsRef.current[key].getObserversCount() === 0) { + requestsRef.current[key].destroy() + delete requestsRef.current[key] + } + }) + cleanRequest() + }, 300) + } + + cleanRequest() + }, []) + const value = useMemo( () => ({ - addCachedData, - addReload, addRequest, cacheKeyPrefix, clearAllCachedData, - clearAllReloads, clearCachedData, - clearReload, getCachedData, + getOrAddRequest, getReloads, getRequest, onError, @@ -241,15 +212,12 @@ const DataLoaderProvider = ({ reloadAll, }), [ - addCachedData, - addReload, addRequest, cacheKeyPrefix, clearAllCachedData, - clearAllReloads, clearCachedData, - clearReload, getCachedData, + getOrAddRequest, getRequest, getReloads, onError, @@ -259,7 +227,7 @@ const DataLoaderProvider = ({ ) return ( - + {children} ) @@ -268,14 +236,17 @@ const DataLoaderProvider = ({ DataLoaderProvider.propTypes = { cacheKeyPrefix: PropTypes.string, children: PropTypes.node.isRequired, + maxConcurrentRequests: PropTypes.number, onError: PropTypes.func, } DataLoaderProvider.defaultProps = { cacheKeyPrefix: undefined, + maxConcurrentRequests: DEFAULT_MAX_CONCURRENT_REQUESTS, onError: undefined, } -export const useDataLoaderContext = (): Context => useContext(DataLoaderContext) +export const useDataLoaderContext = (): IDataLoaderContext => + useContext(DataLoaderContext) export default DataLoaderProvider diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx index 68ce02fd4..af218ffa5 100644 --- a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx +++ b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx @@ -4,6 +4,14 @@ import React, { ReactNode } from 'react' import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' import { KEY_IS_NOT_STRING_ERROR, StatusEnum } from '../constants' +const TEST_KEY = 'test' +const PROMISE_TIMEOUT = 5 +const fakePromise = () => + new Promise(resolve => setTimeout(() => resolve(true), PROMISE_TIMEOUT)) + +const fakeNullPromise = () => + new Promise(resolve => setTimeout(() => resolve(null), PROMISE_TIMEOUT)) + const wrapper = ({ children }: { children: ReactNode }) => ( {children} ) @@ -12,258 +20,149 @@ const wrapperWithCacheKey = ({ children }: { children?: React.ReactNode }) => ( {children} ) +const wrapperWith2ConcurrentRequests = ({ + children, +}: { + children?: React.ReactNode +}) => ( + {children} +) + describe('DataLoaderProvider', () => { test('should render correctly', () => { render(Test) expect(screen.getByText('Test')).toBeTruthy() }) - test('should add cached data', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addCachedData('test', undefined) - result.current.addCachedData('test', 'test') - }) - - try { - // @ts-expect-error used because we test with bad key - result.current.addCachedData(3, 'testWrong') - throw new Error('It should throw an error before') - } catch (err) { - expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) - } - - expect( - Object.values(result.current.getCachedData() as Record) - .length, - ).toBe(1) - expect( - (result.current.getCachedData() as Record).test, - ).toBe('test') - }) - - test('should delete cached data', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - 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') - - try { - result.current.clearCachedData() - throw new Error('It should throw an error before') - } catch (err) { - expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) - } - - act(() => { - result.current.clearCachedData('testA') - }) - expect( - Object.values(result.current.getCachedData() as Record) - .length, - ).toBe(4) - expect( - (result.current.getCachedData() as Record).testA, - ).toBe(undefined) - - act(() => { - result.current.clearAllCachedData() - }) - expect( - Object.values(result.current.getCachedData() as Record) - .length, - ).toBe(0) - expect(result.current.getCachedData()).toStrictEqual({}) - }) - - test('should get cached data', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addCachedData('test', 'test') + test('should add request', async () => { + const method = jest.fn(fakePromise) + const { result, waitFor } = renderHook(useDataLoaderContext, { + wrapper, }) - - expect( - Object.values(result.current.getCachedData() as Record) - .length, - ).toBe(1) - expect(result.current.getCachedData('test')).toBe('test') - expect(result.current.getCachedData()).toStrictEqual({ test: 'test' }) - expect(result.current.getCachedData('scaleway')).toBe(undefined) - }) - - test('should add reload', async () => { - const fn = () => new Promise(resolve => resolve(true)) - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) + expect(result.current.getRequest(TEST_KEY)).toBeUndefined() act(() => { - // @ts-expect-error used because we test with bad method - result.current.addReload('test', undefined) - result.current.addReload('test', fn) + result.current.addRequest(TEST_KEY, { + method, + }) }) - try { - // @ts-expect-error used because we test with bad key - result.current.addReload(1, () => new Promise(resolve => resolve(true))) - throw new Error('It should throw an error before') - } catch (err) { - expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) - } - expect(Object.values(result.current.getReloads()).length).toBe(1) - expect(result.current.getReloads('test')).toBe(fn) - expect(result.current.getReloads('testWrong')).toBe(undefined) - expect( - await ( - result.current.getReloads() as unknown as { - test: () => Promise - } - ).test(), - ).toBe(true) - }) - - test('should clear reload', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addReload( - 'testA', - () => new Promise(resolve => resolve('testA')), - ) - result.current.addReload( - 'testB', - () => new Promise(resolve => resolve('testB')), - ) - result.current.addReload( - 'testC', - () => new Promise(resolve => resolve('testC')), - ) - result.current.addReload( - 'testD', - () => new Promise(resolve => resolve('testD')), - ) - }) - - expect(Object.values(result.current.getReloads()).length).toBe(4) - - act(() => { - result.current.clearReload('testA') - }) - expect(Object.values(result.current.getReloads()).length).toBe(3) - expect((result.current.getReloads() as Record).testA).toBe( - undefined, - ) - + const testRequest = result.current.getRequest(TEST_KEY) + expect(testRequest).toBeDefined() + expect(testRequest?.status).toBe(StatusEnum.IDLE) act(() => { - result.current.clearAllReloads() + testRequest?.launch() }) - expect(Object.values(result.current.getReloads()).length).toBe(0) - }) - - test('should trigger reload', async () => { - const reloadFn = jest.fn() - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addReload('testA', reloadFn) - result.current.addReload('testB', reloadFn) + expect(method).toBeCalledTimes(1) + expect(testRequest.status).toBe(StatusEnum.LOADING) + await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) + expect(result.current.getCachedData(TEST_KEY)).toBe(true) + await act(async () => { + await result.current.reload(TEST_KEY) }) - expect(Object.values(result.current.getReloads()).length).toBe(2) - expect( - Object.values( - result.current.getReloads('testA') as () => Promise, - ), - ).toBeDefined() - try { - await result.current.reload() - throw new Error('It should throw an error before') - } catch (err) { - expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) - } - await result.current.reload('testA') - expect(reloadFn).toBeCalledTimes(1) - - try { - result.current.clearReload() - throw new Error('It should throw an error before') - } catch (err) { - expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) - } - expect(Object.values(result.current.getReloads()).length).toBe(2) - - const multipleReloads = jest.fn() - + const observer = jest.fn() act(() => { - result.current.clearAllReloads() - result.current.addReload('testA', multipleReloads) - result.current.addReload('testB', multipleReloads) - result.current.addReload('testC', multipleReloads) - result.current.addReload('testD', multipleReloads) + const request = result.current.getRequest(TEST_KEY) + request.addObserver(observer) + expect(request.getObserversCount()).toBe(1) }) - - await result.current.reloadAll() - expect(multipleReloads).toBeCalledTimes(4) }) - test('should add request', async () => { - const method = jest.fn(() => Promise.resolve(true)) - const { result, waitForNextUpdate } = renderHook(useDataLoaderContext, { - wrapper, + test('should add request with cache key prefix', async () => { + const method = jest.fn(fakePromise) + const { result, waitFor } = renderHook(useDataLoaderContext, { + wrapper: wrapperWithCacheKey, }) - expect(result.current.getRequest('test')).toBeUndefined() + expect(result.current.getRequest(TEST_KEY)).toBeUndefined() act(() => { - result.current.addRequest('test', { - key: 'test', + result.current.addRequest(TEST_KEY, { method, }) }) + const testRequest = result.current.getRequest(TEST_KEY) expect(Object.values(result.current.getReloads()).length).toBe(1) - expect(result.current.getRequest('test')).toBeDefined() - expect(result.current.getRequest('test')?.status).toBe(StatusEnum.IDLE) + expect(testRequest).toBeDefined() + expect(testRequest?.status).toBe(StatusEnum.IDLE) act(() => { - result.current.getRequest('test')?.launch() + testRequest?.launch() }) expect(method).toBeCalledTimes(1) - await waitForNextUpdate() - expect(result.current.getCachedData('test')).toBe(true) + await waitFor(() => expect(testRequest.getData()).toBe(true)) + expect(result.current.getCachedData(TEST_KEY)).toBe(true) + await act(async () => { + await result.current.reload(TEST_KEY) + }) }) - test('should add request with result is null thus no data is cached', async () => { - const method = jest.fn(() => Promise.resolve(null)) - const { result, waitForNextUpdate } = renderHook(useDataLoaderContext, { + test('should add request with result is null', async () => { + const method = jest.fn(fakeNullPromise) + const { result, waitFor } = renderHook(useDataLoaderContext, { wrapper, }) act(() => { - // eslint-disable-next-line no-void - void result.current - .addRequest('test', { - enabled: true, - key: 'test', - method, - }) - .launch() + result.current.addRequest(TEST_KEY, { + enabled: true, + method, + }) }) - await waitForNextUpdate() expect(method).toBeCalledTimes(1) - expect(result.current.getCachedData('test')).toBe(undefined) + await waitFor(() => + expect(result.current.getRequest(TEST_KEY).status).toBe( + StatusEnum.SUCCESS, + ), + ) + const reloads = result.current.getReloads() + expect(reloads).toHaveProperty(TEST_KEY) + const testReload = result.current.getReloads(TEST_KEY) + expect(result.current.getCachedData(TEST_KEY)).toBe(undefined) + expect(result.current.getCachedData()).toStrictEqual({ test: undefined }) + expect(result.current.getRequest(TEST_KEY)).toBeDefined() + await act(async () => { + await reloads.test() + }) + expect(method).toBeCalledTimes(2) + expect(testReload).toBeDefined() + await act(async () => { + await (testReload as () => Promise)() + }) + expect(method).toBeCalledTimes(3) + await act(result.current.reloadAll) + expect(method).toBeCalledTimes(4) + act(() => result.current.clearCachedData(TEST_KEY)) + act(() => result.current.clearCachedData('unknown')) + expect(result.current.getCachedData(TEST_KEY)).toBeUndefined() + act(() => result.current.clearAllCachedData()) + expect(result.current.getCachedData()).toStrictEqual({ test: undefined }) + await waitFor(() => + expect(result.current.getRequest(TEST_KEY)).toBeUndefined(), + ) + + try { + // @ts-expect-error Should throw an error + result.current.clearCachedData(3) + } catch (e) { + expect((e as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } + expect(result.current.getReloads()).not.toHaveProperty(TEST_KEY) + expect(result.current.getReloads(TEST_KEY)).not.toBeDefined() + expect(result.current.getCachedData(TEST_KEY)).toBe(undefined) + expect(result.current.getCachedData()).not.toStrictEqual({ + test: undefined, + }) + try { + // @ts-expect-error Should throw an error + await result.current.reload(3) + } catch (e) { + expect((e as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } }) test('should add request with bad key', () => { - const method = jest.fn(() => Promise.resolve(true)) + const method = jest.fn(fakePromise) const { result } = renderHook(useDataLoaderContext, { wrapper, }) @@ -271,7 +170,6 @@ describe('DataLoaderProvider', () => { act(() => { // @ts-expect-error used because we test with bad key result.current.addRequest(3, { - key: 'test', method, }) }) @@ -280,13 +178,37 @@ describe('DataLoaderProvider', () => { } }) - test('should add cached data with prefix', () => { - const { result } = renderHook(useDataLoaderContext, { - wrapper: wrapperWithCacheKey, + test('should delay max concurrent request', async () => { + const method = jest.fn( + () => new Promise(resolve => setTimeout(() => resolve(true), 100)), + ) + const { result, waitFor } = renderHook(useDataLoaderContext, { + wrapper: wrapperWith2ConcurrentRequests, }) act(() => { - result.current.addCachedData('test', 'test') + result.current.addRequest(TEST_KEY, { + enabled: true, + method, + }) + result.current.addRequest(`${TEST_KEY}-2`, { + enabled: true, + method, + }) + result.current.addRequest(`${TEST_KEY}-3`, { + enabled: true, + method, + }) + result.current.addRequest(`${TEST_KEY}-4`, { + enabled: true, + method, + }) + result.current.addRequest(`${TEST_KEY}-5`, { + enabled: true, + method, + }) }) - expect(result.current.getCachedData('test')).toBeDefined() + expect(method).toBeCalledTimes(2) + await waitFor(() => expect(method).toBeCalledTimes(4)) + await waitFor(() => expect(method).toBeCalledTimes(5)) }) }) diff --git a/packages/use-dataloader/src/__tests__/dataloader.test.ts b/packages/use-dataloader/src/__tests__/dataloader.test.ts index e480a41da..ebd21427c 100644 --- a/packages/use-dataloader/src/__tests__/dataloader.test.ts +++ b/packages/use-dataloader/src/__tests__/dataloader.test.ts @@ -8,6 +8,15 @@ const fakeSuccessPromise = () => setTimeout(() => resolve(true), PROMISE_TIMEOUT) }) +const fakeNullPromise = () => + new Promise(resolve => { + setTimeout(() => resolve(null), PROMISE_TIMEOUT) + }) +const fakeUndefinedPromise = () => + new Promise(resolve => { + setTimeout(() => resolve(undefined), PROMISE_TIMEOUT) + }) + const fakeErrorPromise = () => new Promise((_, reject) => { setTimeout(() => reject(new Error('test')), PROMISE_TIMEOUT) @@ -15,49 +24,107 @@ const fakeErrorPromise = () => describe('Dataloader class', () => { test('should create instance not enabled then load then destroy', async () => { - const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const instance = new DataLoader({ key: 'test', method, - notify, }) expect(instance.status).toBe(StatusEnum.IDLE) - expect(notify).toBeCalledTimes(0) expect(method).toBeCalledTimes(0) await instance.load() expect(method).toBeCalledTimes(1) - expect(notify).toBeCalledTimes(2) instance.destroy() + instance.clearData() }) - test('should create instance enabled then load', async () => { + test('should create instance enabled with cancel', async () => { const notify = jest.fn() - const method = jest.fn() + const method = jest.fn(fakeSuccessPromise) const instance = new DataLoader({ enabled: true, key: 'test', method, - notify, }) + instance.addObserver(notify) expect(instance.status).toBe(StatusEnum.LOADING) + expect(instance.getData()).toBe(undefined) expect(notify).toBeCalledTimes(0) - expect(method).toBeCalledTimes(0) - await instance.load() - // This does nothing because no cancel listener is set + expect(method).toBeCalledTimes(1) await instance.cancel() + await new Promise(resolve => + setTimeout(() => { + expect(notify).toBeCalledTimes(1) + resolve(true) + }, PROMISE_TIMEOUT), + ) + expect(instance.getData()).toBe(undefined) + instance.clearData() + }) + + test('should create instance enabled without cancel', async () => { + const notify = jest.fn() + const method = jest.fn(fakeSuccessPromise) + const instance = new DataLoader({ + enabled: true, + key: 'test', + method, + }) + instance.addObserver(notify) + expect(instance.status).toBe(StatusEnum.LOADING) + expect(notify).toBeCalledTimes(0) + expect(method).toBeCalledTimes(1) + await new Promise(resolve => + setTimeout(() => { + expect(notify).toBeCalledTimes(1) + resolve(true) + }, PROMISE_TIMEOUT), + ) + expect(instance.getData()).toBe(true) + instance.getObserversCount() + instance.removeObserver(notify) + instance.removeObserver(notify) + instance.clearData() + }) + + test('should create instance enabled with null data', async () => { + const method = jest.fn(fakeNullPromise) + const instance = new DataLoader({ + enabled: true, + key: 'test', + method, + }) + expect(instance.status).toBe(StatusEnum.LOADING) + expect(method).toBeCalledTimes(1) + await new Promise(resolve => + setTimeout(() => { + resolve(true) + }, PROMISE_TIMEOUT), + ) + expect(instance.getData()).toBe(undefined) + }) + test('should create instance enabled with undefined data', async () => { + const method = jest.fn(fakeUndefinedPromise) + const instance = new DataLoader({ + enabled: true, + key: 'test', + method, + }) + expect(instance.status).toBe(StatusEnum.LOADING) expect(method).toBeCalledTimes(1) - expect(notify).toBeCalledTimes(1) + await new Promise(resolve => + setTimeout(() => { + resolve(true) + }, PROMISE_TIMEOUT), + ) + expect(instance.getData()).toBe(undefined) }) test('should create instance with cancel listener and success', async () => { - const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const onCancel = jest.fn() const instance = new DataLoader({ key: 'test', method, - notify, }) instance.addOnCancelListener(onCancel) instance.addOnCancelListener(onCancel) @@ -70,13 +137,11 @@ describe('Dataloader class', () => { }) test('should create instance with cancel listener and error', async () => { - const notify = jest.fn() const method = jest.fn(fakeErrorPromise) const onCancel = jest.fn() const instance = new DataLoader({ key: 'test', method, - notify, }) instance.addOnCancelListener(onCancel) instance.addOnCancelListener(onCancel) @@ -89,13 +154,11 @@ describe('Dataloader class', () => { }) test('should create instance with success listener', async () => { - const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const onSuccess = jest.fn() const instance = new DataLoader({ key: 'test', method, - notify, }) instance.addOnSuccessListener(onSuccess) instance.addOnSuccessListener(onSuccess) @@ -106,13 +169,11 @@ describe('Dataloader class', () => { }) test('should create instance with error listener', async () => { - const notify = jest.fn() const method = jest.fn(fakeErrorPromise) const onError = jest.fn() const instance = new DataLoader({ key: 'test', method, - notify, }) instance.addOnErrorListener(onError) instance.addOnErrorListener(onError) @@ -124,12 +185,10 @@ describe('Dataloader class', () => { }) test('should create instance with polling', async () => { - const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const instance = new DataLoader({ key: 'test', method, - notify, pollingInterval: PROMISE_TIMEOUT, }) await instance.load() @@ -140,6 +199,7 @@ describe('Dataloader class', () => { expect(method).toBeCalledTimes(3) await instance.load() await instance.load() + await instance.load() expect(method).toBeCalledTimes(4) await instance.load() await instance.load() @@ -149,7 +209,6 @@ describe('Dataloader class', () => { }) test('should update outdated data', async () => { - const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const onSuccess = jest.fn() const instance = new DataLoader({ @@ -157,14 +216,11 @@ describe('Dataloader class', () => { key: 'test', maxDataLifetime: PROMISE_TIMEOUT, method, - notify, }) instance.addOnSuccessListener(onSuccess) expect(instance.status).toBe(StatusEnum.LOADING) - expect(method).toBeCalledTimes(0) - expect(onSuccess).toBeCalledTimes(0) - await instance.load() expect(method).toBeCalledTimes(1) + await new Promise(resolve => setTimeout(resolve, PROMISE_TIMEOUT)) expect(onSuccess).toBeCalledTimes(1) await instance.load() expect(method).toBeCalledTimes(1) @@ -174,4 +230,20 @@ describe('Dataloader class', () => { expect(method).toBeCalledTimes(2) expect(onSuccess).toBeCalledTimes(2) }) + + test('should launch 2 concurrent requests', async () => { + const method = jest.fn(fakeSuccessPromise) + DataLoader.maxConcurrent = 2 + for (let i = 0; i < 5; i += 1) { + const instance = new DataLoader({ + enabled: true, + key: `test-${i}`, + method, + }) + expect(instance.status).toBe(StatusEnum.LOADING) + } + // Because wait for setTimeout tryLaunch in dataloader.ts + await new Promise(resolve => setTimeout(resolve)) + expect(method).toBeCalledTimes(2) + }) }) diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx index f0e64c5ed..2258cf3bc 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx @@ -20,10 +20,12 @@ const initialProps = { keepPreviousData: true, }, key: 'test', - method: () => - new Promise(resolve => { - setTimeout(() => resolve(true), PROMISE_TIMEOUT) - }), + method: jest.fn( + () => + new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }), + ), } // eslint-disable-next-line react/prop-types const wrapper = ({ children }: { children?: React.ReactNode }) => ( @@ -42,7 +44,7 @@ const wrapperWithOnError = describe('useDataLoader', () => { test('should render correctly without options', async () => { - const { result, waitForNextUpdate, rerender } = renderHook< + const { result, waitForNextUpdate } = renderHook< UseDataLoaderHookProps, UseDataLoaderResult >(props => useDataLoader(props.key, props.method), { @@ -51,11 +53,13 @@ describe('useDataLoader', () => { }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - rerender() + expect(result.current.previousData).toBe(undefined) await waitForNextUpdate() - expect(result.current.data).toBe(true) - expect(result.current.isSuccess).toBe(true) + expect(initialProps.method).toBeCalledTimes(1) expect(result.current.isLoading).toBe(false) + expect(result.current.isSuccess).toBe(true) + expect(result.current.previousData).toBe(undefined) + expect(result.current.data).toBe(true) }) test('should render correctly without valid key', () => { @@ -83,6 +87,7 @@ describe('useDataLoader', () => { config: { keepPreviousData: false, }, + key: 'test-2', }, wrapper, }) @@ -101,6 +106,7 @@ describe('useDataLoader', () => { >(props => useDataLoader(props.key, props.method, props.config), { initialProps: { ...initialProps, + key: 'test-3', method: () => new Promise(resolve => setTimeout(() => resolve(null), PROMISE_TIMEOUT), @@ -123,7 +129,7 @@ describe('useDataLoader', () => { >( props => [ useDataLoader(props.key, props.method, props.config), - useDataLoader('test-2', props.method, { + useDataLoader('test-4', props.method, { ...props.config, enabled: false, }), @@ -160,7 +166,10 @@ describe('useDataLoader', () => { UseDataLoaderHookProps, ReturnType >(props => useDataLoader(props.key, props.method, props.config), { - initialProps, + initialProps: { + ...initialProps, + key: 'test-5', + }, wrapper, }) expect(result.current.data).toBe(undefined) @@ -190,7 +199,7 @@ describe('useDataLoader', () => { test('should render correctly with key update', async () => { const propsToPass = { ...initialProps, - key: 'test', + key: 'test-key-update', } const { result, waitForNextUpdate, rerender } = renderHook< UseDataLoaderHookProps, @@ -225,7 +234,7 @@ describe('useDataLoader', () => { config: { pollingInterval: PROMISE_TIMEOUT, }, - key: 'test', + key: 'test-6', method: jest.fn( () => new Promise(resolve => { @@ -316,6 +325,7 @@ describe('useDataLoader', () => { config: { enabled: false, }, + key: 'test-7', }, wrapper, }) @@ -347,6 +357,7 @@ describe('useDataLoader', () => { config: { onSuccess, }, + key: 'test-8', }, wrapper, }) @@ -371,7 +382,7 @@ describe('useDataLoader', () => { onError, onSuccess, }, - key: 'test', + key: 'test-9', method: () => new Promise((resolve, reject) => { setTimeout(() => { @@ -384,9 +395,9 @@ describe('useDataLoader', () => { expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() - expect(result.current.data).toBe(undefined) expect(result.current.error).toBe(error) expect(result.current.isError).toBe(true) + expect(result.current.data).toBe(undefined) expect(onError).toBeCalledTimes(1) expect(onError).toBeCalledWith(error) @@ -407,7 +418,7 @@ describe('useDataLoader', () => { onError, onSuccess, }, - key: 'test', + key: 'test-10', method: () => new Promise((_, reject) => { setTimeout(() => { @@ -442,7 +453,7 @@ describe('useDataLoader', () => { config: { onSuccess, }, - key: 'test', + key: 'test-11', method: () => new Promise((_, reject) => { setTimeout(() => { @@ -481,7 +492,7 @@ describe('useDataLoader', () => { onError, onSuccess, }, - key: 'test', + key: 'test-12', method: () => new Promise((resolve, reject) => { setTimeout(() => { @@ -498,9 +509,9 @@ describe('useDataLoader', () => { expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() - expect(result.current.data).toBe(undefined) expect(result.current.error).toBe(error) expect(result.current.isError).toBe(true) + expect(result.current.data).toBe(undefined) expect(onError).toBeCalledTimes(1) expect(onSuccess).toBeCalledTimes(0) @@ -531,7 +542,10 @@ describe('useDataLoader', () => { }), ], { - initialProps, + initialProps: { + ...initialProps, + key: 'test-13', + }, wrapper, }, ) @@ -539,8 +553,8 @@ describe('useDataLoader', () => { 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(false) expect(result.current[1].isLoading).toBe(true) + expect(result.current[1].isIdle).toBe(false) await waitForNextUpdate() expect(result.current[0].data).toBe(true) expect(result.current[0].isSuccess).toBe(true) @@ -580,6 +594,7 @@ describe('useDataLoader', () => { { initialProps: { ...initialProps, + key: 'test-15', method: mockedFn, }, wrapper, @@ -630,7 +645,10 @@ describe('useDataLoader', () => { props.config, ), { - initialProps, + initialProps: { + ...initialProps, + key: 'test-16', + }, wrapper, }, ) @@ -667,7 +685,10 @@ describe('useDataLoader', () => { props.config, ), { - initialProps, + initialProps: { + ...initialProps, + key: 'test-17', + }, wrapper, }, ) diff --git a/packages/use-dataloader/src/constants.ts b/packages/use-dataloader/src/constants.ts index a9472cf48..183d2dc8e 100644 --- a/packages/use-dataloader/src/constants.ts +++ b/packages/use-dataloader/src/constants.ts @@ -5,12 +5,5 @@ export enum StatusEnum { SUCCESS = 'success', } -export enum ActionEnum { - ON_ERROR = 'ON_ERROR', - ON_LOADING = 'ON_LOADING', - ON_SUCCESS = 'ON_SUCCESS', - ON_UPDATE_DATA = 'ON_UPDATE_DATA', - RESET = 'RESET', -} - export const KEY_IS_NOT_STRING_ERROR = 'Key should be a string' +export const DEFAULT_MAX_CONCURRENT_REQUESTS = 20 diff --git a/packages/use-dataloader/src/dataloader.ts b/packages/use-dataloader/src/dataloader.ts index a624f8230..2d83d44a7 100644 --- a/packages/use-dataloader/src/dataloader.ts +++ b/packages/use-dataloader/src/dataloader.ts @@ -1,4 +1,4 @@ -import { StatusEnum } from './constants' +import { DEFAULT_MAX_CONCURRENT_REQUESTS, StatusEnum } from './constants' import { OnCancelFn, OnErrorFn, OnSuccessFn, PromiseType } from './types' export type DataLoaderConstructorArgs = { @@ -8,10 +8,15 @@ export type DataLoaderConstructorArgs = { pollingInterval?: number maxDataLifetime?: number keepPreviousData?: boolean - notify: (updatedRequest: DataLoader) => void } class DataLoader { + public static started = 0 + + public static maxConcurrent = DEFAULT_MAX_CONCURRENT_REQUESTS + + public static cachedData = {} as Record + public key: string public status: StatusEnum @@ -22,8 +27,6 @@ class DataLoader { public isDataOutdated = false - private notify: (updatedRequest: DataLoader) => void - public method: () => PromiseType private cancelMethod?: () => void @@ -38,20 +41,45 @@ class DataLoader { private cancelListeners: Array = [] + private observerListeners: Array<(dataloader: DataLoader) => void> = [] + public error?: Error private dataOutdatedTimeout?: number public timeout?: number + private destroyed = false + public constructor(args: DataLoaderConstructorArgs) { this.key = args.key this.status = args.enabled ? StatusEnum.LOADING : StatusEnum.IDLE this.method = args.method this.pollingInterval = args?.pollingInterval this.keepPreviousData = args?.keepPreviousData - this.notify = args.notify this.maxDataLifetime = args.maxDataLifetime + if (args.enabled) { + const tryLaunch = () => { + if (DataLoader.started < DataLoader.maxConcurrent) { + // Because we want to launch the request directly without waiting the return + // eslint-disable-next-line no-void + void this.load() + } else { + setTimeout(tryLaunch) + } + } + tryLaunch() + } else { + this.notifyChanges() + } + } + + public getData(): T | undefined { + return (DataLoader.cachedData[this.key] as T) ?? undefined + } + + private notifyChanges(): void { + this.observerListeners.forEach(observerListener => observerListener(this)) } public load = async (force = false): Promise => { @@ -68,21 +96,23 @@ class DataLoader { } } - private launch = async (): Promise => { + public launch = async (): Promise => { try { if (this.status !== StatusEnum.LOADING) { this.canceled = false this.status = StatusEnum.LOADING - this.notify(this) + this.notifyChanges() } + DataLoader.started += 1 const promise = this.method() this.cancelMethod = promise.cancel const result = await promise.then(res => res) this.status = StatusEnum.SUCCESS this.error = undefined - this.notify(this) if (!this.canceled) { + DataLoader.cachedData[this.key] = result + await Promise.all( this.successListeners.map(listener => listener?.(result)), ) @@ -92,23 +122,27 @@ class DataLoader { clearTimeout(this.dataOutdatedTimeout) this.dataOutdatedTimeout = undefined } + if (this.maxDataLifetime) { this.dataOutdatedTimeout = setTimeout(() => { this.isDataOutdated = true + this.notifyChanges() }, this.maxDataLifetime) as unknown as number } } + this.notifyChanges() } catch (err) { this.status = StatusEnum.ERROR this.error = err as Error - this.notify(this) + this.notifyChanges() if (!this.canceled) { await Promise.all( this.errorListeners.map(listener => listener?.(err as Error)), ) } } - if (this.pollingInterval) { + DataLoader.started -= 1 + if (this.pollingInterval && !this.destroyed) { this.timeout = setTimeout( // eslint-disable-next-line @typescript-eslint/no-misused-promises this.launch, @@ -162,10 +196,32 @@ class DataLoader { } } + public addObserver(fn: (args: DataLoader) => void): void { + this.observerListeners.push(fn) + } + + public removeObserver(fn: (args: DataLoader) => void): void { + const index = this.observerListeners.indexOf(fn) + if (index > -1) { + this.observerListeners.splice(index, 1) + } + } + + public clearData(): void { + DataLoader.cachedData[this.key] = undefined + this.notifyChanges() + } + public destroy(): void { + this.cancel?.() if (this.timeout) { clearTimeout(this.timeout) } + this.destroyed = true + } + + public getObserversCount(): number { + return this.observerListeners.length } } diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index 2a18fccc1..af0243210 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -1,6 +1,14 @@ -import { useEffect, useLayoutEffect, useMemo, useRef } from 'react' +import { + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, +} from 'react' import { useDataLoaderContext } from './DataLoaderProvider' import { StatusEnum } from './constants' +import DataLoader from './dataloader' import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from './types' /** @@ -22,43 +30,43 @@ const useDataLoader = ( maxDataLifetime, }: UseDataLoaderConfig = {}, ): UseDataLoaderResult => { - const { - addRequest, - getCachedData, - getRequest, - onError: onErrorProvider, - } = useDataLoaderContext() - - const data = useMemo( - () => (getCachedData(fetchKey) || initialData) as T, - [getCachedData, fetchKey, initialData], - ) + const isMountedRef = useRef(false) + const isFetchingRef = useRef(false) + const unsubscribeRequestRef = useRef<() => void>() + const previousDataRef = useRef() + const { getOrAddRequest, onError: onErrorProvider } = useDataLoaderContext() + const [, forceReload] = useState(0) + const subscribeFn = useCallback(() => { + forceReload(x => x + 1) + }, []) - const request = useMemo( - () => - getRequest(fetchKey) ?? - addRequest(fetchKey, { - key: fetchKey, - maxDataLifetime, - method, - pollingInterval, - }), - [ - addRequest, - fetchKey, - getRequest, + const request = useMemo(() => { + if (unsubscribeRequestRef.current) { + unsubscribeRequestRef.current() + } + + const newRequest = getOrAddRequest(fetchKey, { + enabled, + keepPreviousData, + maxDataLifetime, method, pollingInterval, - maxDataLifetime, - ], - ) + }) as DataLoader - useEffect(() => { - if (enabled && request.status === StatusEnum.IDLE) { - // eslint-disable-next-line no-void - void request.load() - } - }, [request, enabled]) + unsubscribeRequestRef.current = () => newRequest.removeObserver(subscribeFn) + newRequest.addObserver(subscribeFn) + + return newRequest + }, [ + enabled, + fetchKey, + getOrAddRequest, + maxDataLifetime, + method, + pollingInterval, + keepPreviousData, + subscribeFn, + ]) useEffect(() => { if (request.method !== method) { @@ -74,10 +82,6 @@ const useDataLoader = ( }, [onSuccess, onError, onErrorProvider, method, request]) const cancelMethodRef = useRef<(() => void) | undefined>(request?.cancel) - const isMountedRef = useRef(false) - const isFetchingRef = useRef(false) - - const previousDataRef = useRef() const isLoading = useMemo( () => request.status === StatusEnum.LOADING, @@ -116,20 +120,18 @@ const useDataLoader = ( if (isFetchingRef.current && cancelMethodRef.current) { cancelMethodRef.current() } + unsubscribeRequestRef.current?.() } }, []) - useEffect( - () => () => { - if (data !== previousDataRef.current) { - previousDataRef.current = data - } - }, - [data, keepPreviousData], - ) + useEffect(() => () => { + if (request.getData() !== previousDataRef.current && keepPreviousData) { + previousDataRef.current = request.getData() + } + }) return { - data: (getCachedData(fetchKey) || initialData) as T, + data: request.getData() || (initialData as T), error: request?.error, isError, isIdle,