From a308c26b8dcb8353f367ee6994e6af16e06dc040 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 19 May 2022 15:00:26 +0200 Subject: [PATCH 01/12] feat: dataloader v2 --- .../use-dataloader/src/DataLoaderProvider.tsx | 12 +- .../src/__tests__/DataLoaderProvider.test.tsx | 162 ++++------- .../src/__tests__/dataloader.test.ts | 220 +++----------- .../src/__tests__/useDataLoader.test.tsx | 166 +++-------- .../__tests__/usePaginatedDataLoader.test.tsx | 3 +- packages/use-dataloader/src/dataloader.ts | 269 ++++++------------ packages/use-dataloader/src/types.ts | 24 +- packages/use-dataloader/src/useDataLoader.ts | 201 +++++-------- 8 files changed, 299 insertions(+), 758 deletions(-) diff --git a/packages/use-dataloader/src/DataLoaderProvider.tsx b/packages/use-dataloader/src/DataLoaderProvider.tsx index 6c9a46793..81ef3e085 100644 --- a/packages/use-dataloader/src/DataLoaderProvider.tsx +++ b/packages/use-dataloader/src/DataLoaderProvider.tsx @@ -11,24 +11,21 @@ import { import { DEFAULT_MAX_CONCURRENT_REQUESTS, KEY_IS_NOT_STRING_ERROR, - StatusEnum, } from './constants' import DataLoader from './dataloader' -import { NeedPollingType, OnErrorFn, PromiseType } from './types' +import { OnErrorFn, PromiseType } from './types' type CachedData = Record type Reloads = Record Promise> +type Requests = Record type UseDataLoaderInitializerArgs = { - status?: StatusEnum method: () => PromiseType - pollingInterval?: number - keepPreviousData?: boolean /** * Max time before data from previous success is considered as outdated (in millisecond) */ maxDataLifetime?: number - needPolling?: NeedPollingType + enabled?: boolean } type GetCachedDataFn = { @@ -72,7 +69,8 @@ const DataLoaderProvider = ({ onError: OnErrorFn maxConcurrentRequests?: number }): ReactElement => { - const requestsRef = useRef>({}) + const requestsRef = useRef({}) + const computeKey = useCallback( (key: string) => `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${key}`, [cacheKeyPrefix], diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx index 14a0102be..9da2d4012 100644 --- a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx +++ b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx @@ -40,40 +40,32 @@ describe('DataLoaderProvider', () => { test('should add request', async () => { const method = jest.fn(fakePromise) - const { result, waitFor } = renderHook(useDataLoaderContext, { - wrapper, - }) + const { result, waitForNextUpdate, rerender } = renderHook( + useDataLoaderContext, + { + wrapper, + }, + ) expect(result.current.getRequest(TEST_KEY)).toBeUndefined() - act(() => { - result.current.addRequest(TEST_KEY, { - method, - }) + result.current.addRequest(TEST_KEY, { + method, }) expect(Object.values(result.current.getReloads()).length).toBe(1) const testRequest = result.current.getRequest(TEST_KEY) + testRequest.addObserver(rerender) + expect(testRequest).toBeDefined() expect(testRequest.status).toBe(StatusEnum.IDLE) - act(() => { - // launch should never throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - testRequest.load() - }) - expect(method).toBeCalledTimes(1) + testRequest.load().catch(undefined) + await waitForNextUpdate() expect(testRequest.status).toBe(StatusEnum.LOADING) - await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) + expect(method).toBeCalledTimes(1) + await waitForNextUpdate() + expect(testRequest.status).toBe(StatusEnum.SUCCESS) expect(result.current.getCachedData(TEST_KEY)).toBe(true) - await act(async () => { - await result.current.reload(TEST_KEY) - }) - - const observer = jest.fn() - act(() => { - const request = result.current.getRequest(TEST_KEY) - request.addObserver(observer) - expect(request.getObserversCount()).toBe(1) - }) + await result.current.reload(TEST_KEY) }) test('should add request with cache key prefix', async () => { @@ -83,91 +75,50 @@ describe('DataLoaderProvider', () => { }) expect(result.current.getRequest(TEST_KEY)).toBeUndefined() - act(() => { - result.current.addRequest(TEST_KEY, { - method, - }) + result.current.addRequest(TEST_KEY, { + method, }) const testRequest = result.current.getRequest(TEST_KEY) expect(Object.values(result.current.getReloads()).length).toBe(1) expect(testRequest).toBeDefined() expect(testRequest.status).toBe(StatusEnum.IDLE) - act(() => { - // launch should never throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - testRequest.load() - }) + testRequest.load().catch(undefined) expect(method).toBeCalledTimes(1) await waitFor(() => expect(testRequest.getData()).toBe(true)) expect(result.current.getCachedData(TEST_KEY)).toBe(true) - await act(async () => { - await result.current.reload(TEST_KEY) - }) + await result.current.reload(TEST_KEY) }) test('should add request with result is null', async () => { const method = jest.fn(fakeNullPromise) - const { result, waitFor } = renderHook(useDataLoaderContext, { + const { result } = renderHook(useDataLoaderContext, { wrapper, }) - act(() => { - const request = result.current.getOrAddRequest(TEST_KEY, { - method, - }) - result.current.getOrAddRequest(TEST_KEY, { - method, - }) - // eslint-disable-next-line @typescript-eslint/no-floating-promises - request.load() + const request = result.current.getOrAddRequest(TEST_KEY, { + method, }) + await request.load() expect(method).toBeCalledTimes(1) - await waitFor(() => - expect(result.current.getRequest(TEST_KEY).status).toBe( - StatusEnum.SUCCESS, - ), - ) + 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(testReload).toBeDefined() + expect(result.current.getCachedData(TEST_KEY)).toBe(null) + expect(result.current.getCachedData()).toStrictEqual({ test: null }) expect(result.current.getRequest(TEST_KEY)).toBeDefined() const unknownReload = result.current.getReloads('unknown') expect(unknownReload).toBeUndefined() - await act(async () => { - await reloads.test() - }) + 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(method).toBeCalledTimes(3) + result.current.clearCachedData(TEST_KEY) + result.current.clearCachedData('unknown') expect(result.current.getCachedData(TEST_KEY)).toBeUndefined() - act(() => result.current.clearAllCachedData()) + result.current.clearAllCachedData() expect(result.current.getCachedData()).toStrictEqual({ test: undefined }) - - 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.getCachedData(TEST_KEY)).toBe(undefined) - expect(result.current.getCachedData()).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', () => { @@ -176,11 +127,9 @@ describe('DataLoaderProvider', () => { wrapper, }) try { - act(() => { - // @ts-expect-error used because we test with bad key - result.current.addRequest(3, { - method, - }) + // @ts-expect-error used because we test with bad key + result.current.addRequest(3, { + method, }) } catch (e) { expect((e as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) @@ -197,27 +146,24 @@ describe('DataLoaderProvider', () => { const { result, waitFor } = renderHook(useDataLoaderContext, { wrapper: wrapperWith2ConcurrentRequests, }) - act(() => { - ;[ - result.current.addRequest(TEST_KEY, { - method, - }), - result.current.addRequest(`${TEST_KEY}-2`, { - method, - }), - result.current.addRequest(`${TEST_KEY}-3`, { - method, - }), - result.current.addRequest(`${TEST_KEY}-4`, { - method, - }), - result.current.addRequest(`${TEST_KEY}-5`, { - method, - }), - ].forEach(request => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - request.load() - }) + ;[ + result.current.addRequest(TEST_KEY, { + method, + }), + result.current.addRequest(`${TEST_KEY}-2`, { + method, + }), + result.current.addRequest(`${TEST_KEY}-3`, { + method, + }), + result.current.addRequest(`${TEST_KEY}-4`, { + method, + }), + result.current.addRequest(`${TEST_KEY}-5`, { + method, + }), + ].forEach(request => { + request.load().catch(undefined) }) expect(method).toBeCalledTimes(2) await waitFor(() => expect(method).toBeCalledTimes(4)) diff --git a/packages/use-dataloader/src/__tests__/dataloader.test.ts b/packages/use-dataloader/src/__tests__/dataloader.test.ts index 0adc4a7f9..b301df460 100644 --- a/packages/use-dataloader/src/__tests__/dataloader.test.ts +++ b/packages/use-dataloader/src/__tests__/dataloader.test.ts @@ -1,5 +1,4 @@ import waitForExpect from 'wait-for-expect' -import { StatusEnum } from '../constants' import DataLoader from '../dataloader' const PROMISE_TIMEOUT = 100 @@ -9,6 +8,11 @@ const fakeSuccessPromise = () => setTimeout(() => resolve(true), PROMISE_TIMEOUT) }) +const fakeLongSuccessPromise = () => + new Promise(resolve => { + setTimeout(() => resolve(true), 200) + }) + const fakeNullPromise = () => new Promise(resolve => { setTimeout(() => resolve(null), PROMISE_TIMEOUT) @@ -26,34 +30,32 @@ const fakeErrorPromise = () => describe('Dataloader class', () => { test('should create instance then load then destroy', async () => { const method = jest.fn(fakeSuccessPromise) + const notifyChanges = jest.fn() const instance = new DataLoader({ key: 'test', method, + notifyChanges, }) - expect(instance.status).toBe(StatusEnum.IDLE) expect(method).toBeCalledTimes(0) await instance.load() - expect(instance.status).toBe(StatusEnum.SUCCESS) expect(method).toBeCalledTimes(1) - await instance.destroy() instance.clearData() }) - test('should create instance with cancel', async () => { + test('should create instance with cancel', () => { const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const instance = new DataLoader({ key: 'test', method, + notifyChanges: notify, }) - instance.addObserver(notify) expect(instance.getData()).toBe(undefined) expect(notify).toBeCalledTimes(0) - // eslint-disable-next-line @typescript-eslint/no-floating-promises - instance.load() + instance.load().catch(undefined) expect(method).toBeCalledTimes(1) - await instance.cancel() - expect(notify).toBeCalledTimes(1) + instance.cancel() + expect(notify).toBeCalledTimes(0) expect(instance.getData()).toBe(undefined) instance.clearData() }) @@ -64,227 +66,77 @@ describe('Dataloader class', () => { const instance = new DataLoader({ key: 'test', method, + notifyChanges: notify, }) - instance.addObserver(notify) expect(notify).toBeCalledTimes(0) - expect(instance.status).toBe(StatusEnum.IDLE) await instance.load() - expect(notify).toBeCalledTimes(2) expect(method).toBeCalledTimes(1) + expect(notify).toBeCalledTimes(1) expect(instance.getData()).toBe(true) - instance.getObserversCount() - instance.removeObserver(notify) - instance.removeObserver(notify) instance.clearData() }) test('should create instance with null data', async () => { const method = jest.fn(fakeNullPromise) + const notifyChanges = jest.fn() const instance = new DataLoader({ key: 'test', method, + notifyChanges, }) - expect(instance.status).toBe(StatusEnum.IDLE) await instance.load() expect(method).toBeCalledTimes(1) - expect(instance.getData()).toBe(undefined) + expect(instance.getData()).toBe(null) }) test('should create instance with undefined data', async () => { const method = jest.fn(fakeUndefinedPromise) + const notifyChanges = jest.fn() + const instance = new DataLoader({ key: 'test', method, + notifyChanges, }) - expect(instance.status).toBe(StatusEnum.IDLE) await instance.load() expect(method).toBeCalledTimes(1) expect(instance.getData()).toBe(undefined) }) - test('should create instance with cancel listener and success', async () => { - const method = jest.fn(fakeSuccessPromise) - const onCancel = jest.fn() - const instance = new DataLoader({ - key: 'test', - method, - }) - instance.addOnCancelListener(onCancel) - instance.addOnCancelListener(onCancel) - // eslint-disable-next-line @typescript-eslint/no-floating-promises - instance.load() - await instance.cancel() - expect(onCancel).toBeCalledTimes(1) - instance.removeOnCancelListener(onCancel) - instance.removeOnCancelListener(onCancel) - }) - test('should create instance with cancel listener and error', async () => { const method = jest.fn(fakeErrorPromise) - const onCancel = jest.fn() - const instance = new DataLoader({ - key: 'test', - method, - }) - instance.addOnCancelListener(onCancel) - instance.addOnCancelListener(onCancel) - // eslint-disable-next-line @typescript-eslint/no-floating-promises - instance.load() - await instance.cancel() - expect(onCancel).toBeCalledTimes(1) - instance.removeOnCancelListener(onCancel) - instance.removeOnCancelListener(onCancel) - }) - - test('should create instance with success listener', async () => { - const method = jest.fn(fakeSuccessPromise) - const onSuccess = jest.fn() - const instance = new DataLoader({ - key: 'test', - method, - }) - instance.addOnSuccessListener(onSuccess) - instance.addOnSuccessListener(onSuccess) - await instance.load() - expect(onSuccess).toBeCalledTimes(1) - instance.removeOnSuccessListener(onSuccess) - instance.removeOnSuccessListener(onSuccess) - }) - - test('should create instance with error listener', async () => { - const method = jest.fn(fakeErrorPromise) + const notifyChanges = jest.fn() const onError = jest.fn() - const instance = new DataLoader({ - key: 'test', - method, - }) - instance.addOnErrorListener(onError) - instance.addOnErrorListener(onError) - await instance.load() - expect(onError).toBeCalledTimes(1) - expect(instance.error?.message).toBe('test') - instance.removeOnErrorListener(onError) - instance.removeOnErrorListener(onError) - }) - test('should create instance with polling', async () => { - const method = jest.fn(fakeSuccessPromise) const instance = new DataLoader({ key: 'test', method, - pollingInterval: PROMISE_TIMEOUT, - }) - await instance.load() - expect(method).toBeCalledTimes(1) - await waitForExpect(() => { - expect(method).toBeCalledTimes(2) - }) - await waitForExpect(() => { - expect(method).toBeCalledTimes(3) - }) - await instance.load() - await instance.load() - await waitForExpect(() => { - expect(method).toBeCalledTimes(4) - }) - await waitForExpect(() => { - expect(method).toBeCalledTimes(5) + notifyChanges, }) - await instance.load() - await instance.load() - await instance.load(true) - await waitForExpect(() => { - expect(method).toBeCalledTimes(6) - }) - instance.setPollingInterval(PROMISE_TIMEOUT * 2) - await instance.destroy() - }) - - test('should create instance with polling and needPolling', async () => { - const method = jest.fn(fakeSuccessPromise) - const instance = new DataLoader({ - key: 'test', - method, - needPolling: () => true, - pollingInterval: PROMISE_TIMEOUT * 2, - }) - await instance.load() - expect(method).toBeCalledTimes(1) - // eslint-disable-next-line @typescript-eslint/no-floating-promises - await instance.load() - await waitForExpect(() => { - expect(method).toBeCalledTimes(2) - }) - await waitForExpect(() => { - expect(method).toBeCalledTimes(3) - }) - await waitForExpect(() => { - expect(method).toBeCalledTimes(4) - }) - await instance.load() - await instance.load() - await instance.load(true) - await waitForExpect(() => { - expect(method).toBeCalledTimes(5) - }) - instance.setPollingInterval(PROMISE_TIMEOUT * 4) - await instance.destroy() - }) - - test('should create instance with polling and needPolling that return false', async () => { - const method = jest.fn(fakeSuccessPromise) - const instance = new DataLoader({ - key: 'test', - method, - needPolling: () => false, - pollingInterval: PROMISE_TIMEOUT * 2, - }) - await instance.load() - expect(method).toBeCalledTimes(1) - await new Promise(resolve => { - setTimeout(resolve, PROMISE_TIMEOUT * 3) - }) - expect(method).toBeCalledTimes(1) - instance.setNeedPolling(true) - await instance.destroy() - }) - - test('should update outdated data', async () => { - const method = jest.fn(fakeSuccessPromise) - const onSuccess = jest.fn() - const instance = new DataLoader({ - key: 'test', - maxDataLifetime: PROMISE_TIMEOUT * 3, - method, - }) - instance.addOnSuccessListener(onSuccess) - expect(instance.status).toBe(StatusEnum.IDLE) - await instance.load() - expect(method).toBeCalledTimes(1) - expect(onSuccess).toBeCalledTimes(1) - await instance.load() - expect(method).toBeCalledTimes(1) - expect(onSuccess).toBeCalledTimes(1) - // Wait until data is outdated - await waitForExpect(() => expect(instance.isDataOutdated).toBeTruthy()) - await instance.load() - expect(method).toBeCalledTimes(2) - expect(onSuccess).toBeCalledTimes(2) + await instance.load().catch(onError) + expect(notifyChanges).toBeCalledTimes(1) + expect(onError).toBeCalledTimes(1) }) - test('should launch 2 concurrent requests', async () => { - const method = jest.fn(fakeSuccessPromise) + test.only('should launch 2 concurrent requests', async () => { + const method = jest.fn(fakeLongSuccessPromise) + const notifyChanges = jest.fn() DataLoader.maxConcurrent = 2 for (let i = 0; i < 5; i += 1) { const instance = new DataLoader({ key: `test-${i}`, method, + notifyChanges, }) - // eslint-disable-next-line @typescript-eslint/no-floating-promises - instance.load() + instance.load().catch(undefined) } - // Because wait for setTimeout tryLaunch in dataloader.ts await waitForExpect(() => { expect(method).toBeCalledTimes(2) }) + await waitForExpect(() => { + expect(method).toBeCalledTimes(4) + }) + await waitForExpect(() => { + expect(method).toBeCalledTimes(5) + }) }) }) diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx index bc0277b6f..96c108894 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx @@ -1,8 +1,9 @@ -import { act, renderHook } from '@testing-library/react-hooks' +/* eslint-disable no-console */ +import { renderHook } from '@testing-library/react-hooks' import { ReactNode } from 'react' import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' import { KEY_IS_NOT_STRING_ERROR } from '../constants' -import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from '../types' +import { UseDataLoaderConfig, UseDataLoaderResult } from '../types' import useDataLoader from '../useDataLoader' type UseDataLoaderHookProps = { @@ -12,7 +13,7 @@ type UseDataLoaderHookProps = { children?: ReactNode } -const PROMISE_TIMEOUT = 5 +const PROMISE_TIMEOUT = 50 const initialProps = { config: { @@ -52,15 +53,16 @@ describe('useDataLoader', () => { expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) expect(result.current.previousData).toBe(undefined) + expect(initialProps.method).toBeCalledTimes(1) await waitForNextUpdate() expect(initialProps.method).toBeCalledTimes(1) + expect(result.current.data).toBe(true) 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 requets enabled then enable it', async () => { + test('should render correctly without request enabled then enable it', async () => { const method = jest.fn( () => new Promise(resolve => { @@ -90,6 +92,7 @@ describe('useDataLoader', () => { expect(method).toBeCalledTimes(0) enabled = true rerender() + await waitForNextUpdate() expect(method).toBeCalledTimes(1) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) @@ -155,7 +158,7 @@ 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.data).toBe(null) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) }) @@ -186,11 +189,8 @@ describe('useDataLoader', () => { expect(result.current[0].data).toBe(true) expect(result.current[0].isSuccess).toBe(true) - act((): void => { - // eslint-disable-next-line no-void - void result.current[1].reload() - }) - + result.current[1].reload().catch(undefined) + await waitForNextUpdate() expect(result.current[1].data).toBe(undefined) expect(result.current[1].isLoading).toBe(true) @@ -216,16 +216,9 @@ describe('useDataLoader', () => { expect(result.current.data).toBe(true) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) - - act(() => { - // eslint-disable-next-line no-void - void result.current.reload() - }) - act(() => { - // eslint-disable-next-line no-void - void result.current.reload() - }) - + result.current.reload().catch(undefined) + result.current.reload().catch(undefined) + await waitForNextUpdate() expect(result.current.data).toBe(true) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() @@ -319,9 +312,11 @@ describe('useDataLoader', () => { }, method: method2, }) - await waitForNextUpdate() expect(result.current.data).toBe(true) expect(result.current.isPolling).toBe(true) + expect(result.current.isSuccess).toBe(true) + expect(result.current.isLoading).toBe(false) + await waitForNextUpdate() expect(result.current.isLoading).toBe(true) expect(result.current.isSuccess).toBe(false) await waitForNextUpdate() @@ -367,17 +362,11 @@ describe('useDataLoader', () => { }) expect(result.current.data).toBe(undefined) expect(result.current.isIdle).toBe(true) - - act(() => { - // eslint-disable-next-line no-void - void result.current.reload() - }) - + result.current.reload().catch(undefined) + await waitForNextUpdate() 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) }) @@ -420,7 +409,7 @@ describe('useDataLoader', () => { }, key: 'test-9', method: () => - new Promise((resolve, reject) => { + new Promise((_, reject) => { setTimeout(() => { reject(error) }, PROMISE_TIMEOUT) @@ -552,11 +541,8 @@ describe('useDataLoader', () => { expect(onError).toBeCalledTimes(1) expect(onSuccess).toBeCalledTimes(0) - act(() => { - // eslint-disable-next-line no-void - void result.current.reload() - }) - + result.current.reload().catch(undefined) + await waitForNextUpdate() await waitForNextUpdate() expect(result.current.data).toBe(true) @@ -566,6 +552,7 @@ describe('useDataLoader', () => { }) test('should use cached data', async () => { + const fakePromise = jest.fn(initialProps.method) const { result, waitForNextUpdate } = renderHook< UseDataLoaderHookProps, UseDataLoaderResult[] @@ -581,6 +568,7 @@ describe('useDataLoader', () => { initialProps: { ...initialProps, key: 'test-13', + method: fakePromise, }, wrapper, }, @@ -588,23 +576,24 @@ describe('useDataLoader', () => { expect(result.current[0].data).toBe(undefined) expect(result.current[0].isLoading).toBe(true) + expect(result.current[0].isIdle).toBe(false) + expect(result.current[0].isSuccess).toBe(false) expect(result.current[1].data).toBe(undefined) - expect(result.current[1].isLoading).toBe(true) expect(result.current[1].isIdle).toBe(false) + expect(result.current[1].isSuccess).toBe(false) + expect(result.current[1].isLoading).toBe(true) await waitForNextUpdate() expect(result.current[0].data).toBe(true) expect(result.current[0].isSuccess).toBe(true) - act(() => { - // eslint-disable-next-line no-void - void result.current[1].reload() - }) - + result.current[1].reload().catch(undefined) + await waitForNextUpdate() expect(result.current[1].data).toBe(true) expect(result.current[1].isLoading).toBe(true) await waitForNextUpdate() expect(result.current[1].isSuccess).toBe(true) + expect(fakePromise).toBeCalledTimes(2) }) test('should be reloaded from dataloader context', async () => { @@ -645,11 +634,8 @@ describe('useDataLoader', () => { expect(result.current[0].isSuccess).toBe(true) expect(mockedFn).toBeCalledTimes(1) - act(() => { - // eslint-disable-next-line no-void - void result.current[1].reloadAll() - }) - + result.current[1].reloadAll().catch(undefined) + await waitForNextUpdate() expect(result.current[0].data).toBe(true) expect(Object.values(result.current[1].getReloads()).length).toBe(1) expect(result.current[0].isLoading).toBe(true) @@ -658,89 +644,5 @@ describe('useDataLoader', () => { expect(result.current[0].isSuccess).toBe(true) expect(mockedFn).toBeCalledTimes(2) }) - - /* eslint-disable no-console */ - - test('should cancel request', async () => { - const originalError = console.error - console.error = jest.fn() - 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: { - ...initialProps, - key: 'test-16', - }, - 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) - console.error = originalError - }) - - test('should cancel request with Error', async () => { - const originalError = console.error - console.error = jest.fn() - 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: { - ...initialProps, - key: 'test-17', - }, - 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) - console.error = originalError - }) - - /* eslint-enable no-console */ }) +/* eslint-enable no-console */ diff --git a/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx index 2a41146b9..14611eecb 100644 --- a/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx @@ -86,6 +86,7 @@ describe('useDataLoader', () => { expect(method).toBeCalledTimes(0) enabled = true rerender() + await waitForNextUpdate() expect(method).toBeCalledTimes(1) expect(result.current.pageData).toBe(undefined) expect(result.current.isLoading).toBe(true) @@ -128,7 +129,7 @@ describe('useDataLoader', () => { expect(result.current.pageData).toBe(undefined) expect(result.current.isLoading).toBe(true) await waitForNextUpdate() - expect(result.current.pageData).toBe(undefined) + expect(result.current.pageData).toBe(null) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) }) diff --git a/packages/use-dataloader/src/dataloader.ts b/packages/use-dataloader/src/dataloader.ts index 2116d86df..33a6f01a2 100644 --- a/packages/use-dataloader/src/dataloader.ts +++ b/packages/use-dataloader/src/dataloader.ts @@ -1,248 +1,143 @@ import { DEFAULT_MAX_CONCURRENT_REQUESTS, StatusEnum } from './constants' -import { - NeedPollingType, - OnCancelFn, - OnErrorFn, - OnSuccessFn, - PromiseType, -} from './types' +import { PromiseType } from './types' export type DataLoaderConstructorArgs = { key: string method: () => PromiseType - pollingInterval?: number - needPolling?: NeedPollingType - maxDataLifetime?: number - keepPreviousData?: boolean + enabled?: boolean + notifyChanges?: () => void } -class DataLoader { - public static started = 0 - +class DataLoader { public static maxConcurrent = DEFAULT_MAX_CONCURRENT_REQUESTS - public static cachedData = {} as Record - - public key: string - - public status: StatusEnum - - public pollingInterval?: number - - public needPolling: NeedPollingType = true - - public maxDataLifetime?: number - - public isDataOutdated = false + public static started = 0 - public method: () => PromiseType + public static cachedData = {} as Record - private cancelMethod?: () => void + public static queue = {} as Record - private canceled = false + public key: string - public keepPreviousData?: boolean + public method: () => PromiseType - private errorListeners: Array = [] + public isCalled = false - private successListeners: Array> = [] + public isCancelled = true - private cancelListeners: Array = [] + public status: StatusEnum = StatusEnum.IDLE - private observerListeners: Array<(dataloader: DataLoader) => void> = [] + public error?: ErrorType - public error?: Error + public data?: ResultType - private dataOutdatedTimeout?: number + public observers: Array<() => void> = [] public timeout?: number - private destroyed = false + public loadCount = 0 - public constructor(args: DataLoaderConstructorArgs) { + public constructor(args: DataLoaderConstructorArgs) { this.key = args.key - this.status = StatusEnum.IDLE this.method = args.method - this.pollingInterval = args?.pollingInterval - this.keepPreviousData = args?.keepPreviousData - this.maxDataLifetime = args.maxDataLifetime - this.needPolling = args.needPolling ?? true - this.notifyChanges() + if (args.enabled) { + this.status = StatusEnum.LOADING + } + this.data = DataLoader.cachedData[this.key] as ResultType + if (args.notifyChanges) this.observers.push(args.notifyChanges) } - public getData(): T | undefined { - return (DataLoader.cachedData[this.key] as T) ?? undefined + public notifyChanges() { + if (this.timeout) { + clearTimeout(this.timeout) + } + this.timeout = setTimeout(() => { + this.observers.forEach(observer => observer()) + }) as unknown as number } - private notifyChanges(): void { - this.observerListeners.forEach(observerListener => observerListener(this)) + public getData(): ResultType | undefined { + return DataLoader.cachedData[this.key] as ResultType } - public load = async (force = false): Promise => { - if ( - force || - this.status === StatusEnum.IDLE || - this.status === StatusEnum.ERROR || - (this.status === StatusEnum.SUCCESS && this.isDataOutdated) - ) { - if (DataLoader.started < DataLoader.maxConcurrent) { - // Because we want to launch the request directly without waiting the return - if (this.timeout) { - // Prevent multiple call at the same time - clearTimeout(this.timeout) - } - await this.launch() - } else { - await new Promise(resolve => { - setTimeout(resolve) - }).then(() => this.load(force)) - } + private tryLaunch = async (): Promise => { + if (DataLoader.started < DataLoader.maxConcurrent) { + DataLoader.started += 1 + DataLoader.queue[this.key] = this.launch() + } else { + DataLoader.queue[this.key] = new Promise(resolve => { + setTimeout(resolve) + }).then(() => this.tryLaunch()) } + + return DataLoader.queue[this.key] as Promise } - private launch = async (): Promise => { - try { + public load = async (force = false): Promise => { + if (force || !this.isCalled) { + this.isCalled = true + if (this.status !== StatusEnum.LOADING) { - this.canceled = false this.status = StatusEnum.LOADING 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 - if (!this.canceled) { - DataLoader.cachedData[this.key] = result - - await Promise.all( - this.successListeners.map(listener => listener?.(result)), - ) - - this.isDataOutdated = false - if (this.dataOutdatedTimeout) { - clearTimeout(this.dataOutdatedTimeout) - this.dataOutdatedTimeout = undefined - } - - if (this.maxDataLifetime) { - this.dataOutdatedTimeout = setTimeout(() => { - this.isDataOutdated = true - this.notifyChanges() - }, this.maxDataLifetime) as unknown as number - } else { - this.isDataOutdated = true - } - } - this.notifyChanges() - } catch (err) { - this.status = StatusEnum.ERROR - this.error = err as Error - this.notifyChanges() - if (!this.canceled) { - await Promise.all( - this.errorListeners.map(listener => listener?.(err as Error)), - ) - } - } - DataLoader.started -= 1 - if ( - this.pollingInterval && - !this.destroyed && - (typeof this.needPolling === 'function' - ? this.needPolling(DataLoader.cachedData[this.key] as T) - : this.needPolling) - ) { - this.timeout = setTimeout( - // eslint-disable-next-line @typescript-eslint/no-misused-promises - this.launch, - this.pollingInterval, - ) as unknown as number - } - } - - public cancel = async (): Promise => { - this.canceled = true - this.cancelMethod?.() - await Promise.all(this.cancelListeners.map(listener => listener?.())) - } - - public addOnSuccessListener(fn: OnSuccessFn): void { - if (!this.successListeners.includes(fn)) { - this.successListeners.push(fn) - } - } - public addOnErrorListener(fn: OnErrorFn): void { - if (!this.errorListeners.includes(fn)) { - this.errorListeners.push(fn) + DataLoader.queue[this.key] = this.tryLaunch() } - } - public addOnCancelListener(fn: OnCancelFn): void { - if (!this.cancelListeners.includes(fn)) { - this.cancelListeners.push(fn) - } + return DataLoader.queue[this.key] as Promise } - public removeOnSuccessListener(fn: OnSuccessFn): void { - const index = this.successListeners.indexOf(fn) - if (index > -1) { - this.successListeners.splice(index, 1) - } - } + public launch = async (): Promise => { + try { + this.isCancelled = false + this.loadCount += 1 - public removeOnErrorListener(fn: OnErrorFn): void { - const index = this.errorListeners.indexOf(fn) - if (index > -1) { - this.errorListeners.splice(index, 1) - } - } + const data = await this.method() - public removeOnCancelListener(fn: OnCancelFn): void { - const index = this.cancelListeners.indexOf(fn) - if (index > -1) { - this.cancelListeners.splice(index, 1) - } - } + if (!this.isCancelled) { + DataLoader.cachedData[this.key] = data + this.status = StatusEnum.SUCCESS + this.data = data + this.error = undefined + } + this.isCalled = false + DataLoader.started -= 1 + delete DataLoader.queue[this.key] + this.notifyChanges() - public addObserver(fn: (args: DataLoader) => void): void { - this.observerListeners.push(fn) - } + return data + } catch (error) { + if (!this.isCancelled) { + this.status = StatusEnum.ERROR + this.error = error as ErrorType + } + this.isCalled = false + DataLoader.started -= 1 + delete DataLoader.queue[this.key] + this.notifyChanges() - public removeObserver(fn: (args: DataLoader) => void): void { - const index = this.observerListeners.indexOf(fn) - if (index > -1) { - this.observerListeners.splice(index, 1) + throw error } } public clearData(): void { DataLoader.cachedData[this.key] = undefined - this.notifyChanges() } - public async destroy(): Promise { - DataLoader.cachedData[this.key] = undefined - await this.cancel?.() - if (this.timeout) { - clearTimeout(this.timeout) - } - this.destroyed = true - } - - public getObserversCount(): number { - return this.observerListeners.length + public cancel(): void { + DataLoader.started -= 1 + delete DataLoader.queue[this.key] + this.isCancelled = true } - public setPollingInterval(newPollingInterval?: number): void { - this.pollingInterval = newPollingInterval + public addObserver(observer: () => void) { + this.observers.push(observer) } - public setNeedPolling(needPolling: NeedPollingType): void { - this.needPolling = needPolling + public removeObserver(observer: () => void) { + const index = this.observers.indexOf(observer) + if (index > -1) this.observers.splice(index, 1) } } diff --git a/packages/use-dataloader/src/types.ts b/packages/use-dataloader/src/types.ts index 1e6691a36..efcfaf630 100644 --- a/packages/use-dataloader/src/types.ts +++ b/packages/use-dataloader/src/types.ts @@ -45,9 +45,9 @@ export interface UseDataLoaderConfig { * @property {string} error the error occured during the request * @property {Function} reload reload the data */ -export interface UseDataLoaderResult { +export interface UseDataLoaderResult { data?: T - error?: Error + error?: ErrorType isError: boolean isIdle: boolean isLoading: boolean @@ -65,21 +65,11 @@ export type UsePaginatedDataLoaderMethodParams = { perPage: number } -export type UsePaginatedDataLoaderConfig = { - enabled?: boolean - initialData?: T - keepPreviousData?: boolean - onError?: OnErrorFn - onSuccess?: OnSuccessFn - pollingInterval?: number - /** - * Max time before data from previous success is considered as outdated (in millisecond) - */ - maxDataLifetime?: number - needPolling?: NeedPollingType - initialPage?: number - perPage?: number -} +export type UsePaginatedDataLoaderConfig = + UseDataLoaderConfig & { + initialPage?: number + perPage?: number + } export type UsePaginatedDataLoaderResult = { pageData?: T diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index 3de3b9e91..aa2c771c1 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -1,171 +1,128 @@ -import { - useCallback, - useEffect, - useLayoutEffect, - useMemo, - useRef, - useState, -} from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useDataLoaderContext } from './DataLoaderProvider' import { StatusEnum } from './constants' import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from './types' -/** - * @param {string} key key to save the data fetched in a local cache - * @param {() => PromiseType} 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 = ( +function useDataLoaderV2( fetchKey: string, method: () => PromiseType, { enabled = true, - initialData, - keepPreviousData = true, onError, onSuccess, + keepPreviousData = false, + needPolling = true, pollingInterval, - maxDataLifetime, - needPolling, + initialData, }: UseDataLoaderConfig = {}, -): UseDataLoaderResult => { - 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) +): UseDataLoaderResult { + const { getOrAddRequest, onError: onGlobalError } = useDataLoaderContext() + const methodRef = useRef(method) + const onSuccessRef = useRef(onSuccess) + const onErrorRef = useRef(onError ?? onGlobalError) + const [, setCounter] = useState(0) + const forceRerender = useCallback(() => { + setCounter(current => current + 1) }, []) - const request = useMemo(() => { - if (unsubscribeRequestRef.current) { - unsubscribeRequestRef.current() - } - - const newRequest = getOrAddRequest(fetchKey, { - keepPreviousData, - maxDataLifetime, - method, - needPolling, - pollingInterval, - }) - - unsubscribeRequestRef.current = () => newRequest.removeObserver(subscribeFn) - newRequest.addObserver(subscribeFn) - - return newRequest - }, [ - fetchKey, - getOrAddRequest, - maxDataLifetime, - method, - needPolling, - pollingInterval, - keepPreviousData, - subscribeFn, - ]) + const request = useMemo( + () => + getOrAddRequest(fetchKey, { + enabled, + method: methodRef.current, + }) as DataLoader, + [getOrAddRequest, fetchKey, enabled], + ) useEffect(() => { - if (request.method !== method) { - request.method = method - } - request.addOnErrorListener(onError ?? onErrorProvider) - request.addOnSuccessListener(onSuccess) + request.addObserver(forceRerender) return () => { - request.removeOnErrorListener(onError ?? onErrorProvider) - request.removeOnSuccessListener(onSuccess) + request.removeObserver(forceRerender) } - }, [onSuccess, onError, onErrorProvider, method, request]) + }, [request, forceRerender]) + + const previousDataRef = useRef(request.data) + + const isLoading = request.status === StatusEnum.LOADING + + const isSuccess = request.status === StatusEnum.SUCCESS - const cancelMethodRef = useRef<(() => Promise) | undefined>( - request?.cancel, + const isError = request.status === StatusEnum.ERROR + + const isIdle = request.status === StatusEnum.IDLE && !enabled + + const isPolling = !!( + pollingInterval && + ((typeof needPolling === 'function' && needPolling(request.data)) || + (typeof needPolling !== 'function' && needPolling)) ) - const isLoading = useMemo( + const reload = useCallback( () => - (enabled && request.status === StatusEnum.IDLE) || - request.status === StatusEnum.LOADING, - [request.status, enabled], - ) - const isIdle = useMemo( - () => request.status === StatusEnum.IDLE, - [request.status], - ) - const isSuccess = useMemo( - () => request.status === StatusEnum.SUCCESS, - [request.status], - ) - const isError = useMemo( - () => request.status === StatusEnum.ERROR, - [request.status], - ) - const isPolling = useMemo( - () => !!(enabled && pollingInterval && (isSuccess || isLoading)), - [isSuccess, isLoading, enabled, pollingInterval], + request.load(true).then(onSuccessRef.current).catch(onErrorRef.current), + [request], ) useEffect(() => { - if (enabled) { - // launch should never throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - request.load() - } - }, [request, enabled, isIdle]) + request.method = method + }, [method, request]) useEffect(() => { - if (pollingInterval !== request.pollingInterval) { - request.setPollingInterval(pollingInterval) - } - }, [pollingInterval, request]) + onSuccessRef.current = onSuccess + }, [onSuccess]) useEffect(() => { - if (needPolling !== request.needPolling) { - request.setNeedPolling(needPolling ?? true) - } - }, [needPolling, request]) + onErrorRef.current = onError ?? onGlobalError + }, [onError, onGlobalError]) useEffect(() => { - isFetchingRef.current = isLoading || isPolling - }, [isLoading, isPolling]) - - useLayoutEffect(() => { - cancelMethodRef.current = request?.cancel - }, [request?.cancel]) + if (enabled && request.loadCount === 0) { + if (keepPreviousData) { + previousDataRef.current = request.data + } + request.load().then(onSuccessRef.current).catch(onErrorRef.current) + } + }, [enabled, request, keepPreviousData]) useEffect(() => { - isMountedRef.current = true - - return () => { - isMountedRef.current = false - if (isFetchingRef.current && cancelMethodRef.current) { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - cancelMethodRef.current() + let timeout: NodeJS.Timeout + + if ( + pollingInterval && + needPolling && + (request.status === StatusEnum.SUCCESS || + request.status === StatusEnum.ERROR) + ) { + if ( + (typeof needPolling === 'function' && needPolling(request.data)) || + (typeof needPolling !== 'function' && needPolling) + ) { + timeout = setTimeout(() => { + request + .load(true) + .then(onSuccessRef.current) + .catch(onErrorRef.current) + }, pollingInterval) } - unsubscribeRequestRef.current?.() } - }, []) - useEffect(() => () => { - if (request.getData() !== previousDataRef.current && keepPreviousData) { - previousDataRef.current = request.getData() + return () => { + if (timeout) clearTimeout(timeout) } - }) + }, [pollingInterval, needPolling, request, request.status, request.data]) return { - data: request.getData() ?? (initialData as T), - error: request?.error, + data: request.loadCount > 0 ? request.data : initialData, + error: request.error, isError, isIdle, isLoading, isPolling, isSuccess, previousData: previousDataRef.current, - reload: () => request.load(true), + reload, } } -export default useDataLoader +export default useDataLoaderV2 From 583528ee374ebc035ad4ecc5c5e62f5acf2f6d15 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 19 May 2022 15:14:43 +0200 Subject: [PATCH 02/12] fix: rebase --- .../src/__tests__/dataloader.test.ts | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/packages/use-dataloader/src/__tests__/dataloader.test.ts b/packages/use-dataloader/src/__tests__/dataloader.test.ts index b301df460..d872d6954 100644 --- a/packages/use-dataloader/src/__tests__/dataloader.test.ts +++ b/packages/use-dataloader/src/__tests__/dataloader.test.ts @@ -1,4 +1,3 @@ -import waitForExpect from 'wait-for-expect' import DataLoader from '../dataloader' const PROMISE_TIMEOUT = 100 @@ -8,11 +7,6 @@ const fakeSuccessPromise = () => setTimeout(() => resolve(true), PROMISE_TIMEOUT) }) -const fakeLongSuccessPromise = () => - new Promise(resolve => { - setTimeout(() => resolve(true), 200) - }) - const fakeNullPromise = () => new Promise(resolve => { setTimeout(() => resolve(null), PROMISE_TIMEOUT) @@ -116,27 +110,4 @@ describe('Dataloader class', () => { expect(notifyChanges).toBeCalledTimes(1) expect(onError).toBeCalledTimes(1) }) - - test.only('should launch 2 concurrent requests', async () => { - const method = jest.fn(fakeLongSuccessPromise) - const notifyChanges = jest.fn() - DataLoader.maxConcurrent = 2 - for (let i = 0; i < 5; i += 1) { - const instance = new DataLoader({ - key: `test-${i}`, - method, - notifyChanges, - }) - instance.load().catch(undefined) - } - await waitForExpect(() => { - expect(method).toBeCalledTimes(2) - }) - await waitForExpect(() => { - expect(method).toBeCalledTimes(4) - }) - await waitForExpect(() => { - expect(method).toBeCalledTimes(5) - }) - }) }) From 11eb296d3e21cdfca6416d42bf35407e19e0aca5 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 19 May 2022 15:25:31 +0200 Subject: [PATCH 03/12] fix: typing --- packages/use-dataloader/src/types.ts | 2 +- packages/use-dataloader/src/useDataLoader.ts | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/use-dataloader/src/types.ts b/packages/use-dataloader/src/types.ts index efcfaf630..1655ac84a 100644 --- a/packages/use-dataloader/src/types.ts +++ b/packages/use-dataloader/src/types.ts @@ -7,7 +7,7 @@ export type OnSuccessFn = | ((result: T) => void | Promise) | undefined export type OnCancelFn = (() => void | Promise) | undefined -export type NeedPollingType = boolean | ((data: T) => boolean) +export type NeedPollingType = boolean | ((data?: T) => boolean) /** * @typedef {Object} UseDataLoaderConfig diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index aa2c771c1..8f370343d 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -1,11 +1,12 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useDataLoaderContext } from './DataLoaderProvider' import { StatusEnum } from './constants' +import DataLoader from './dataloader' import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from './types' -function useDataLoaderV2( +function useDataLoaderV2( fetchKey: string, - method: () => PromiseType, + method: () => PromiseType, { enabled = true, onError, @@ -14,8 +15,8 @@ function useDataLoaderV2( needPolling = true, pollingInterval, initialData, - }: UseDataLoaderConfig = {}, -): UseDataLoaderResult { + }: UseDataLoaderConfig = {}, +): UseDataLoaderResult { const { getOrAddRequest, onError: onGlobalError } = useDataLoaderContext() const methodRef = useRef(method) const onSuccessRef = useRef(onSuccess) @@ -30,7 +31,7 @@ function useDataLoaderV2( getOrAddRequest(fetchKey, { enabled, method: methodRef.current, - }) as DataLoader, + }) as DataLoader, [getOrAddRequest, fetchKey, enabled], ) From 36da3c3d7008480b024da225452f78a0d0dc367b Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 19 May 2022 17:45:53 +0200 Subject: [PATCH 04/12] feat: use testing library react --- .../src/__tests__/DataLoaderProvider.test.tsx | 33 +- .../src/__tests__/useDataLoader.test.tsx | 459 ++++++++---------- .../__tests__/usePaginatedDataLoader.test.tsx | 184 ++++--- packages/use-dataloader/src/useDataLoader.ts | 14 +- 4 files changed, 321 insertions(+), 369 deletions(-) diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx index 9da2d4012..c3af2e05e 100644 --- a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx +++ b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx @@ -1,5 +1,4 @@ -import { render, screen } from '@testing-library/react' -import { act, renderHook } from '@testing-library/react-hooks' +import { render, renderHook, screen, waitFor } from '@testing-library/react' import { ReactNode } from 'react' import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' import { KEY_IS_NOT_STRING_ERROR, StatusEnum } from '../constants' @@ -40,12 +39,9 @@ describe('DataLoaderProvider', () => { test('should add request', async () => { const method = jest.fn(fakePromise) - const { result, waitForNextUpdate, rerender } = renderHook( - useDataLoaderContext, - { - wrapper, - }, - ) + const { result, rerender } = renderHook(useDataLoaderContext, { + wrapper, + }) expect(result.current.getRequest(TEST_KEY)).toBeUndefined() result.current.addRequest(TEST_KEY, { @@ -59,18 +55,18 @@ describe('DataLoaderProvider', () => { expect(testRequest).toBeDefined() expect(testRequest.status).toBe(StatusEnum.IDLE) testRequest.load().catch(undefined) - await waitForNextUpdate() expect(testRequest.status).toBe(StatusEnum.LOADING) expect(method).toBeCalledTimes(1) - await waitForNextUpdate() - expect(testRequest.status).toBe(StatusEnum.SUCCESS) + await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) expect(result.current.getCachedData(TEST_KEY)).toBe(true) - await result.current.reload(TEST_KEY) + result.current.reload(TEST_KEY).catch(undefined) + await waitFor(() => expect(testRequest.status).toBe(StatusEnum.LOADING)) + await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) }) test('should add request with cache key prefix', async () => { const method = jest.fn(fakePromise) - const { result, waitFor } = renderHook(useDataLoaderContext, { + const { result } = renderHook(useDataLoaderContext, { wrapper: wrapperWithCacheKey, }) expect(result.current.getRequest(TEST_KEY)).toBeUndefined() @@ -84,10 +80,13 @@ describe('DataLoaderProvider', () => { expect(testRequest).toBeDefined() expect(testRequest.status).toBe(StatusEnum.IDLE) testRequest.load().catch(undefined) + await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) expect(method).toBeCalledTimes(1) - await waitFor(() => expect(testRequest.getData()).toBe(true)) + expect(testRequest.data).toBe(true) expect(result.current.getCachedData(TEST_KEY)).toBe(true) - await result.current.reload(TEST_KEY) + result.current.reload(TEST_KEY).catch(undefined) + await waitFor(() => expect(testRequest.status).toBe(StatusEnum.LOADING)) + await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) }) test('should add request with result is null', async () => { @@ -112,7 +111,7 @@ describe('DataLoaderProvider', () => { expect(unknownReload).toBeUndefined() await reloads.test() expect(method).toBeCalledTimes(2) - await act(result.current.reloadAll) + await result.current.reloadAll() expect(method).toBeCalledTimes(3) result.current.clearCachedData(TEST_KEY) result.current.clearCachedData('unknown') @@ -143,7 +142,7 @@ describe('DataLoaderProvider', () => { setTimeout(() => resolve(true), 100) }), ) - const { result, waitFor } = renderHook(useDataLoaderContext, { + const { result } = renderHook(useDataLoaderContext, { wrapper: wrapperWith2ConcurrentRequests, }) ;[ diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx index 96c108894..2495c907d 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx @@ -1,9 +1,9 @@ /* eslint-disable no-console */ -import { renderHook } from '@testing-library/react-hooks' +import { renderHook, waitFor } from '@testing-library/react' import { ReactNode } from 'react' import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' import { KEY_IS_NOT_STRING_ERROR } from '../constants' -import { UseDataLoaderConfig, UseDataLoaderResult } from '../types' +import { UseDataLoaderConfig } from '../types' import useDataLoader from '../useDataLoader' type UseDataLoaderHookProps = { @@ -43,22 +43,21 @@ const wrapperWithOnError = describe('useDataLoader', () => { test('should render correctly without options', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method), { - initialProps, - wrapper, - }) + const { result } = renderHook( + props => useDataLoader(props.key, props.method), + { + initialProps, + wrapper, + }, + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) expect(result.current.previousData).toBe(undefined) expect(initialProps.method).toBeCalledTimes(1) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(initialProps.method).toBeCalledTimes(1) expect(result.current.data).toBe(true) expect(result.current.isLoading).toBe(false) - expect(result.current.isSuccess).toBe(true) expect(result.current.previousData).toBe(undefined) }) @@ -69,105 +68,101 @@ describe('useDataLoader', () => { setTimeout(() => resolve(true), PROMISE_TIMEOUT) }), ) - let enabled = false - const { rerender, result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >( - props => - useDataLoader(props.key, props.method, { - enabled, - }), + const testProps = { + config: { + enabled: false, + }, + key: 'test-not-enabled-then-reload', + method, + } + const { rerender, result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - key: 'test-not-enabled-then-reload', - method, - }, + initialProps: testProps, wrapper, }, ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(false) expect(method).toBeCalledTimes(0) - enabled = true - rerender() - await waitForNextUpdate() + testProps.config.enabled = true + rerender({ ...testProps }) + await waitFor(() => expect(result.current.isLoading).toBe(true)) expect(method).toBeCalledTimes(1) expect(result.current.data).toBe(undefined) - expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) 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', () => { - const { result } = renderHook( - props => useDataLoader(props.key, props.method), + const orignalConsoleError = console.error + console.error = jest.fn + try { + renderHook( + // @ts-expect-error used because we test with bad key + props => useDataLoader(props.key, props.method), + { + initialProps: { + ...initialProps, + key: 2, + }, + wrapper, + }, + ) + fail('It shoulded fail with a bad key') + } catch (error) { + expect((error as Error)?.message).toBe(KEY_IS_NOT_STRING_ERROR) + } + console.error = orignalConsoleError + }) + + test('should render correctly without keepPreviousData', async () => { + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), { initialProps: { ...initialProps, - // @ts-expect-error used because we test with bad key - key: 2, + config: { + keepPreviousData: false, + }, + key: 'test-2', }, wrapper, }, ) - expect(result.error?.message).toBe(KEY_IS_NOT_STRING_ERROR) - }) - - test('should render correctly without keepPreviousData', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - config: { - keepPreviousData: false, - }, - key: 'test-2', - }, - wrapper, - }) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) - expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) }) test('should render correctly with result null', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - key: 'test-3', - method: () => - new Promise(resolve => { - setTimeout(() => resolve(null), PROMISE_TIMEOUT) - }), + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + ...initialProps, + key: 'test-3', + method: () => + new Promise(resolve => { + setTimeout(() => resolve(null), PROMISE_TIMEOUT) + }), + }, + wrapper, }, - wrapper, - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(null) - expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) }) test('should render and cache correctly with cacheKeyPrefix', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - ReturnType[] - >( + const { result } = renderHook( props => [ useDataLoader(props.key, props.method, props.config), useDataLoader('test-4', props.method, { @@ -185,43 +180,38 @@ describe('useDataLoader', () => { expect(result.current[0].isLoading).toBe(true) expect(result.current[1].data).toBe(undefined) expect(result.current[1].isIdle).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current[0].isSuccess).toBe(true)) expect(result.current[0].data).toBe(true) - expect(result.current[0].isSuccess).toBe(true) result.current[1].reload().catch(undefined) - await waitForNextUpdate() + await waitFor(() => expect(result.current[1].isLoading).toBe(true)) expect(result.current[1].data).toBe(undefined) - expect(result.current[1].isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current[1].isSuccess).toBe(true)) expect(result.current[1].data).toBe(true) - expect(result.current[1].isSuccess).toBe(true) }) test('should render correctly with enabled true', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - ReturnType - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - key: 'test-5', + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + ...initialProps, + key: 'test-5', + }, + wrapper, }, - wrapper, - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) - expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) result.current.reload().catch(undefined) result.current.reload().catch(undefined) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isLoading).toBe(true)) expect(result.current.data).toBe(true) - expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) @@ -232,10 +222,7 @@ describe('useDataLoader', () => { ...initialProps, key: 'test-key-update', } - const { result, waitForNextUpdate, rerender } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >( + const { result, rerender } = renderHook( () => useDataLoader(propsToPass.key, propsToPass.method, propsToPass.config), { @@ -245,7 +232,7 @@ describe('useDataLoader', () => { expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) expect(result.current.data).toBe(true) @@ -254,7 +241,7 @@ describe('useDataLoader', () => { rerender() expect(result.current.isLoading).toBe(true) expect(result.current.data).toBe(undefined) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) @@ -273,7 +260,7 @@ describe('useDataLoader', () => { setTimeout(() => resolve(true), PROMISE_TIMEOUT) }), ), - } + } as UseDataLoaderHookProps const method2 = jest.fn( () => @@ -282,26 +269,26 @@ describe('useDataLoader', () => { }), ) - const { result, waitForNextUpdate, rerender } = renderHook< - UseDataLoaderHookProps, - ReturnType - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: pollingProps, - wrapper, - }) + const { result, rerender } = renderHook( + 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) expect(pollingProps.method).toBeCalledTimes(1) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) expect(result.current.isSuccess).toBe(true) expect(result.current.isPolling).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isLoading).toBe(true)) expect(pollingProps.method).toBeCalledTimes(2) expect(result.current.isPolling).toBe(true) - expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) @@ -316,14 +303,12 @@ describe('useDataLoader', () => { expect(result.current.isPolling).toBe(true) expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) - await waitForNextUpdate() - expect(result.current.isLoading).toBe(true) + await waitFor(() => expect(result.current.isLoading).toBe(true)) expect(result.current.isSuccess).toBe(false) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(method2).toBeCalledTimes(1) expect(result.current.isPolling).toBe(true) expect(result.current.isLoading).toBe(false) - expect(result.current.isSuccess).toBe(true) expect(result.current.data).toBe(2) rerender({ @@ -333,64 +318,59 @@ describe('useDataLoader', () => { }, method: method2, }) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isLoading).toBe(true)) expect(result.current.data).toBe(2) expect(result.current.isPolling).toBe(true) - expect(result.current.isLoading).toBe(true) expect(result.current.isSuccess).toBe(false) expect(method2).toBeCalledTimes(2) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.isPolling).toBe(true) expect(result.current.isLoading).toBe(false) - expect(result.current.isSuccess).toBe(true) expect(result.current.data).toBe(2) }) test('should render correctly with enabled off', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - ReturnType - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - config: { - enabled: false, + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + ...initialProps, + config: { + enabled: false, + }, + key: 'test-7', }, - key: 'test-7', + wrapper, }, - wrapper, - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isIdle).toBe(true) result.current.reload().catch(undefined) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isLoading).toBe(true)) expect(result.current.data).toBe(undefined) - expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) - expect(result.current.isSuccess).toBe(true) }) test('should call onSuccess', async () => { const onSuccess = jest.fn() - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - config: { - onSuccess, + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + ...initialProps, + config: { + onSuccess, + }, + key: 'test-8', }, - key: 'test-8', + wrapper, }, - wrapper, - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) - expect(result.current.isSuccess).toBe(true) expect(onSuccess).toBeCalledTimes(1) }) @@ -398,30 +378,29 @@ describe('useDataLoader', () => { const onSuccess = jest.fn() const onError = jest.fn() const error = new Error('Test error') - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - config: { - onError, - onSuccess, + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + config: { + onError, + onSuccess, + }, + key: 'test-9', + method: () => + new Promise((_, reject) => { + setTimeout(() => { + reject(error) + }, PROMISE_TIMEOUT) + }), }, - key: 'test-9', - method: () => - new Promise((_, reject) => { - setTimeout(() => { - reject(error) - }, PROMISE_TIMEOUT) - }), + wrapper, }, - wrapper, - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isError).toBe(true)) expect(result.current.error).toBe(error) - expect(result.current.isError).toBe(true) expect(result.current.data).toBe(undefined) expect(onError).toBeCalledTimes(1) @@ -434,31 +413,30 @@ describe('useDataLoader', () => { const onError = jest.fn() const error = new Error('Test error') const onErrorProvider = jest.fn() - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - config: { - onError, - onSuccess, + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + config: { + onError, + onSuccess, + }, + key: 'test-10', + method: () => + new Promise((_, reject) => { + setTimeout(() => { + reject(error) + }, PROMISE_TIMEOUT) + }), }, - key: 'test-10', - method: () => - new Promise((_, reject) => { - setTimeout(() => { - reject(error) - }, PROMISE_TIMEOUT) - }), + wrapper: wrapperWithOnError(onErrorProvider), }, - wrapper: wrapperWithOnError(onErrorProvider), - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isError).toBe(true)) expect(result.current.data).toBe(undefined) expect(result.current.error).toBe(error) - expect(result.current.isError).toBe(true) expect(onError).toBeCalledTimes(1) expect(onError).toBeCalledWith(error) @@ -470,28 +448,28 @@ describe('useDataLoader', () => { const onSuccess = jest.fn() const error = new Error('Test error') const onErrorProvider = jest.fn() - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - config: { - onSuccess, + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + config: { + onSuccess, + }, + key: 'test-11', + method: () => + new Promise((_, reject) => { + setTimeout(() => { + reject(error) + }, PROMISE_TIMEOUT) + }), }, - key: 'test-11', - method: () => - new Promise((_, reject) => { - setTimeout(() => { - reject(error) - }, PROMISE_TIMEOUT) - }), + wrapper: wrapperWithOnError(onErrorProvider), }, - wrapper: wrapperWithOnError(onErrorProvider), - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isError).toBe(true)) expect(result.current.data).toBe(undefined) expect(result.current.error).toBe(error) expect(result.current.isError).toBe(true) @@ -508,32 +486,32 @@ describe('useDataLoader', () => { success = true }) const error = new Error('Test error') - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method, props.config), { - initialProps: { - config: { - onError, - onSuccess, + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: { + config: { + onError, + onSuccess, + }, + key: 'test-12', + method: () => + new Promise((resolve, reject) => { + setTimeout(() => { + if (success) { + resolve(true) + } else { + reject(error) + } + }, PROMISE_TIMEOUT) + }), }, - key: 'test-12', - method: () => - new Promise((resolve, reject) => { - setTimeout(() => { - if (success) { - resolve(true) - } else { - reject(error) - } - }, PROMISE_TIMEOUT) - }), + wrapper, }, - wrapper, - }) + ) expect(result.current.data).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isError).toBe(true)) expect(result.current.error).toBe(error) expect(result.current.isError).toBe(true) expect(result.current.data).toBe(undefined) @@ -542,21 +520,15 @@ describe('useDataLoader', () => { expect(onSuccess).toBeCalledTimes(0) result.current.reload().catch(undefined) - await waitForNextUpdate() - await waitForNextUpdate() - + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) expect(result.current.error).toBe(undefined) expect(result.current.isError).toBe(false) - expect(result.current.isSuccess).toBe(true) }) test('should use cached data', async () => { const fakePromise = jest.fn(initialProps.method) - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult[] - >( + const { result } = renderHook( props => [ useDataLoader(props.key, props.method, props.config), useDataLoader(props.key, props.method, { @@ -582,16 +554,14 @@ describe('useDataLoader', () => { expect(result.current[1].isIdle).toBe(false) expect(result.current[1].isSuccess).toBe(false) expect(result.current[1].isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current[0].isSuccess).toBe(true)) expect(result.current[0].data).toBe(true) - expect(result.current[0].isSuccess).toBe(true) result.current[1].reload().catch(undefined) - await waitForNextUpdate() + await waitFor(() => expect(result.current[1].isLoading).toBe(true)) expect(result.current[1].data).toBe(true) - expect(result.current[1].isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current[1].isSuccess).toBe(true)) expect(result.current[1].isSuccess).toBe(true) expect(fakePromise).toBeCalledTimes(2) }) @@ -605,12 +575,12 @@ describe('useDataLoader', () => { }, PROMISE_TIMEOUT) }), ) - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, + const { result } = renderHook< [ ReturnType, ReturnType, - ] + ], + UseDataLoaderHookProps >( props => [ useDataLoader(props.key, props.method, props.config), @@ -629,19 +599,16 @@ describe('useDataLoader', () => { expect(result.current[0].data).toBe(undefined) expect(result.current[0].isLoading).toBe(true) expect(Object.values(result.current[1].getReloads()).length).toBe(1) - await waitForNextUpdate() + await waitFor(() => expect(result.current[0].isSuccess).toBe(true)) expect(result.current[0].data).toBe(true) - expect(result.current[0].isSuccess).toBe(true) expect(mockedFn).toBeCalledTimes(1) result.current[1].reloadAll().catch(undefined) - await waitForNextUpdate() + await waitFor(() => expect(result.current[0].isLoading).toBe(true)) expect(result.current[0].data).toBe(true) expect(Object.values(result.current[1].getReloads()).length).toBe(1) - expect(result.current[0].isLoading).toBe(true) - await waitForNextUpdate() - expect(result.current[0].isSuccess).toBe(true) + await waitFor(() => expect(result.current[0].isSuccess).toBe(true)) expect(mockedFn).toBeCalledTimes(2) }) }) diff --git a/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx index 14611eecb..be20158ae 100644 --- a/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx @@ -1,22 +1,11 @@ -import { act } from '@testing-library/react' -import { renderHook } from '@testing-library/react-hooks' +/* eslint-disable no-console */ +import { act, renderHook, waitFor } from '@testing-library/react' import { ReactNode } from 'react' import DataLoaderProvider from '../DataLoaderProvider' import { KEY_IS_NOT_STRING_ERROR } from '../constants' -import { - UsePaginatedDataLoaderConfig, - UsePaginatedDataLoaderMethodParams, - UsePaginatedDataLoaderResult, -} from '../types' +import { UsePaginatedDataLoaderMethodParams } from '../types' import usePaginatedDataLoader from '../usePaginatedDataLoader' -type UseDataLoaderHookProps = { - config: UsePaginatedDataLoaderConfig - key: string - method: (params: UsePaginatedDataLoaderMethodParams) => Promise - children?: ReactNode -} - const PROMISE_TIMEOUT = 5 const initialProps = { @@ -36,22 +25,21 @@ const wrapper = ({ children }: { children?: ReactNode }) => ( {children} ) -describe('useDataLoader', () => { +describe('usePaginatedDataLoader', () => { test('should render correctly without options', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UsePaginatedDataLoaderResult - >(props => usePaginatedDataLoader(props.key, props.method), { - initialProps, - wrapper, - }) + const { result } = renderHook( + props => usePaginatedDataLoader(props.key, props.method), + { + initialProps, + wrapper, + }, + ) expect(result.current.data).toStrictEqual({}) expect(result.current.pageData).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => 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.data).toStrictEqual({ 1: '1-1' }) expect(result.current.pageData).toBe('1-1') }) @@ -60,95 +48,97 @@ describe('useDataLoader', () => { const method = jest.fn( () => new Promise(resolve => { - setTimeout(() => resolve(true), PROMISE_TIMEOUT) + setTimeout(() => resolve(true), 500) }), ) - let enabled = false - const { rerender, result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UsePaginatedDataLoaderResult - >( - props => - usePaginatedDataLoader(props.key, props.method, { - enabled, - }), + const testProps = { + ...initialProps, + config: { + enabled: false, + }, + key: 'test-not-enabled-then-reload', + method, + } + + const { rerender, result } = renderHook( + props => usePaginatedDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - key: 'test-not-enabled-then-reload', - method, - }, + initialProps: testProps, wrapper, }, ) + expect(result.current.pageData).toBe(undefined) expect(result.current.isLoading).toBe(false) expect(method).toBeCalledTimes(0) - enabled = true - rerender() - await waitForNextUpdate() + testProps.config.enabled = true + + rerender({ ...testProps }) expect(method).toBeCalledTimes(1) - expect(result.current.pageData).toBe(undefined) - expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.isLoading).toBe(false) - expect(result.current.isSuccess).toBe(true) expect(result.current.pageData).toBe(true) }) test('should render correctly without valid key', () => { - const { result } = renderHook< - UseDataLoaderHookProps, - UsePaginatedDataLoaderResult - >(props => usePaginatedDataLoader(props.key, props.method), { - initialProps: { - ...initialProps, + const orignalConsoleError = console.error + console.error = jest.fn + try { + renderHook( // @ts-expect-error used because we test with bad key - key: 2, - }, - wrapper, - }) - expect(result.error?.message).toBe(KEY_IS_NOT_STRING_ERROR) + props => usePaginatedDataLoader(props.key, props.method), + { + initialProps: { + ...initialProps, + key: 2, + }, + wrapper, + }, + ) + fail('It shoulded fail with a bad key') + } catch (error) { + expect((error as Error)?.message).toBe(KEY_IS_NOT_STRING_ERROR) + } + console.error = orignalConsoleError }) test('should render correctly with result null', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UsePaginatedDataLoaderResult - >(props => usePaginatedDataLoader(props.key, props.method, props.config), { - initialProps: { - ...initialProps, - key: 'test-3', - method: () => - new Promise(resolve => { - setTimeout(() => resolve(null), PROMISE_TIMEOUT) - }), + const { result } = renderHook( + props => usePaginatedDataLoader(props.key, props.method, props.config), + { + initialProps: { + ...initialProps, + key: 'test-3', + method: () => + new Promise(resolve => { + setTimeout(() => resolve(null), PROMISE_TIMEOUT) + }), + }, + wrapper, }, - wrapper, - }) + ) expect(result.current.pageData).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.pageData).toBe(null) - expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) }) test('should render correctly then change page', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UsePaginatedDataLoaderResult - >(props => usePaginatedDataLoader(props.key, props.method), { - initialProps: { - ...initialProps, - key: 'test-4', + const { result } = renderHook( + props => usePaginatedDataLoader(props.key, props.method), + { + initialProps: { + ...initialProps, + key: 'test-4', + }, + wrapper, }, - wrapper, - }) + ) expect(result.current.data).toStrictEqual({}) expect(result.current.pageData).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.isLoading).toBe(false) expect(result.current.isSuccess).toBe(true) expect(result.current.data).toStrictEqual({ 1: '1-1' }) @@ -160,9 +150,8 @@ describe('useDataLoader', () => { expect(result.current.page).toBe(2) expect(result.current.pageData).toBe(undefined) expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.isLoading).toBe(false) - expect(result.current.isSuccess).toBe(true) expect(result.current.data).toStrictEqual({ 1: '1-1', 2: '2-1' }) expect(result.current.pageData).toBe('2-1') act(() => { @@ -187,28 +176,29 @@ describe('useDataLoader', () => { ...initialProps, key: 'test-5', } - const { rerender, result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UsePaginatedDataLoaderResult - >(props => usePaginatedDataLoader(props.key, props.method), { - initialProps: hookProps, - wrapper, - }) - await waitForNextUpdate() + const { rerender, result } = renderHook( + props => usePaginatedDataLoader(props.key, props.method), + { + initialProps: hookProps, + wrapper, + }, + ) + await waitFor(() => expect(result.current.data).toStrictEqual({ 1: '1-1' })) act(() => { result.current.goToNextPage() }) - await waitForNextUpdate() - expect(result.current.data).toStrictEqual({ 1: '1-1', 2: '2-1' }) + await waitFor(() => + expect(result.current.data).toStrictEqual({ 1: '1-1', 2: '2-1' }), + ) hookProps.key = 'test-5-bis' - rerender() + rerender(hookProps) expect(result.current.isLoading).toBe(true) expect(result.current.pageData).toBe(undefined) expect(result.current.data).toStrictEqual({}) - await waitForNextUpdate() + await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toStrictEqual({ 1: '1-1' }) expect(result.current.pageData).toBe('1-1') - expect(result.current.isSuccess).toBe(true) expect(result.current.isLoading).toBe(false) }) }) +/* eslint-enable no-console */ diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index 8f370343d..0cc3fe987 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { useDataLoaderContext } from './DataLoaderProvider' import { StatusEnum } from './constants' import DataLoader from './dataloader' @@ -26,14 +26,10 @@ function useDataLoaderV2( setCounter(current => current + 1) }, []) - const request = useMemo( - () => - getOrAddRequest(fetchKey, { - enabled, - method: methodRef.current, - }) as DataLoader, - [getOrAddRequest, fetchKey, enabled], - ) + const request = getOrAddRequest(fetchKey, { + enabled, + method: methodRef.current, + }) as DataLoader useEffect(() => { request.addObserver(forceRerender) From 852119ae85fc69aafe82f2498426b9d7608b12b8 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 19 May 2022 18:08:27 +0200 Subject: [PATCH 05/12] test: improve coverage --- .../src/__tests__/DataLoaderProvider.test.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx index c3af2e05e..3433f5ebc 100644 --- a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx +++ b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx @@ -59,9 +59,24 @@ describe('DataLoaderProvider', () => { expect(method).toBeCalledTimes(1) await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) expect(result.current.getCachedData(TEST_KEY)).toBe(true) + try { + // @ts-expect-error Should throw an error + await result.current.reload(3).catch(undefined) + fail('It should throw an error') + } catch (error) { + expect((error as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } result.current.reload(TEST_KEY).catch(undefined) await waitFor(() => expect(testRequest.status).toBe(StatusEnum.LOADING)) await waitFor(() => expect(testRequest.status).toBe(StatusEnum.SUCCESS)) + try { + // @ts-expect-error Should throw an error + result.current.clearCachedData(3) + fail('It should throw an error') + } catch (error) { + expect((error as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + expect(result.current.getCachedData(TEST_KEY)).toBe(true) + } }) test('should add request with cache key prefix', async () => { From 33b7ad7a9aa602c0d1ebdc1a47e7f354323e07c3 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 19 May 2022 18:35:50 +0200 Subject: [PATCH 06/12] test: improve coverage --- .../src/__tests__/dataloader.test.ts | 83 +++++++++++- .../src/__tests__/useDataLoader.test.tsx | 123 ++++++++++++++++++ packages/use-dataloader/src/dataloader.ts | 9 +- 3 files changed, 206 insertions(+), 9 deletions(-) diff --git a/packages/use-dataloader/src/__tests__/dataloader.test.ts b/packages/use-dataloader/src/__tests__/dataloader.test.ts index d872d6954..a716352db 100644 --- a/packages/use-dataloader/src/__tests__/dataloader.test.ts +++ b/packages/use-dataloader/src/__tests__/dataloader.test.ts @@ -1,3 +1,5 @@ +import waitForExpect from 'wait-for-expect' +import { StatusEnum } from '../constants' import DataLoader from '../dataloader' const PROMISE_TIMEOUT = 100 @@ -26,21 +28,38 @@ describe('Dataloader class', () => { const method = jest.fn(fakeSuccessPromise) const notifyChanges = jest.fn() const instance = new DataLoader({ - key: 'test', + key: 'test-destroy', method, notifyChanges, }) + instance.removeObserver(() => {}) expect(method).toBeCalledTimes(0) await instance.load() expect(method).toBeCalledTimes(1) instance.clearData() }) + test('should create instance then load multiple times', async () => { + const method = jest.fn(fakeSuccessPromise) + const notifyChanges = jest.fn() + const instance = new DataLoader({ + key: 'test-load', + method, + notifyChanges, + }) + expect(method).toBeCalledTimes(0) + instance.load().catch(undefined) + instance.load().catch(undefined) + await instance.load() + expect(method).toBeCalledTimes(1) + instance.clearData() + }) + test('should create instance with cancel', () => { const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const instance = new DataLoader({ - key: 'test', + key: 'test-cancel', method, notifyChanges: notify, }) @@ -58,7 +77,7 @@ describe('Dataloader class', () => { const notify = jest.fn() const method = jest.fn(fakeSuccessPromise) const instance = new DataLoader({ - key: 'test', + key: 'test-without-cancel', method, notifyChanges: notify, }) @@ -74,7 +93,7 @@ describe('Dataloader class', () => { const method = jest.fn(fakeNullPromise) const notifyChanges = jest.fn() const instance = new DataLoader({ - key: 'test', + key: 'test-null', method, notifyChanges, }) @@ -87,7 +106,7 @@ describe('Dataloader class', () => { const notifyChanges = jest.fn() const instance = new DataLoader({ - key: 'test', + key: 'test-undefined', method, notifyChanges, }) @@ -96,13 +115,13 @@ describe('Dataloader class', () => { expect(instance.getData()).toBe(undefined) }) - test('should create instance with cancel listener and error', async () => { + test('should create instance with error', async () => { const method = jest.fn(fakeErrorPromise) const notifyChanges = jest.fn() const onError = jest.fn() const instance = new DataLoader({ - key: 'test', + key: 'test-error', method, notifyChanges, }) @@ -110,4 +129,54 @@ describe('Dataloader class', () => { expect(notifyChanges).toBeCalledTimes(1) expect(onError).toBeCalledTimes(1) }) + + test('should create instance with cancel', async () => { + const method = jest.fn(fakeSuccessPromise) + const notifyChanges = jest.fn() + + const instance = new DataLoader({ + key: 'test-cancel', + method, + notifyChanges, + }) + instance.load().catch(undefined) + instance.cancel() + await waitForExpect(() => expect(instance.status).toBe(StatusEnum.IDLE)) + expect(notifyChanges).toBeCalledTimes(1) + }) + + test.only('should create instance with error and cancel', async () => { + const method = jest.fn(fakeErrorPromise) + const notifyChanges = jest.fn() + const onError = jest.fn() + + const instance = new DataLoader({ + key: 'test-error-cancel', + method, + notifyChanges, + }) + instance.load().catch(onError) + instance.cancel() + await waitForExpect(() => expect(instance.status).toBe(StatusEnum.IDLE)) + expect(notifyChanges).toBeCalledTimes(1) + expect(onError).toBeCalledTimes(0) + }) + + test('should launch multiple dataloader', async () => { + const method = jest.fn(fakeErrorPromise) + const notifyChanges = jest.fn() + + const instances = Array.from({ length: 5 }, (_, index) => index).map( + index => + new DataLoader({ + key: `test-${index}`, + method, + notifyChanges, + }), + ) + instances.forEach(instance => { + instance.load().catch(undefined) + }) + await waitForExpect(() => expect(method).toBeCalledTimes(5)) + }) }) diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx index 2495c907d..3b8eb6c8e 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx @@ -329,6 +329,129 @@ describe('useDataLoader', () => { expect(result.current.data).toBe(2) }) + test('should render correctly without pooling and needPolling', async () => { + const pollingProps = { + config: { + needPolling: () => true, + pollingInterval: undefined, + }, + key: 'test-needpolling-no-interval', + method: jest.fn( + () => + new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }), + ), + } as UseDataLoaderHookProps + + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: pollingProps, + wrapper, + }, + ) + expect(result.current.data).toBe(undefined) + expect(result.current.isPolling).toBe(false) + expect(result.current.isLoading).toBe(true) + expect(pollingProps.method).toBeCalledTimes(1) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(result.current.data).toBe(true) + expect(result.current.isSuccess).toBe(true) + }) + test('should render correctly with pooling and needPolling true', async () => { + const pollingProps = { + config: { + needPolling: true, + pollingInterval: PROMISE_TIMEOUT, + }, + key: 'test-needpolling-no-interval', + method: jest.fn( + () => + new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }), + ), + } as UseDataLoaderHookProps + + const { result } = renderHook( + 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) + expect(pollingProps.method).toBeCalledTimes(1) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(result.current.data).toBe(true) + expect(result.current.isSuccess).toBe(true) + }) + + test('should render correctly with pooling and needPolling false', async () => { + const pollingProps = { + config: { + needPolling: false, + pollingInterval: PROMISE_TIMEOUT, + }, + key: 'test-needpolling-no-interval', + method: jest.fn( + () => + new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }), + ), + } as UseDataLoaderHookProps + + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: pollingProps, + wrapper, + }, + ) + expect(result.current.data).toBe(undefined) + expect(result.current.isPolling).toBe(false) + expect(result.current.isLoading).toBe(true) + expect(pollingProps.method).toBeCalledTimes(1) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(result.current.data).toBe(true) + expect(result.current.isSuccess).toBe(true) + }) + + test('should render correctly with pooling and needPolling function false', async () => { + const pollingProps = { + config: { + needPolling: () => false, + pollingInterval: PROMISE_TIMEOUT, + }, + key: 'test-needpolling-no-interval', + method: jest.fn( + () => + new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }), + ), + } as UseDataLoaderHookProps + + const { result } = renderHook( + props => useDataLoader(props.key, props.method, props.config), + { + initialProps: pollingProps, + wrapper, + }, + ) + expect(result.current.data).toBe(undefined) + expect(result.current.isPolling).toBe(false) + expect(result.current.isLoading).toBe(true) + expect(pollingProps.method).toBeCalledTimes(1) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(result.current.data).toBe(true) + expect(result.current.isSuccess).toBe(true) + }) + test('should render correctly with enabled off', async () => { const { result } = renderHook( props => useDataLoader(props.key, props.method, props.config), diff --git a/packages/use-dataloader/src/dataloader.ts b/packages/use-dataloader/src/dataloader.ts index 33a6f01a2..43ae35aac 100644 --- a/packages/use-dataloader/src/dataloader.ts +++ b/packages/use-dataloader/src/dataloader.ts @@ -88,7 +88,7 @@ class DataLoader { return DataLoader.queue[this.key] as Promise } - public launch = async (): Promise => { + public launch = async (): Promise => { try { this.isCancelled = false this.loadCount += 1 @@ -117,7 +117,11 @@ class DataLoader { delete DataLoader.queue[this.key] this.notifyChanges() - throw error + if (!this.isCancelled) { + throw error + } + + return undefined } } @@ -129,6 +133,7 @@ class DataLoader { DataLoader.started -= 1 delete DataLoader.queue[this.key] this.isCancelled = true + this.status = StatusEnum.IDLE } public addObserver(observer: () => void) { From dde632695e5190ff3c6a752cbc223e634e97ceb2 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Fri, 20 May 2022 15:38:37 +0200 Subject: [PATCH 07/12] test: cento per cento --- .../src/__tests__/DataLoaderProvider.test.tsx | 9 +++++++-- .../use-dataloader/src/__tests__/dataloader.test.ts | 12 +++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx index 3433f5ebc..46d32bb2d 100644 --- a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx +++ b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx @@ -119,15 +119,20 @@ describe('DataLoaderProvider', () => { expect(reloads).toHaveProperty(TEST_KEY) const testReload = result.current.getReloads(TEST_KEY) expect(testReload).toBeDefined() + if (testReload) { + expect(await testReload()).toBeNull() + } else { + fail('It shoulded be defined') + } expect(result.current.getCachedData(TEST_KEY)).toBe(null) expect(result.current.getCachedData()).toStrictEqual({ test: null }) expect(result.current.getRequest(TEST_KEY)).toBeDefined() const unknownReload = result.current.getReloads('unknown') expect(unknownReload).toBeUndefined() await reloads.test() - expect(method).toBeCalledTimes(2) - await result.current.reloadAll() expect(method).toBeCalledTimes(3) + await result.current.reloadAll() + expect(method).toBeCalledTimes(4) result.current.clearCachedData(TEST_KEY) result.current.clearCachedData('unknown') expect(result.current.getCachedData(TEST_KEY)).toBeUndefined() diff --git a/packages/use-dataloader/src/__tests__/dataloader.test.ts b/packages/use-dataloader/src/__tests__/dataloader.test.ts index a716352db..5fafe0cb1 100644 --- a/packages/use-dataloader/src/__tests__/dataloader.test.ts +++ b/packages/use-dataloader/src/__tests__/dataloader.test.ts @@ -23,6 +23,11 @@ const fakeErrorPromise = () => setTimeout(() => reject(new Error('test')), PROMISE_TIMEOUT) }) +const fakeLongErrorPromise = () => + new Promise((_, reject) => { + setTimeout(() => reject(new Error('test')), 1000) + }) + describe('Dataloader class', () => { test('should create instance then load then destroy', async () => { const method = jest.fn(fakeSuccessPromise) @@ -145,8 +150,8 @@ describe('Dataloader class', () => { expect(notifyChanges).toBeCalledTimes(1) }) - test.only('should create instance with error and cancel', async () => { - const method = jest.fn(fakeErrorPromise) + test('should create instance with error and cancel', async () => { + const method = jest.fn(fakeLongErrorPromise) const notifyChanges = jest.fn() const onError = jest.fn() @@ -155,11 +160,12 @@ describe('Dataloader class', () => { method, notifyChanges, }) - instance.load().catch(onError) + const res = instance.load().catch(onError) instance.cancel() await waitForExpect(() => expect(instance.status).toBe(StatusEnum.IDLE)) expect(notifyChanges).toBeCalledTimes(1) expect(onError).toBeCalledTimes(0) + await waitForExpect(async () => expect(await res).toBeUndefined()) }) test('should launch multiple dataloader', async () => { From c76647c488de6b7970f8dd4f23a80f7007e1c5c2 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Fri, 20 May 2022 16:16:47 +0200 Subject: [PATCH 08/12] fix: first load with custom property --- packages/use-dataloader/src/dataloader.ts | 4 ++++ packages/use-dataloader/src/useDataLoader.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/use-dataloader/src/dataloader.ts b/packages/use-dataloader/src/dataloader.ts index 43ae35aac..b10b48980 100644 --- a/packages/use-dataloader/src/dataloader.ts +++ b/packages/use-dataloader/src/dataloader.ts @@ -37,6 +37,8 @@ class DataLoader { public loadCount = 0 + public isFirstLoading = true + public constructor(args: DataLoaderConstructorArgs) { this.key = args.key this.method = args.method @@ -102,6 +104,7 @@ class DataLoader { this.error = undefined } this.isCalled = false + this.isFirstLoading = false DataLoader.started -= 1 delete DataLoader.queue[this.key] this.notifyChanges() @@ -113,6 +116,7 @@ class DataLoader { this.error = error as ErrorType } this.isCalled = false + this.isFirstLoading = false DataLoader.started -= 1 delete DataLoader.queue[this.key] this.notifyChanges() diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index 0cc3fe987..dcfa5f38b 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -110,7 +110,7 @@ function useDataLoaderV2( }, [pollingInterval, needPolling, request, request.status, request.data]) return { - data: request.loadCount > 0 ? request.data : initialData, + data: !request.isFirstLoading ? request.data : initialData, error: request.error, isError, isIdle, From 71544d46cf536de07abcc85e8b63c9ecd6d015de Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Fri, 20 May 2022 16:25:16 +0200 Subject: [PATCH 09/12] fix: ispolling should be true if needPolling is a function when first load --- packages/use-dataloader/src/useDataLoader.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index dcfa5f38b..e9ae7cbe1 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -51,7 +51,8 @@ function useDataLoaderV2( const isPolling = !!( pollingInterval && - ((typeof needPolling === 'function' && needPolling(request.data)) || + ((typeof needPolling === 'function' && + (!request.data || needPolling(request.data))) || (typeof needPolling !== 'function' && needPolling)) ) From ed915ba6149f497d36a98fa53a859ee6d87fd607 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Fri, 20 May 2022 16:31:24 +0200 Subject: [PATCH 10/12] fix: ispolling should be true if needPolling is a function when first load --- packages/use-dataloader/src/__tests__/useDataLoader.test.tsx | 3 ++- packages/use-dataloader/src/useDataLoader.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx index 3b8eb6c8e..dbc01bfac 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx @@ -444,11 +444,12 @@ describe('useDataLoader', () => { }, ) expect(result.current.data).toBe(undefined) - expect(result.current.isPolling).toBe(false) + expect(result.current.isPolling).toBe(true) expect(result.current.isLoading).toBe(true) expect(pollingProps.method).toBeCalledTimes(1) await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(result.current.data).toBe(true) + expect(result.current.isPolling).toBe(false) expect(result.current.isSuccess).toBe(true) }) diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index e9ae7cbe1..cb73365f4 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -52,7 +52,7 @@ function useDataLoaderV2( const isPolling = !!( pollingInterval && ((typeof needPolling === 'function' && - (!request.data || needPolling(request.data))) || + (request.isFirstLoading || needPolling(request.data))) || (typeof needPolling !== 'function' && needPolling)) ) From b5f345f9b52abb81c663c1642b18b2d9f1d48449 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Mon, 23 May 2022 15:21:07 +0200 Subject: [PATCH 11/12] fix: feedbacks --- packages/use-dataloader/src/dataloader.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/use-dataloader/src/dataloader.ts b/packages/use-dataloader/src/dataloader.ts index b10b48980..113971b8e 100644 --- a/packages/use-dataloader/src/dataloader.ts +++ b/packages/use-dataloader/src/dataloader.ts @@ -13,9 +13,9 @@ class DataLoader { public static started = 0 - public static cachedData = {} as Record + public static cachedData: Record = {} - public static queue = {} as Record + public static queue: Record = {} public key: string @@ -69,7 +69,7 @@ class DataLoader { } else { DataLoader.queue[this.key] = new Promise(resolve => { setTimeout(resolve) - }).then(() => this.tryLaunch()) + }).then(this.tryLaunch) } return DataLoader.queue[this.key] as Promise From 6674167e528dd17bef733a371432a7ef37851dfe Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Mon, 23 May 2022 15:51:02 +0200 Subject: [PATCH 12/12] fix: rename hook --- packages/use-dataloader/src/useDataLoader.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index cb73365f4..edac41afa 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -4,7 +4,7 @@ import { StatusEnum } from './constants' import DataLoader from './dataloader' import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from './types' -function useDataLoaderV2( +function useDataLoader( fetchKey: string, method: () => PromiseType, { @@ -123,4 +123,4 @@ function useDataLoaderV2( } } -export default useDataLoaderV2 +export default useDataLoader