From c42285fd6242db4a3637b69890880336c3580c44 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Tue, 21 Sep 2021 14:26:01 +0200 Subject: [PATCH 1/2] feat: concurrent request handle --- .../use-dataloader/src/DataLoaderProvider.tsx | 196 +++++++++----- packages/use-dataloader/src/constants.ts | 7 +- packages/use-dataloader/src/dataloader.ts | 145 +++++++++++ packages/use-dataloader/src/reducer.ts | 43 --- packages/use-dataloader/src/types.ts | 53 ++++ packages/use-dataloader/src/useDataLoader.ts | 246 +++++------------- 6 files changed, 414 insertions(+), 276 deletions(-) create mode 100644 packages/use-dataloader/src/dataloader.ts delete mode 100644 packages/use-dataloader/src/reducer.ts create mode 100644 packages/use-dataloader/src/types.ts diff --git a/packages/use-dataloader/src/DataLoaderProvider.tsx b/packages/use-dataloader/src/DataLoaderProvider.tsx index d27c807ad..c7ccaa8bf 100644 --- a/packages/use-dataloader/src/DataLoaderProvider.tsx +++ b/packages/use-dataloader/src/DataLoaderProvider.tsx @@ -7,73 +7,139 @@ import React, { useContext, useMemo, useRef, + useState, } from 'react' +import DataLoader from './dataloader' +import { DataLoaderStatus, OnErrorFn, PromiseType } from './types' + +type RequestQueue = Record +type CachedData = Record +type Reloads = Record Promise> + +type UseDataLoaderInitializerArgs = { + enabled?: boolean + key: string + status?: DataLoaderStatus + method: () => PromiseType + pollingInterval?: number + keepPreviousData?: boolean +} interface Context { - addCachedData: (key: string, newData: unknown) => void; - addReload: (key: string, method: () => Promise) => void; - cacheKeyPrefix: string; + addCachedData: (key: string, newData: unknown) => void + addReload: (key: string, method: () => Promise) => void + addRequest: (key: string, args: UseDataLoaderInitializerArgs) => DataLoader + cacheKeyPrefix: string onError?: (error: Error) => void | Promise - clearAllCachedData: () => void; - clearAllReloads: () => void; - clearCachedData: (key?: string | undefined) => void; - clearReload: (key?: string | undefined) => void; - getCachedData: (key?: string | undefined) => unknown; - getReloads: (key?: string | undefined) => (() => Promise) | Reloads; - reload: (key?: string | undefined) => Promise; - reloadAll: () => Promise; + clearAllCachedData: () => void + clearAllReloads: () => void + clearCachedData: (key?: string) => void + clearReload: (key?: string) => void + getCachedData: (key?: string) => unknown | CachedData + getReloads: (key?: string) => (() => Promise) | Reloads + getRequest: (key: string) => DataLoader | undefined + reload: (key?: string) => Promise + reloadAll: () => Promise } -type CachedData = Record -type Reloads = Record Promise> - // @ts-expect-error we force the context to undefined, should be corrected with default values export const DataLoaderContext = createContext(undefined) -const DataLoaderProvider = ({ children, cacheKeyPrefix, onError }: { - children: ReactNode, cacheKeyPrefix: string, onError: (error: Error) => void | Promise +const DataLoaderProvider = ({ + children, + cacheKeyPrefix, + onError, +}: { + children: ReactNode + cacheKeyPrefix: string + onError: OnErrorFn }): ReactElement => { - const cachedData = useRef({}) + const [requestQueue, setRequestQueue] = useState({} as RequestQueue) + const [cachedData, setCachedDataPrivate] = useState({}) const reloads = useRef({}) - const setCachedData = useCallback((compute: CachedData | ((data: CachedData) => CachedData)) => { - if (typeof compute === 'function') { - cachedData.current = compute(cachedData.current) - } else { - cachedData.current = compute - } - }, []) + const computeKey = useCallback( + (key: string) => `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${key}`, + [cacheKeyPrefix], + ) - const setReloads = useCallback((compute: Reloads | ((data: Reloads) => Reloads)) => { - if (typeof compute === 'function') { - reloads.current = compute(reloads.current) - } else { - reloads.current = compute - } - }, []) + const setCachedData = useCallback( + (compute: CachedData | ((data: CachedData) => CachedData)) => { + if (typeof compute === 'function') { + setCachedDataPrivate(current => compute(current)) + } else { + setCachedDataPrivate(compute) + } + }, + [], + ) + + const setReloads = useCallback( + (compute: Reloads | ((data: Reloads) => Reloads)) => { + if (typeof compute === 'function') { + reloads.current = compute(reloads.current) + } else { + reloads.current = compute + } + }, + [], + ) const addCachedData = useCallback( (key: string, newData: unknown) => { if (key && typeof key === 'string' && newData) { setCachedData(actualCachedData => ({ ...actualCachedData, - [key]: newData, + [computeKey(key)]: newData, })) } }, - [setCachedData], + [setCachedData, computeKey], ) const addReload = useCallback( - (key: string, method: () => Promise) => { - if (key && typeof key === 'string' && method) { + (key: string, method: () => Promise) => { + if (key && method && typeof key === 'string') { setReloads(actualReloads => ({ ...actualReloads, - [key]: method, + [computeKey(key)]: method, + })) + } + }, + [setReloads, computeKey], + ) + + const addRequest = useCallback( + (key: string, args: UseDataLoaderInitializerArgs) => { + if (key && typeof key === 'string') { + const notifyChanges = (updatedRequest: DataLoader) => { + setRequestQueue(current => ({ + ...current, + [computeKey(updatedRequest.key)]: updatedRequest, + })) + } + const newRequest = new DataLoader({ ...args, notify: notifyChanges }) + newRequest.addOnSuccessListener(result => { + if (result !== undefined && result !== null) + addCachedData(key, result) + }) + setRequestQueue(current => ({ + ...current, + [computeKey(key)]: newRequest, })) + + addReload(key, newRequest.launch) + + return newRequest } + throw new Error('Key should be a string') }, - [setReloads], + [computeKey, addCachedData, addReload], + ) + + const getRequest = useCallback( + (key: string) => requestQueue[computeKey(key)], + [computeKey, requestQueue], ) const clearReload = useCallback( @@ -81,13 +147,13 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix, onError }: { if (key && typeof key === 'string') { setReloads(actualReloads => { const tmp = actualReloads - delete tmp[key] + delete tmp[computeKey(key)] return tmp }) } }, - [setReloads], + [setReloads, computeKey], ) const clearAllReloads = useCallback(() => { @@ -99,23 +165,27 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix, onError }: { if (key && typeof key === 'string') { setCachedData(actualCachedData => { const tmp = actualCachedData - delete tmp[key] + delete tmp[computeKey(key)] return tmp }) } }, - [setCachedData], + [setCachedData, computeKey], ) const clearAllCachedData = useCallback(() => { setCachedData({}) }, [setCachedData]) - const reload = useCallback(async (key?: string) => { - if (key && typeof key === 'string') { - await (reloads.current[key] && reloads.current[key]()) - } - }, []) + const reload = useCallback( + async (key?: string) => { + if (key && typeof key === 'string') { + await (reloads.current[computeKey(key)] && + reloads.current[computeKey(key)]()) + } + }, + [computeKey], + ) const reloadAll = useCallback(async () => { await Promise.all( @@ -123,26 +193,33 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix, onError }: { ) }, []) - const getCachedData = useCallback((key?: string) => { - if (key) { - return cachedData.current[key] || undefined - } + const getCachedData = useCallback( + (key?: string) => { + if (key) { + return cachedData[computeKey(key)] || undefined + } - return cachedData.current - }, []) + return cachedData + }, + [computeKey, cachedData], + ) - const getReloads = useCallback((key?: string) => { - if (key) { - return reloads.current[key] || undefined - } + const getReloads = useCallback( + (key?: string) => { + if (key) { + return reloads.current[computeKey(key)] || undefined + } - return reloads.current - }, []) + return reloads.current + }, + [computeKey], + ) const value = useMemo( () => ({ addCachedData, addReload, + addRequest, cacheKeyPrefix, clearAllCachedData, clearAllReloads, @@ -150,6 +227,7 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix, onError }: { clearReload, getCachedData, getReloads, + getRequest, onError, reload, reloadAll, @@ -157,12 +235,14 @@ const DataLoaderProvider = ({ children, cacheKeyPrefix, onError }: { [ addCachedData, addReload, + addRequest, cacheKeyPrefix, clearAllCachedData, clearAllReloads, clearCachedData, clearReload, getCachedData, + getRequest, getReloads, onError, reload, diff --git a/packages/use-dataloader/src/constants.ts b/packages/use-dataloader/src/constants.ts index 3ddf7b626..69f9ebbc0 100644 --- a/packages/use-dataloader/src/constants.ts +++ b/packages/use-dataloader/src/constants.ts @@ -1,4 +1,9 @@ -export const StatusEnum = { +import { DataLoaderStatus } from './types' + +export const StatusEnum: Record< + 'ERROR' | 'IDLE' | 'LOADING' | 'SUCCESS', + DataLoaderStatus +> = { ERROR: 'error', IDLE: 'idle', LOADING: 'loading', diff --git a/packages/use-dataloader/src/dataloader.ts b/packages/use-dataloader/src/dataloader.ts new file mode 100644 index 000000000..477a233c3 --- /dev/null +++ b/packages/use-dataloader/src/dataloader.ts @@ -0,0 +1,145 @@ +/* eslint-disable no-void */ +import { StatusEnum } from './constants' +import { + DataLoaderStatus, + OnCancelFn, + OnErrorFn, + OnSuccessFn, + PromiseType, +} from './types' + +export type DataLoaderConstructorArgs = { + enabled?: boolean + key: string + method: () => PromiseType + pollingInterval?: number + keepPreviousData?: boolean + notify: (updatedRequest: DataLoader) => void +} + +class DataLoader { + public key: string + + public status: DataLoaderStatus + + public pollingInterval?: number + + private notify: (updatedRequest: DataLoader) => void + + public method: () => PromiseType + + private cancelMethod?: () => void + + public keepPreviousData?: boolean + + private errorListeners: Array = [] + + private successListeners: Array> = [] + + private cancelListeners: Array = [] + + public error?: Error + + public timeout?: number + + public constructor(args: DataLoaderConstructorArgs) { + this.key = args.key + this.status = args.enabled ? StatusEnum.LOADING : StatusEnum.IDLE + this.method = args.method + this.pollingInterval = args?.pollingInterval + this.keepPreviousData = args?.keepPreviousData + this.notify = args.notify + } + + public launch = async (): Promise => { + try { + if (this.timeout) { + // Prevent multiple call at the same time + clearTimeout(this.timeout) + } + if (this.status !== StatusEnum.LOADING) { + this.status = StatusEnum.LOADING + this.notify(this) + } + const promise = this.method() + this.cancelMethod = promise.cancel + const result = await promise.then(res => res) + + this.status = StatusEnum.SUCCESS + this.error = undefined + this.notify(this) + if (this.successListeners.length) { + this.successListeners.forEach(listener => void listener?.(result)) + } + } catch (err) { + this.status = StatusEnum.ERROR + this.error = err as Error + this.notify(this) + if (this.errorListeners?.length) { + this.errorListeners.forEach( + listener => void listener?.(this.error as Error), + ) + } + } + if (this.pollingInterval) { + this.timeout = setTimeout( + () => void this.launch(), + this.pollingInterval, + ) as unknown as number + } + } + + public cancel = (): void => { + this.cancelMethod?.() + if (this.cancelListeners.length) { + this.cancelListeners.forEach(listener => void 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) + } + } + + public addOnCancelListener(fn: OnCancelFn): void { + if (!this.cancelListeners.includes(fn)) { + this.cancelListeners.push(fn) + } + } + + public removeOnSuccessListener(fn: OnSuccessFn): void { + const index = this.successListeners.indexOf(fn) + if (index > -1) { + this.successListeners.splice(index, 1) + } + } + + public removeOnErrorListener(fn: OnErrorFn): void { + const index = this.errorListeners.indexOf(fn) + if (index > -1) { + this.errorListeners.splice(index, 1) + } + } + + public removeOnCancelListener(fn: OnCancelFn): void { + const index = this.cancelListeners.indexOf(fn) + if (index > -1) { + this.cancelListeners.splice(index, 1) + } + } + + public destroy(): void { + if (this.timeout) { + clearTimeout(this.timeout) + } + } +} + +export default DataLoader diff --git a/packages/use-dataloader/src/reducer.ts b/packages/use-dataloader/src/reducer.ts deleted file mode 100644 index bb4670db4..000000000 --- a/packages/use-dataloader/src/reducer.ts +++ /dev/null @@ -1,43 +0,0 @@ -/* eslint-disable consistent-return */ -/* eslint-disable default-case */ -import { ActionEnum, StatusEnum } from './constants' - -interface Action { - type: typeof ActionEnum[keyof typeof ActionEnum]; - error?: Error; -} - -interface State { - error?: Error; - status: typeof StatusEnum[keyof typeof StatusEnum]; - [key: string]: unknown -} - -// @ts-expect-error we have no default return -export default (state: State, action: Action): State => { - switch (action.type) { - case ActionEnum.ON_LOADING: - return { - ...state, - error: undefined, - status: StatusEnum.LOADING, - } - case ActionEnum.ON_SUCCESS: - return { - ...state, - error: undefined, - status: StatusEnum.SUCCESS, - } - case ActionEnum.RESET: - return { - error: undefined, - status: StatusEnum.IDLE, - } - case ActionEnum.ON_ERROR: - return { - ...state, - error: action.error, - status: StatusEnum.ERROR, - } - } -} diff --git a/packages/use-dataloader/src/types.ts b/packages/use-dataloader/src/types.ts new file mode 100644 index 000000000..de50deb37 --- /dev/null +++ b/packages/use-dataloader/src/types.ts @@ -0,0 +1,53 @@ +export class PromiseType extends Promise { + public cancel?: () => void +} + +export type DataLoaderStatus = 'error' | 'idle' | 'loading' | 'success' + +export type OnErrorFn = ((err: Error) => void | Promise) | undefined +export type OnSuccessFn = + | ((result: T) => void | Promise) + | undefined +export type OnCancelFn = (() => void | Promise) | undefined + +/** + * @typedef {Object} UseDataLoaderConfig + * @property {Function} [onSuccess] callback when a request success + * @property {Function} [onError] callback when a error is occured, this will override the onError specified on the Provider if any + * @property {*} [initialData] initial data if no one is present in the cache before the request + * @property {number} [pollingInterval] relaunch the request after the last success + * @property {boolean} [enabled=true] launch request automatically (default true) + * @property {boolean} [keepPreviousData=true] do we need to keep the previous data after reload (default true) + */ +export interface UseDataLoaderConfig { + enabled?: boolean + initialData?: T + keepPreviousData?: boolean + onError?: OnErrorFn + onSuccess?: OnSuccessFn + pollingInterval?: number +} + +/** + * @typedef {Object} UseDataLoaderResult + * @property {boolean} isIdle true if the hook in initial state + * @property {boolean} isLoading true if the request is launched + * @property {boolean} isSuccess true if the request success + * @property {boolean} isError true if the request throw an error + * @property {boolean} isPolling true if the request if enabled is true, pollingInterval is defined and the status is isLoading or isSuccess + * @property {*} previousData if keepPreviousData is true it return the last data fetched + * @property {*} data initialData if no data is fetched or not present in the cache otherwise return the data fetched + * @property {string} error the error occured during the request + * @property {Function} reload reload the data + */ +export interface UseDataLoaderResult { + data?: T + error?: Error + isError: boolean + isIdle: boolean + isLoading: boolean + isPolling: boolean + isSuccess: boolean + previousData?: T + reload: () => Promise +} diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index 3a2a4bc18..6f5b7f8a5 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -1,71 +1,11 @@ -import { - useCallback, - useEffect, - useLayoutEffect, - useMemo, - useReducer, - useRef, -} from 'react' +import { useEffect, useLayoutEffect, useMemo, useRef } from 'react' import { useDataLoaderContext } from './DataLoaderProvider' -import { ActionEnum, StatusEnum } from './constants' -import reducer from './reducer' - -const Actions = { - createOnError: (error: Error) => ({ error, type: ActionEnum.ON_ERROR }), - createOnLoading: () => ({ type: ActionEnum.ON_LOADING }), - createOnSuccess: () => ({ type: ActionEnum.ON_SUCCESS }), - createReset: () => ({ type: ActionEnum.RESET }), -} - -export class PromiseType extends Promise { - public cancel?: () => void -} - -/** - * @typedef {Object} UseDataLoaderConfig - * @property {Function} [onSuccess] callback when a request success - * @property {Function} [onError] callback when a error is occured, this will override the onError specified on the Provider if any - * @property {*} [initialData] initial data if no one is present in the cache before the request - * @property {number} [pollingInterval] relaunch the request after the last success - * @property {boolean} [enabled=true] launch request automatically (default true) - * @property {boolean} [keepPreviousData=true] do we need to keep the previous data after reload (default true) - */ -export interface UseDataLoaderConfig { - enabled?: boolean - initialData?: T - keepPreviousData?: boolean - onError?: (err: Error) => void | Promise - onSuccess?: (data: T) => void | Promise - pollingInterval?: number -} - -/** - * @typedef {Object} UseDataLoaderResult - * @property {boolean} isIdle true if the hook in initial state - * @property {boolean} isLoading true if the request is launched - * @property {boolean} isSuccess true if the request success - * @property {boolean} isError true if the request throw an error - * @property {boolean} isPolling true if the request if enabled is true, pollingInterval is defined and the status is isLoading or isSuccess - * @property {*} previousData if keepPreviousData is true it return the last data fetched - * @property {*} data initialData if no data is fetched or not present in the cache otherwise return the data fetched - * @property {string} error the error occured during the request - * @property {Function} reload reload the data - */ -export interface UseDataLoaderResult { - data?: T - error?: Error - isError: boolean - isIdle: boolean - isLoading: boolean - isPolling: boolean - isSuccess: boolean - previousData?: T - reload: () => Promise -} +import { StatusEnum } from './constants' +import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from './types' /** * @param {string} key key to save the data fetched in a local cache - * @param {() => Promise} method a method that return a promise + * @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 */ @@ -82,155 +22,113 @@ const useDataLoader = ( }: UseDataLoaderConfig = {}, ): UseDataLoaderResult => { const { - addReload, - clearReload, + addRequest, getCachedData, - addCachedData, - cacheKeyPrefix, + getRequest, onError: onErrorProvider, } = useDataLoaderContext() - const [{ status, error }, dispatch] = useReducer(reducer, { - error: undefined, - status: StatusEnum.IDLE, - }) - - const addReloadRef = useRef(addReload) - const clearReloadRef = useRef(clearReload) - const cancelMethodRef = useRef<(() => void) | undefined>(undefined) - const isMountedRef = useRef(false) - const isFetchingRef = useRef(false) - const key = useMemo(() => { - if (!fetchKey || typeof fetchKey !== 'string') { - return fetchKey - } - - return `${cacheKeyPrefix ? `${cacheKeyPrefix}-` : ''}${fetchKey}` - }, [cacheKeyPrefix, fetchKey]) - - const previousDataRef = useRef() - - const isLoading = useMemo(() => status === StatusEnum.LOADING, [status]) - const isIdle = useMemo(() => status === StatusEnum.IDLE, [status]) - const isSuccess = useMemo(() => status === StatusEnum.SUCCESS, [status]) - const isError = useMemo(() => status === StatusEnum.ERROR, [status]) - const isPolling = useMemo( - () => !!(enabled && pollingInterval && (isSuccess || isLoading)), - [isSuccess, isLoading, enabled, pollingInterval], + const data = useMemo( + () => (getCachedData(fetchKey) || initialData) as T, + [getCachedData, fetchKey, initialData], ) - const handleRequest = useCallback( - async cacheKey => { - try { - dispatch(Actions.createOnLoading()) - const promise = method() - cancelMethodRef.current = promise.cancel - const result = await promise.then(res => res) - - if (isMountedRef.current) { - if (keepPreviousData) { - previousDataRef.current = getCachedData(cacheKey) as T - } - if (result !== undefined && result !== null && cacheKey) - addCachedData(cacheKey, result) - - dispatch(Actions.createOnSuccess()) - - await onSuccess?.(result) - } - } catch (err) { - if (isMountedRef.current) { - dispatch(Actions.createOnError(err as Error)) - await (onError ?? onErrorProvider)?.(err as Error) - } - } - }, - [ - addCachedData, - getCachedData, - keepPreviousData, - method, - onError, - onErrorProvider, - onSuccess, - ], + const request = useMemo( + () => + getRequest(fetchKey) ?? + addRequest(fetchKey, { + key: fetchKey, + method, + pollingInterval, + }), + [addRequest, fetchKey, getRequest, method, pollingInterval], ) - const handleRequestRef = useRef(handleRequest) - useEffect(() => { - let handler: ReturnType - - async function fetch() { - if (enabled) { - if (isIdle) { - await handleRequestRef.current(key) - } - if (pollingInterval && (isSuccess || isError)) { - handler = setTimeout( - // eslint-disable-next-line @typescript-eslint/no-misused-promises - () => handleRequestRef.current(key), - pollingInterval, - ) - } - } - } - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - fetch() - - return () => { - if (handler) clearTimeout(handler) + if (enabled && request.status === StatusEnum.IDLE) { + // eslint-disable-next-line no-void + void request.launch() } - }, [key, pollingInterval, isIdle, isSuccess, isError, enabled]) + }, [request, enabled]) - useLayoutEffect(() => { - dispatch(Actions.createReset()) - if (key && typeof key === 'string') { - addReloadRef.current?.(key, () => handleRequestRef.current(key)) + useEffect(() => { + if (request.method !== method) { + request.method = method } + request.addOnErrorListener(onError ?? onErrorProvider) + request.addOnSuccessListener(onSuccess) return () => { - if (key && typeof key === 'string') { - clearReloadRef.current?.(key) - } + request.removeOnErrorListener(onError ?? onErrorProvider) + request.removeOnSuccessListener(onSuccess) } - }, [enabled, key]) + }, [onSuccess, onError, onErrorProvider, method, request]) - useLayoutEffect(() => { - clearReloadRef.current = clearReload - addReloadRef.current = addReload - }, [clearReload, addReload]) + const cancelMethodRef = useRef<(() => void) | undefined>(request?.cancel) + const isMountedRef = useRef(false) + const isFetchingRef = useRef(false) - useLayoutEffect(() => { - handleRequestRef.current = handleRequest - }, [handleRequest]) + const previousDataRef = useRef() + + const isLoading = useMemo( + () => request.status === StatusEnum.LOADING, + [request.status], + ) + 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], + ) useEffect(() => { isFetchingRef.current = isLoading || isPolling }, [isLoading, isPolling]) + useLayoutEffect(() => { + cancelMethodRef.current = request?.cancel + }, [request?.cancel]) + useEffect(() => { isMountedRef.current = true return () => { isMountedRef.current = false if (isFetchingRef.current && cancelMethodRef.current) { - cancelMethodRef.current?.() + cancelMethodRef.current() } } }, []) + useEffect( + () => () => { + if (data !== previousDataRef.current) { + previousDataRef.current = data + } + }, + [data, keepPreviousData], + ) + return { - data: (getCachedData(key) || initialData) as T, - error, + data: (getCachedData(fetchKey) || initialData) as T, + error: request?.error, isError, isIdle, isLoading, isPolling, isSuccess, previousData: previousDataRef.current, - reload: () => handleRequestRef.current(key), + reload: request.launch, } } From 74be2e8a9f7f0e76337da5090f75557833fea194 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Tue, 21 Sep 2021 14:26:24 +0200 Subject: [PATCH 2/2] test: update testing --- .../use-dataloader/src/DataLoaderProvider.tsx | 44 +-- .../src/__tests__/DataLoaderProvider.test.tsx | 292 ++++++++++++++++++ .../src/__tests__/DataLoaderProvider.tsx | 155 ---------- .../src/__tests__/dataloader.test.ts | 125 ++++++++ ...eDataLoader.tsx => useDataLoader.test.tsx} | 50 ++- packages/use-dataloader/src/constants.ts | 29 +- packages/use-dataloader/src/dataloader.ts | 34 +- packages/use-dataloader/src/types.ts | 2 - 8 files changed, 488 insertions(+), 243 deletions(-) create mode 100644 packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx delete mode 100644 packages/use-dataloader/src/__tests__/DataLoaderProvider.tsx create mode 100644 packages/use-dataloader/src/__tests__/dataloader.test.ts rename packages/use-dataloader/src/__tests__/{useDataLoader.tsx => useDataLoader.test.tsx} (95%) diff --git a/packages/use-dataloader/src/DataLoaderProvider.tsx b/packages/use-dataloader/src/DataLoaderProvider.tsx index c7ccaa8bf..c1425511f 100644 --- a/packages/use-dataloader/src/DataLoaderProvider.tsx +++ b/packages/use-dataloader/src/DataLoaderProvider.tsx @@ -9,8 +9,9 @@ import React, { useRef, useState, } from 'react' +import { KEY_IS_NOT_STRING_ERROR, StatusEnum } from './constants' import DataLoader from './dataloader' -import { DataLoaderStatus, OnErrorFn, PromiseType } from './types' +import { OnErrorFn, PromiseType } from './types' type RequestQueue = Record type CachedData = Record @@ -19,7 +20,7 @@ type Reloads = Record Promise> type UseDataLoaderInitializerArgs = { enabled?: boolean key: string - status?: DataLoaderStatus + status?: StatusEnum method: () => PromiseType pollingInterval?: number keepPreviousData?: boolean @@ -87,11 +88,13 @@ const DataLoaderProvider = ({ const addCachedData = useCallback( (key: string, newData: unknown) => { - if (key && typeof key === 'string' && newData) { - setCachedData(actualCachedData => ({ - ...actualCachedData, - [computeKey(key)]: newData, - })) + if (newData) { + if (key && typeof key === 'string') { + setCachedData(actualCachedData => ({ + ...actualCachedData, + [computeKey(key)]: newData, + })) + } else throw new Error(KEY_IS_NOT_STRING_ERROR) } }, [setCachedData, computeKey], @@ -99,11 +102,13 @@ const DataLoaderProvider = ({ const addReload = useCallback( (key: string, method: () => Promise) => { - if (key && method && typeof key === 'string') { - setReloads(actualReloads => ({ - ...actualReloads, - [computeKey(key)]: method, - })) + if (method) { + if (key && typeof key === 'string') { + setReloads(actualReloads => ({ + ...actualReloads, + [computeKey(key)]: method, + })) + } else throw new Error(KEY_IS_NOT_STRING_ERROR) } }, [setReloads, computeKey], @@ -132,7 +137,7 @@ const DataLoaderProvider = ({ return newRequest } - throw new Error('Key should be a string') + throw new Error(KEY_IS_NOT_STRING_ERROR) }, [computeKey, addCachedData, addReload], ) @@ -151,7 +156,7 @@ const DataLoaderProvider = ({ return tmp }) - } + } else throw new Error(KEY_IS_NOT_STRING_ERROR) }, [setReloads, computeKey], ) @@ -169,7 +174,7 @@ const DataLoaderProvider = ({ return tmp }) - } + } else throw new Error(KEY_IS_NOT_STRING_ERROR) }, [setCachedData, computeKey], ) @@ -180,9 +185,8 @@ const DataLoaderProvider = ({ const reload = useCallback( async (key?: string) => { if (key && typeof key === 'string') { - await (reloads.current[computeKey(key)] && - reloads.current[computeKey(key)]()) - } + await reloads.current[computeKey(key)]?.() + } else throw new Error(KEY_IS_NOT_STRING_ERROR) }, [computeKey], ) @@ -196,7 +200,7 @@ const DataLoaderProvider = ({ const getCachedData = useCallback( (key?: string) => { if (key) { - return cachedData[computeKey(key)] || undefined + return cachedData[computeKey(key)] } return cachedData @@ -207,7 +211,7 @@ const DataLoaderProvider = ({ const getReloads = useCallback( (key?: string) => { if (key) { - return reloads.current[computeKey(key)] || undefined + return reloads.current[computeKey(key)] } return reloads.current diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx new file mode 100644 index 000000000..68ce02fd4 --- /dev/null +++ b/packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx @@ -0,0 +1,292 @@ +import { render, screen } from '@testing-library/react' +import { act, renderHook } from '@testing-library/react-hooks' +import React, { ReactNode } from 'react' +import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' +import { KEY_IS_NOT_STRING_ERROR, StatusEnum } from '../constants' + +const wrapper = ({ children }: { children: ReactNode }) => ( + {children} +) + +const wrapperWithCacheKey = ({ children }: { children?: React.ReactNode }) => ( + {children} +) + +describe('DataLoaderProvider', () => { + test('should render correctly', () => { + render(Test) + expect(screen.getByText('Test')).toBeTruthy() + }) + test('should add cached data', () => { + const { result } = renderHook(useDataLoaderContext, { wrapper }) + expect(result.current.getCachedData()).toStrictEqual({}) + + act(() => { + result.current.addCachedData('test', undefined) + result.current.addCachedData('test', 'test') + }) + + try { + // @ts-expect-error used because we test with bad key + result.current.addCachedData(3, 'testWrong') + throw new Error('It should throw an error before') + } catch (err) { + expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } + + expect( + Object.values(result.current.getCachedData() as Record) + .length, + ).toBe(1) + expect( + (result.current.getCachedData() as Record).test, + ).toBe('test') + }) + + test('should delete cached data', () => { + const { result } = renderHook(useDataLoaderContext, { wrapper }) + + act(() => { + result.current.addCachedData('testA', 'testA') + result.current.addCachedData('testB', 'testB') + result.current.addCachedData('testC', 'testC') + result.current.addCachedData('testD', 'testD') + result.current.addCachedData('testE', 'testE') + }) + expect(result.current.getCachedData('testA')).toBe('testA') + + try { + result.current.clearCachedData() + throw new Error('It should throw an error before') + } catch (err) { + expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } + + act(() => { + result.current.clearCachedData('testA') + }) + expect( + Object.values(result.current.getCachedData() as Record) + .length, + ).toBe(4) + expect( + (result.current.getCachedData() as Record).testA, + ).toBe(undefined) + + act(() => { + result.current.clearAllCachedData() + }) + expect( + Object.values(result.current.getCachedData() as Record) + .length, + ).toBe(0) + expect(result.current.getCachedData()).toStrictEqual({}) + }) + + test('should get cached data', () => { + const { result } = renderHook(useDataLoaderContext, { wrapper }) + expect(result.current.getCachedData()).toStrictEqual({}) + + act(() => { + result.current.addCachedData('test', 'test') + }) + + expect( + Object.values(result.current.getCachedData() as Record) + .length, + ).toBe(1) + expect(result.current.getCachedData('test')).toBe('test') + expect(result.current.getCachedData()).toStrictEqual({ test: 'test' }) + expect(result.current.getCachedData('scaleway')).toBe(undefined) + }) + + test('should add reload', async () => { + const fn = () => new Promise(resolve => resolve(true)) + const { result } = renderHook(useDataLoaderContext, { wrapper }) + expect(result.current.getCachedData()).toStrictEqual({}) + + act(() => { + // @ts-expect-error used because we test with bad method + result.current.addReload('test', undefined) + result.current.addReload('test', fn) + }) + + try { + // @ts-expect-error used because we test with bad key + result.current.addReload(1, () => new Promise(resolve => resolve(true))) + throw new Error('It should throw an error before') + } catch (err) { + expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } + + expect(Object.values(result.current.getReloads()).length).toBe(1) + expect(result.current.getReloads('test')).toBe(fn) + expect(result.current.getReloads('testWrong')).toBe(undefined) + expect( + await ( + result.current.getReloads() as unknown as { + test: () => Promise + } + ).test(), + ).toBe(true) + }) + + test('should clear reload', () => { + const { result } = renderHook(useDataLoaderContext, { wrapper }) + expect(result.current.getCachedData()).toStrictEqual({}) + + act(() => { + result.current.addReload( + 'testA', + () => new Promise(resolve => resolve('testA')), + ) + result.current.addReload( + 'testB', + () => new Promise(resolve => resolve('testB')), + ) + result.current.addReload( + 'testC', + () => new Promise(resolve => resolve('testC')), + ) + result.current.addReload( + 'testD', + () => new Promise(resolve => resolve('testD')), + ) + }) + + expect(Object.values(result.current.getReloads()).length).toBe(4) + + act(() => { + result.current.clearReload('testA') + }) + expect(Object.values(result.current.getReloads()).length).toBe(3) + expect((result.current.getReloads() as Record).testA).toBe( + undefined, + ) + + act(() => { + result.current.clearAllReloads() + }) + expect(Object.values(result.current.getReloads()).length).toBe(0) + }) + + test('should trigger reload', async () => { + const reloadFn = jest.fn() + const { result } = renderHook(useDataLoaderContext, { wrapper }) + expect(result.current.getCachedData()).toStrictEqual({}) + + act(() => { + result.current.addReload('testA', reloadFn) + result.current.addReload('testB', reloadFn) + }) + + expect(Object.values(result.current.getReloads()).length).toBe(2) + expect( + Object.values( + result.current.getReloads('testA') as () => Promise, + ), + ).toBeDefined() + try { + await result.current.reload() + throw new Error('It should throw an error before') + } catch (err) { + expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } + await result.current.reload('testA') + expect(reloadFn).toBeCalledTimes(1) + + try { + result.current.clearReload() + throw new Error('It should throw an error before') + } catch (err) { + expect((err as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } + expect(Object.values(result.current.getReloads()).length).toBe(2) + + const multipleReloads = jest.fn() + + act(() => { + result.current.clearAllReloads() + result.current.addReload('testA', multipleReloads) + result.current.addReload('testB', multipleReloads) + result.current.addReload('testC', multipleReloads) + result.current.addReload('testD', multipleReloads) + }) + + await result.current.reloadAll() + expect(multipleReloads).toBeCalledTimes(4) + }) + + test('should add request', async () => { + const method = jest.fn(() => Promise.resolve(true)) + const { result, waitForNextUpdate } = renderHook(useDataLoaderContext, { + wrapper, + }) + expect(result.current.getRequest('test')).toBeUndefined() + + act(() => { + result.current.addRequest('test', { + key: 'test', + method, + }) + }) + + expect(Object.values(result.current.getReloads()).length).toBe(1) + expect(result.current.getRequest('test')).toBeDefined() + expect(result.current.getRequest('test')?.status).toBe(StatusEnum.IDLE) + act(() => { + result.current.getRequest('test')?.launch() + }) + expect(method).toBeCalledTimes(1) + await waitForNextUpdate() + expect(result.current.getCachedData('test')).toBe(true) + }) + + test('should add request with result is null thus no data is cached', async () => { + const method = jest.fn(() => Promise.resolve(null)) + const { result, waitForNextUpdate } = renderHook(useDataLoaderContext, { + wrapper, + }) + act(() => { + // eslint-disable-next-line no-void + void result.current + .addRequest('test', { + enabled: true, + key: 'test', + method, + }) + .launch() + }) + await waitForNextUpdate() + expect(method).toBeCalledTimes(1) + expect(result.current.getCachedData('test')).toBe(undefined) + }) + + test('should add request with bad key', () => { + const method = jest.fn(() => Promise.resolve(true)) + const { result } = renderHook(useDataLoaderContext, { + wrapper, + }) + try { + act(() => { + // @ts-expect-error used because we test with bad key + result.current.addRequest(3, { + key: 'test', + method, + }) + }) + } catch (e) { + expect((e as Error).message).toBe(KEY_IS_NOT_STRING_ERROR) + } + }) + + test('should add cached data with prefix', () => { + const { result } = renderHook(useDataLoaderContext, { + wrapper: wrapperWithCacheKey, + }) + act(() => { + result.current.addCachedData('test', 'test') + }) + expect(result.current.getCachedData('test')).toBeDefined() + }) +}) diff --git a/packages/use-dataloader/src/__tests__/DataLoaderProvider.tsx b/packages/use-dataloader/src/__tests__/DataLoaderProvider.tsx deleted file mode 100644 index 144b28fd3..000000000 --- a/packages/use-dataloader/src/__tests__/DataLoaderProvider.tsx +++ /dev/null @@ -1,155 +0,0 @@ -import { render, screen } from '@testing-library/react' -import { act, renderHook } from '@testing-library/react-hooks' -import React from 'react' -import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' - -const wrapper = ({ children }) => ( - {children} -) - -describe('DataLoaderProvider', () => { - test('should render correctly', () => { - render(Test) - expect(screen.getByText('Test')).toBeTruthy() - }) - test('should add cached data', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addCachedData('test', 'test') - result.current.addCachedData(3, 'testWrong') - }) - - expect(Object.values(result.current.getCachedData()).length).toBe(1) - expect(result.current.getCachedData().test).toBe('test') - }) - - test('should delete cached data', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - - act(() => { - result.current.addCachedData('testA', 'testA') - result.current.addCachedData('testB', 'testB') - result.current.addCachedData('testC', 'testC') - result.current.addCachedData('testD', 'testD') - result.current.addCachedData('testE', 'testE') - }) - expect(result.current.getCachedData('testA')).toBe('testA') - - act(() => { - result.current.clearCachedData() - result.current.clearCachedData('testA') - }) - expect(Object.values(result.current.getCachedData()).length).toBe(4) - expect(result.current.getCachedData().testA).toBe(undefined) - - act(() => { - result.current.clearAllCachedData() - }) - expect(Object.values(result.current.getCachedData()).length).toBe(0) - expect(result.current.getCachedData()).toStrictEqual({}) - }) - - test('should get cached data', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addCachedData('test', 'test') - }) - - expect(Object.values(result.current.getCachedData()).length).toBe(1) - expect(result.current.getCachedData('test')).toBe('test') - expect(result.current.getCachedData()).toStrictEqual({ test: 'test' }) - expect(result.current.getCachedData('scaleway')).toBe(undefined) - }) - - test('should add reload', async () => { - const fn = () => new Promise(resolve => resolve(true)) - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addReload('test', fn) - result.current.addReload(1, () => new Promise(resolve => resolve(true))) - }) - - expect(Object.values(result.current.getReloads()).length).toBe(1) - expect(result.current.getReloads('test')).toBe(fn) - expect(result.current.getReloads('testWrong')).toBe(undefined) - expect(await (result.current.getReloads() as { test: () => Promise }).test()).toBe(true) - }) - - test('should clear reload', () => { - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addReload( - 'testA', - () => new Promise(resolve => resolve('testA')), - ) - result.current.addReload( - 'testB', - () => new Promise(resolve => resolve('testB')), - ) - result.current.addReload( - 'testC', - () => new Promise(resolve => resolve('testC')), - ) - result.current.addReload( - 'testD', - () => new Promise(resolve => resolve('testD')), - ) - }) - - expect(Object.values(result.current.getReloads()).length).toBe(4) - - act(() => { - result.current.clearReload('testA') - }) - expect(Object.values(result.current.getReloads()).length).toBe(3) - expect(result.current.getReloads().testA).toBe(undefined) - - act(() => { - result.current.clearAllReloads() - }) - expect(Object.values(result.current.getReloads()).length).toBe(0) - }) - - test('should trigger reload', async () => { - const reloadFn = jest.fn() - const { result } = renderHook(useDataLoaderContext, { wrapper }) - expect(result.current.getCachedData()).toStrictEqual({}) - - act(() => { - result.current.addReload('testA', reloadFn) - result.current.addReload('testB', reloadFn) - }) - - expect(Object.values(result.current.getReloads()).length).toBe(2) - expect(Object.values(result.current.getReloads('testA'))).toBeDefined() - await result.current.reload() - await result.current.reload('testA') - expect(reloadFn).toBeCalledTimes(1) - - act(() => { - result.current.clearReload() - }) - expect(Object.values(result.current.getReloads()).length).toBe(2) - - const multipleReloads = jest.fn() - - act(() => { - result.current.clearAllReloads() - result.current.addReload('testA', multipleReloads) - result.current.addReload('testB', multipleReloads) - result.current.addReload('testC', multipleReloads) - result.current.addReload('testD', multipleReloads) - }) - - await result.current.reloadAll() - expect(multipleReloads).toBeCalledTimes(4) - }) -}) diff --git a/packages/use-dataloader/src/__tests__/dataloader.test.ts b/packages/use-dataloader/src/__tests__/dataloader.test.ts new file mode 100644 index 000000000..ee6bdffec --- /dev/null +++ b/packages/use-dataloader/src/__tests__/dataloader.test.ts @@ -0,0 +1,125 @@ +import { StatusEnum } from '../constants' +import DataLoader from '../dataloader' + +const PROMISE_TIMEOUT = 100 + +const fakeSuccessPromise = () => + new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }) + +const fakeErrorPromise = () => + new Promise((_, reject) => { + setTimeout(() => reject(new Error('test')), PROMISE_TIMEOUT) + }) + +describe('Dataloader class', () => { + test('should create instance not enabled then launch then destroy', async () => { + const notify = jest.fn() + const method = jest.fn(fakeSuccessPromise) + const instance = new DataLoader({ + key: 'test', + method, + notify, + }) + expect(instance.status).toBe(StatusEnum.IDLE) + expect(notify).toBeCalledTimes(0) + expect(method).toBeCalledTimes(0) + await instance.launch() + expect(method).toBeCalledTimes(1) + expect(notify).toBeCalledTimes(2) + instance.destroy() + }) + + test('should create instance enabled then launch', async () => { + const notify = jest.fn() + const method = jest.fn() + const instance = new DataLoader({ + enabled: true, + key: 'test', + method, + notify, + }) + expect(instance.status).toBe(StatusEnum.LOADING) + expect(notify).toBeCalledTimes(0) + expect(method).toBeCalledTimes(0) + await instance.launch() + // This does nothing because no cancel listener is set + await instance.cancel() + expect(method).toBeCalledTimes(1) + expect(notify).toBeCalledTimes(1) + }) + + test('should create instance with cancel listener', async () => { + const notify = jest.fn() + const method = jest.fn() + const onCancel = jest.fn() + const instance = new DataLoader({ + key: 'test', + method, + notify, + }) + instance.addOnCancelListener(onCancel) + instance.addOnCancelListener(onCancel) + // eslint-disable-next-line no-void + void instance.launch() + await instance.cancel() + expect(onCancel).toBeCalledTimes(1) + instance.removeOnCancelListener(onCancel) + instance.removeOnCancelListener(onCancel) + }) + + test('should create instance with success listener', async () => { + const notify = jest.fn() + const method = jest.fn(fakeSuccessPromise) + const onSuccess = jest.fn() + const instance = new DataLoader({ + key: 'test', + method, + notify, + }) + instance.addOnSuccessListener(onSuccess) + instance.addOnSuccessListener(onSuccess) + await instance.launch() + expect(onSuccess).toBeCalledTimes(1) + instance.removeOnSuccessListener(onSuccess) + instance.removeOnSuccessListener(onSuccess) + }) + + test('should create instance with error listener', async () => { + const notify = jest.fn() + const method = jest.fn(fakeErrorPromise) + const onError = jest.fn() + const instance = new DataLoader({ + key: 'test', + method, + notify, + }) + instance.addOnErrorListener(onError) + instance.addOnErrorListener(onError) + await instance.launch() + expect(onError).toBeCalledTimes(1) + expect(instance.error?.message).toBe('test') + instance.removeOnErrorListener(onError) + instance.removeOnErrorListener(onError) + }) + + test('should create instance with polling', async () => { + const notify = jest.fn() + const method = jest.fn(fakeSuccessPromise) + const instance = new DataLoader({ + key: 'test', + method, + notify, + pollingInterval: PROMISE_TIMEOUT, + }) + await instance.launch() + expect(method).toBeCalledTimes(1) + await instance.launch() + expect(method).toBeCalledTimes(2) + await new Promise(resolve => setTimeout(resolve, PROMISE_TIMEOUT * 2)) + expect(method).toBeCalledTimes(3) + await instance.launch() + instance.destroy() + }) +}) diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx similarity index 95% rename from packages/use-dataloader/src/__tests__/useDataLoader.tsx rename to packages/use-dataloader/src/__tests__/useDataLoader.test.tsx index 9717976d6..f0e64c5ed 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx @@ -2,11 +2,9 @@ import { act, renderHook } from '@testing-library/react-hooks' import React from 'react' import DataLoaderProvider, { useDataLoaderContext } from '../DataLoaderProvider' -import useDataLoader, { - PromiseType, - UseDataLoaderConfig, - UseDataLoaderResult, -} from '../useDataLoader' +import { KEY_IS_NOT_STRING_ERROR } from '../constants' +import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from '../types' +import useDataLoader from '../useDataLoader' type UseDataLoaderHookProps = { config: UseDataLoaderConfig @@ -14,7 +12,7 @@ type UseDataLoaderHookProps = { method: () => Promise } -const PROMISE_TIMEOUT = 100 +const PROMISE_TIMEOUT = 5 const initialProps = { config: { @@ -60,25 +58,19 @@ describe('useDataLoader', () => { expect(result.current.isLoading).toBe(false) }) - test('should render correctly without valid key', async () => { - const { result, waitForNextUpdate } = renderHook< - UseDataLoaderHookProps, - UseDataLoaderResult - >(props => useDataLoader(props.key, props.method), { - initialProps: { - ...initialProps, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error - key: 2, + test('should render correctly without valid key', () => { + const { result } = renderHook( + props => useDataLoader(props.key, props.method), + { + initialProps: { + ...initialProps, + // @ts-expect-error used because we test with bad key + key: 2, + }, + wrapper, }, - wrapper, - }) - expect(result.current.data).toBe(undefined) - expect(result.current.isLoading).toBe(true) - await waitForNextUpdate() - expect(result.current.data).toBe(undefined) - expect(result.current.isSuccess).toBe(true) - expect(result.current.isLoading).toBe(false) + ) + expect(result.error?.message).toBe(KEY_IS_NOT_STRING_ERROR) }) test('should render correctly without keepPreviousData', async () => { @@ -131,7 +123,7 @@ describe('useDataLoader', () => { >( props => [ useDataLoader(props.key, props.method, props.config), - useDataLoader(props.key, props.method, { + useDataLoader('test-2', props.method, { ...props.config, enabled: false, }), @@ -155,10 +147,11 @@ describe('useDataLoader', () => { void result.current[1].reload() }) - expect(result.current[1].data).toBe(true) + expect(result.current[1].data).toBe(undefined) expect(result.current[1].isLoading).toBe(true) await waitForNextUpdate() + expect(result.current[1].data).toBe(true) expect(result.current[1].isSuccess).toBe(true) }) @@ -274,7 +267,7 @@ describe('useDataLoader', () => { rerender({ ...pollingProps, config: { - pollingInterval: 800, + pollingInterval: 300, }, method: method2, }) @@ -546,7 +539,8 @@ describe('useDataLoader', () => { expect(result.current[0].data).toBe(undefined) expect(result.current[0].isLoading).toBe(true) expect(result.current[1].data).toBe(undefined) - expect(result.current[1].isIdle).toBe(true) + expect(result.current[1].isIdle).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) diff --git a/packages/use-dataloader/src/constants.ts b/packages/use-dataloader/src/constants.ts index 69f9ebbc0..a9472cf48 100644 --- a/packages/use-dataloader/src/constants.ts +++ b/packages/use-dataloader/src/constants.ts @@ -1,19 +1,16 @@ -import { DataLoaderStatus } from './types' - -export const StatusEnum: Record< - 'ERROR' | 'IDLE' | 'LOADING' | 'SUCCESS', - DataLoaderStatus -> = { - ERROR: 'error', - IDLE: 'idle', - LOADING: 'loading', - SUCCESS: 'success', +export enum StatusEnum { + ERROR = 'error', + IDLE = 'idle', + LOADING = 'loading', + SUCCESS = 'success', } -export const ActionEnum = { - ON_ERROR: 'ON_ERROR', - ON_LOADING: 'ON_LOADING', - ON_SUCCESS: 'ON_SUCCESS', - ON_UPDATE_DATA: 'ON_UPDATE_DATA', - RESET: 'RESET', +export enum ActionEnum { + ON_ERROR = 'ON_ERROR', + ON_LOADING = 'ON_LOADING', + ON_SUCCESS = 'ON_SUCCESS', + ON_UPDATE_DATA = 'ON_UPDATE_DATA', + RESET = 'RESET', } + +export const KEY_IS_NOT_STRING_ERROR = 'Key should be a string' diff --git a/packages/use-dataloader/src/dataloader.ts b/packages/use-dataloader/src/dataloader.ts index 477a233c3..79ab88e99 100644 --- a/packages/use-dataloader/src/dataloader.ts +++ b/packages/use-dataloader/src/dataloader.ts @@ -1,12 +1,5 @@ -/* eslint-disable no-void */ import { StatusEnum } from './constants' -import { - DataLoaderStatus, - OnCancelFn, - OnErrorFn, - OnSuccessFn, - PromiseType, -} from './types' +import { OnCancelFn, OnErrorFn, OnSuccessFn, PromiseType } from './types' export type DataLoaderConstructorArgs = { enabled?: boolean @@ -20,7 +13,7 @@ export type DataLoaderConstructorArgs = { class DataLoader { public key: string - public status: DataLoaderStatus + public status: StatusEnum public pollingInterval?: number @@ -68,32 +61,29 @@ class DataLoader { this.status = StatusEnum.SUCCESS this.error = undefined this.notify(this) - if (this.successListeners.length) { - this.successListeners.forEach(listener => void listener?.(result)) - } + await Promise.all( + this.successListeners.map(listener => listener?.(result)), + ) } catch (err) { this.status = StatusEnum.ERROR this.error = err as Error this.notify(this) - if (this.errorListeners?.length) { - this.errorListeners.forEach( - listener => void listener?.(this.error as Error), - ) - } + await Promise.all( + this.errorListeners.map(listener => listener?.(err as Error)), + ) } if (this.pollingInterval) { this.timeout = setTimeout( - () => void this.launch(), + // eslint-disable-next-line @typescript-eslint/no-misused-promises + this.launch, this.pollingInterval, ) as unknown as number } } - public cancel = (): void => { + public cancel = async (): Promise => { this.cancelMethod?.() - if (this.cancelListeners.length) { - this.cancelListeners.forEach(listener => void listener?.()) - } + await Promise.all(this.cancelListeners.map(listener => listener?.())) } public addOnSuccessListener(fn: OnSuccessFn): void { diff --git a/packages/use-dataloader/src/types.ts b/packages/use-dataloader/src/types.ts index de50deb37..4744f1f9f 100644 --- a/packages/use-dataloader/src/types.ts +++ b/packages/use-dataloader/src/types.ts @@ -2,8 +2,6 @@ export class PromiseType extends Promise { public cancel?: () => void } -export type DataLoaderStatus = 'error' | 'idle' | 'loading' | 'success' - export type OnErrorFn = ((err: Error) => void | Promise) | undefined export type OnSuccessFn = | ((result: T) => void | Promise)