From a9f02ab1b303766890628f946610b30c8c65b27a Mon Sep 17 00:00:00 2001 From: Alexandre Philibeaux Date: Tue, 16 Aug 2022 07:34:52 +0000 Subject: [PATCH 1/2] feat(use-dataloader): handle primitive array key --- .../src/__tests__/helpers.test.ts | 40 ++++++++++ .../src/__tests__/useDataLoader.test.tsx | 73 ++++++++++++------- .../__tests__/usePaginatedDataLoader.test.tsx | 23 ------ packages/use-dataloader/src/helpers.ts | 20 +++++ packages/use-dataloader/src/types.ts | 3 + packages/use-dataloader/src/useDataLoader.ts | 15 +++- .../src/usePaginatedDataLoader.ts | 17 +++-- 7 files changed, 132 insertions(+), 59 deletions(-) create mode 100644 packages/use-dataloader/src/__tests__/helpers.test.ts create mode 100644 packages/use-dataloader/src/helpers.ts diff --git a/packages/use-dataloader/src/__tests__/helpers.test.ts b/packages/use-dataloader/src/__tests__/helpers.test.ts new file mode 100644 index 000000000..e7f3aee68 --- /dev/null +++ b/packages/use-dataloader/src/__tests__/helpers.test.ts @@ -0,0 +1,40 @@ +import { marshalQueryKey } from '../helpers' + +describe('marshalQueryKey', () => { + test('should accept a string', () => { + expect(marshalQueryKey('string')).toStrictEqual('string') + }) + + test('should accept a number', () => { + expect(marshalQueryKey(0)).toStrictEqual('0') + expect(marshalQueryKey(1)).toStrictEqual('1') + }) + + test('should accept primitive array', () => { + const date = new Date('2021') + expect(marshalQueryKey(['defaultKey', 3, null, date, true])).toStrictEqual( + 'defaultKey.3.2021-01-01T00:00:00.000Z.true', + ) + + expect( + marshalQueryKey( + [ + 'default key', + ['number', 3], + ['null', null], + ['date', date], + ['boolean', true], + ].flat(), + ), + ).toStrictEqual( + 'default key.number.3.null.date.2021-01-01T00:00:00.000Z.boolean.true', + ) + }) + + test('should not accept object', () => { + // @ts-expect-error used because we test with bad key + expect(marshalQueryKey(['default key', { object: 'no' }])).toStrictEqual( + 'default key.[object Object]', + ) + }) +}) diff --git a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx index a73c71082..919eca328 100644 --- a/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/useDataLoader.test.tsx @@ -2,13 +2,12 @@ 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 } from '../types' +import { KeyType, UseDataLoaderConfig } from '../types' import useDataLoader from '../useDataLoader' type UseDataLoaderHookProps = { config: UseDataLoaderConfig - key: string + key: KeyType method: () => Promise children?: ReactNode } @@ -61,6 +60,52 @@ describe('useDataLoader', () => { expect(result.current.previousData).toBe(undefined) }) + test('should render correctly with a complexe key', async () => { + const key = [ + 'baseKey', + ['null', null], + ['boolean', false], + ['number', 10], + ].flat() + + const method = jest.fn( + () => + new Promise(resolve => { + setTimeout(() => resolve(true), PROMISE_TIMEOUT) + }), + ) + + const initProps = { + ...initialProps, + key, + method, + } + + const { result, rerender } = renderHook( + props => useDataLoader(props.key, props.method), + { + initialProps: initProps, + 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 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.previousData).toBe(undefined) + + rerender({ ...initProps }) + + expect(initialProps.method).toBeCalledTimes(1) + expect(result.current.data).toBe(true) + expect(result.current.isLoading).toBe(false) + expect(result.current.previousData).toBe(undefined) + }) + test('should render correctly without request enabled then enable it', async () => { const method = jest.fn( () => @@ -96,28 +141,6 @@ describe('useDataLoader', () => { expect(result.current.data).toBe(true) }) - test('should render correctly without valid key', () => { - 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), diff --git a/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx b/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx index be20158ae..402ca2a81 100644 --- a/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx +++ b/packages/use-dataloader/src/__tests__/usePaginatedDataLoader.test.tsx @@ -2,7 +2,6 @@ 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 { UsePaginatedDataLoaderMethodParams } from '../types' import usePaginatedDataLoader from '../usePaginatedDataLoader' @@ -80,28 +79,6 @@ describe('usePaginatedDataLoader', () => { expect(result.current.pageData).toBe(true) }) - test('should render correctly without valid key', () => { - const orignalConsoleError = console.error - console.error = jest.fn - try { - renderHook( - // @ts-expect-error used because we test with bad key - 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 } = renderHook( props => usePaginatedDataLoader(props.key, props.method, props.config), diff --git a/packages/use-dataloader/src/helpers.ts b/packages/use-dataloader/src/helpers.ts new file mode 100644 index 000000000..5c09dd793 --- /dev/null +++ b/packages/use-dataloader/src/helpers.ts @@ -0,0 +1,20 @@ +import type { KeyType } from './types' + +/** + * + * @param {KeyType} queryKey + * @returns string + */ +export const marshalQueryKey = (queryKey: KeyType) => + Array.isArray(queryKey) + ? queryKey + .filter(Boolean) + .map(subKey => { + if (subKey instanceof Date) { + return subKey.toISOString() + } + + return subKey?.toString() + }) + .join('.') + : queryKey?.toString() diff --git a/packages/use-dataloader/src/types.ts b/packages/use-dataloader/src/types.ts index 2a0421e87..eec8b728c 100644 --- a/packages/use-dataloader/src/types.ts +++ b/packages/use-dataloader/src/types.ts @@ -1,3 +1,6 @@ +type PrimitiveType = string | number | boolean | null | undefined | Date +export type KeyType = string | number | PrimitiveType[] + export class PromiseType extends Promise { public cancel?: () => void } diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index 5956b3000..6519e9626 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -1,10 +1,16 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useDataLoaderContext } from './DataLoaderProvider' import { StatusEnum } from './constants' -import { PromiseType, UseDataLoaderConfig, UseDataLoaderResult } from './types' +import { marshalQueryKey } from './helpers' +import { + KeyType, + PromiseType, + UseDataLoaderConfig, + UseDataLoaderResult, +} from './types' function useDataLoader( - fetchKey: string, + key: KeyType, method: () => PromiseType, { enabled = true, @@ -23,11 +29,14 @@ function useDataLoader( const onErrorRef = useRef(onError ?? onGlobalError) const needPollingRef = useRef(needPolling) const [, setCounter] = useState(0) + const forceRerender = useCallback(() => { setCounter(current => current + 1) }, []) - const request = getOrAddRequest(fetchKey, { + const queryKey = useMemo(() => marshalQueryKey(key), [key]) + + const request = getOrAddRequest(queryKey, { enabled, method: methodRef.current, }) diff --git a/packages/use-dataloader/src/usePaginatedDataLoader.ts b/packages/use-dataloader/src/usePaginatedDataLoader.ts index b617a8228..e25b1f412 100644 --- a/packages/use-dataloader/src/usePaginatedDataLoader.ts +++ b/packages/use-dataloader/src/usePaginatedDataLoader.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useState } from 'react' -import { KEY_IS_NOT_STRING_ERROR } from './constants' import { + KeyType, PromiseType, UsePaginatedDataLoaderConfig, UsePaginatedDataLoaderMethodParams, @@ -9,13 +9,13 @@ import { import useDataLoader from './useDataLoader' /** - * @param {string} baseFetchKey base key used to cache data. Hook append -page-X to that key for each page you load + * @param {KeyType} key base key used to cache data. Hook append -page-X to that key for each page you load * @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 usePaginatedDataLoader = ( - baseFetchKey: string, + key: KeyType, method: ( params: UsePaginatedDataLoaderMethodParams, ) => PromiseType, @@ -32,9 +32,10 @@ const usePaginatedDataLoader = ( perPage = 1, }: UsePaginatedDataLoaderConfig = {}, ): UsePaginatedDataLoaderResult => { - if (typeof baseFetchKey !== 'string') { - throw new Error(KEY_IS_NOT_STRING_ERROR) - } + const flatKey = useCallback( + (page: number) => [key, ['page', page]].flat(), + [key], + ) const [data, setData] = useState>({}) const [page, setPage] = useState(initialPage ?? 1) @@ -52,7 +53,7 @@ const usePaginatedDataLoader = ( isSuccess, reload, error, - } = useDataLoader(`${baseFetchKey}-page-${page}`, pageMethod, { + } = useDataLoader(flatKey(page), pageMethod, { dataLifetime, enabled, initialData, @@ -88,7 +89,7 @@ const usePaginatedDataLoader = ( useEffect(() => { setPage(1) setData({}) - }, [baseFetchKey]) + }, [key]) return { data, From 9cecba9a73486c1c28bbeb03d2452a38c68b25b2 Mon Sep 17 00:00:00 2001 From: Alexandre Philibeaux Date: Tue, 16 Aug 2022 15:34:10 +0000 Subject: [PATCH 2/2] fix(paginated-dataloader): useless memo --- packages/use-dataloader/src/usePaginatedDataLoader.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/use-dataloader/src/usePaginatedDataLoader.ts b/packages/use-dataloader/src/usePaginatedDataLoader.ts index e25b1f412..ad7e892c4 100644 --- a/packages/use-dataloader/src/usePaginatedDataLoader.ts +++ b/packages/use-dataloader/src/usePaginatedDataLoader.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import { KeyType, PromiseType, @@ -32,14 +32,11 @@ const usePaginatedDataLoader = ( perPage = 1, }: UsePaginatedDataLoaderConfig = {}, ): UsePaginatedDataLoaderResult => { - const flatKey = useCallback( - (page: number) => [key, ['page', page]].flat(), - [key], - ) - const [data, setData] = useState>({}) const [page, setPage] = useState(initialPage ?? 1) + const keyPage = useMemo(() => [key, ['page', page]].flat(), [key, page]) + const pageMethod = useCallback( () => method({ page, perPage }), [method, page, perPage], @@ -53,7 +50,7 @@ const usePaginatedDataLoader = ( isSuccess, reload, error, - } = useDataLoader(flatKey(page), pageMethod, { + } = useDataLoader(keyPage, pageMethod, { dataLifetime, enabled, initialData,