From 85a75491e5edb0688d875982e0270df21f05bf55 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Mon, 14 Apr 2025 16:10:43 +0200 Subject: [PATCH 1/2] fix: use infinite and add defaut lifetime --- .../use-dataloader/src/DataLoaderProvider.tsx | 12 +- packages/use-dataloader/src/useDataLoader.ts | 15 ++- .../src/useInfiniteDataLoader.ts | 122 +++++++++++------- 3 files changed, 91 insertions(+), 58 deletions(-) diff --git a/packages/use-dataloader/src/DataLoaderProvider.tsx b/packages/use-dataloader/src/DataLoaderProvider.tsx index f2254e579..077321837 100644 --- a/packages/use-dataloader/src/DataLoaderProvider.tsx +++ b/packages/use-dataloader/src/DataLoaderProvider.tsx @@ -14,10 +14,6 @@ type Requests = Record> type UseDataLoaderInitializerArgs = { method: () => PromiseType - /** - * Max time before data from previous success is considered as outdated (in millisecond) - */ - maxDataLifetime?: number enabled?: boolean } @@ -42,6 +38,7 @@ export type IDataLoaderContext = { ) => DataLoader computeKey: (key: string) => string cacheKeyPrefix?: string + defaultDatalifetime?: number onError?: (error: Error) => void | Promise clearAllCachedData: () => void clearCachedData: (key: string) => void @@ -63,6 +60,10 @@ type DataLoaderProviderProps = { cacheKeyPrefix?: string onError?: OnErrorFn maxConcurrentRequests?: number + /** + * Default request lifetime in milliseconds. It doesnt override values passed to hooks + */ + defaultDatalifetime?: number } const DataLoaderProvider = ({ @@ -70,6 +71,7 @@ const DataLoaderProvider = ({ cacheKeyPrefix, onError, maxConcurrentRequests = DEFAULT_MAX_CONCURRENT_REQUESTS, + defaultDatalifetime, }: DataLoaderProviderProps): ReactElement => { const requestsRef = useRef({}) @@ -204,6 +206,7 @@ const DataLoaderProvider = ({ reloadAll, reloadGroup, computeKey, + defaultDatalifetime, }), [ addRequest, @@ -219,6 +222,7 @@ const DataLoaderProvider = ({ reloadAll, reloadGroup, computeKey, + defaultDatalifetime, ], ) diff --git a/packages/use-dataloader/src/useDataLoader.ts b/packages/use-dataloader/src/useDataLoader.ts index 247b537c9..d1884eed6 100644 --- a/packages/use-dataloader/src/useDataLoader.ts +++ b/packages/use-dataloader/src/useDataLoader.ts @@ -23,7 +23,12 @@ export const useDataLoader = ( dataLifetime, }: UseDataLoaderConfig = {}, ): UseDataLoaderResult => { - const { getOrAddRequest, onError: onGlobalError } = useDataLoaderContext() + const { + getOrAddRequest, + onError: onGlobalError, + defaultDatalifetime, + } = useDataLoaderContext() + const computedDatalifetime = dataLifetime ?? defaultDatalifetime const methodRef = useRef(method) const onSuccessRef = useRef(onSuccess) const onErrorRef = useRef(onError ?? onGlobalError) @@ -54,12 +59,12 @@ export const useDataLoader = ( !!( enabled && (!request.dataUpdatedAt || - !dataLifetime || + !computedDatalifetime || (request.dataUpdatedAt && - dataLifetime && - request.dataUpdatedAt + dataLifetime < Date.now())) + computedDatalifetime && + request.dataUpdatedAt + computedDatalifetime < Date.now())) ), - [enabled, request.dataUpdatedAt, dataLifetime], + [enabled, request.dataUpdatedAt, computedDatalifetime], ) const optimisticIsLoadingRef = useRef(needLoad) diff --git a/packages/use-dataloader/src/useInfiniteDataLoader.ts b/packages/use-dataloader/src/useInfiniteDataLoader.ts index 3148a677c..180d0a331 100644 --- a/packages/use-dataloader/src/useInfiniteDataLoader.ts +++ b/packages/use-dataloader/src/useInfiniteDataLoader.ts @@ -34,6 +34,7 @@ export const useInfiniteDataLoader = < getOrAddRequest, computeKey, onError: onGlobalError, + defaultDatalifetime, } = useDataLoaderContext() const { enabled = true, @@ -44,24 +45,32 @@ export const useInfiniteDataLoader = < dataLifetime, getNextPage, } = config ?? {} + const computedDatalifetime = dataLifetime ?? defaultDatalifetime const requestRefs = useRef[]>([]) const [page, setPage] = useState(baseParams[pageParamKey]) const nextPageRef = useRef(page) - const getNextPageRef = useRef(getNextPage) - const methodRef = useRef(() => - method( - pageParamKey && page - ? ({ ...baseParams, [pageParamKey]: page } as ParamsType) - : baseParams, - ), + + const getNextPageFnRef = useRef( + (...params: Parameters>) => + getNextPage ? getNextPage(...params) : undefined, + ) + + const getParamsRef = useRef(() => ({ + ...baseParams, + [pageParamKey]: nextPageRef.current, + })) + + const getMethodRef = useRef(() => method(getParamsRef.current())) + + const getOnSuccessRef = useRef( + (...params: Parameters>) => + onSuccess?.(...params), ) - const paramsRef = useRef( - pageParamKey && page - ? ({ ...baseParams, [pageParamKey]: page } as ParamsType) - : baseParams, + + const getOnErrorRef = useRef( + (err: ErrorType) => onError?.(err) ?? onGlobalError?.(err), ) - const onSuccessRef = useRef(onSuccess) - const onErrorRef = useRef(onError ?? onGlobalError) + const [, setCounter] = useState(0) const forceRerender = useCallback(() => { @@ -99,7 +108,7 @@ export const useInfiniteDataLoader = < if (!requestInRef) { const request = getOrAddRequest(currentQueryKey, { enabled, - method: methodRef.current, + method: getMethodRef.current, }) requestRefs.current.push(request) request.addObserver(forceRerender) @@ -117,12 +126,12 @@ export const useInfiniteDataLoader = < !!( enabled && (!request.dataUpdatedAt || - !dataLifetime || + !computedDatalifetime || (request.dataUpdatedAt && - dataLifetime && - request.dataUpdatedAt + dataLifetime < Date.now())) + computedDatalifetime && + request.dataUpdatedAt + computedDatalifetime < Date.now())) ), - [enabled, request.dataUpdatedAt, dataLifetime], + [enabled, request.dataUpdatedAt, computedDatalifetime], ) const optimisticIsLoadingRef = useRef(needLoad) @@ -149,22 +158,28 @@ export const useInfiniteDataLoader = < const reload = useCallback(async () => { await Promise.all( requestRefs.current.map(req => - req.load(true).then(onSuccessRef.current).catch(onErrorRef.current), + req + .load(true) + .then(getOnSuccessRef.current) + .catch(getOnErrorRef.current), ), ) }, []) - useEffect(() => { - request.method = () => method(paramsRef.current) - }, [method, request]) - - useEffect(() => { - onSuccessRef.current = onSuccess - }, [onSuccess]) + const loadMore = useCallback(() => { + const nextPage = nextPageRef.current + if (nextPage) { + setPage(() => nextPage) + getParamsRef.current = () => ({ + ...baseParams, + [pageParamKey]: nextPage, + }) + } + }, [baseParams, pageParamKey]) useEffect(() => { - onErrorRef.current = onError ?? onGlobalError - }, [onError, onGlobalError]) + request.method = () => method(getParamsRef.current()) + }, [method, request]) useEffect(() => { if (keepPreviousData) { @@ -172,31 +187,27 @@ export const useInfiniteDataLoader = < } }, [request.data, keepPreviousData]) + // Reset page when baseParams or pageParamKey change useEffect(() => { - getNextPageRef.current = getNextPage - }, [getNextPage]) - - useEffect(() => { - paramsRef.current = - pageParamKey && page - ? ({ ...baseParams, [pageParamKey]: page } as ParamsType) - : baseParams - }, [baseParams, pageParamKey, page]) + setPage(() => baseParams[pageParamKey]) + nextPageRef.current = baseParams[pageParamKey] + getParamsRef.current = () => ({ + ...baseParams, + [pageParamKey]: nextPageRef.current, + }) + }, [baseParams, pageParamKey]) useEffect(() => { if (needLoad) { - const defaultOnSuccessOrError = () => {} - const onSuccessLoad = onSuccessRef.current ?? defaultOnSuccessOrError - const onFailedLoad = onErrorRef.current ?? defaultOnSuccessOrError + const onSuccessLoad = getOnSuccessRef.current + const onFailedLoad = getOnErrorRef.current request .load() .then(async result => { - if (getNextPageRef.current) { - nextPageRef.current = getNextPageRef.current( - result, - paramsRef.current, - ) as typeof nextPageRef.current - } + nextPageRef.current = getNextPageFnRef.current( + result, + getParamsRef.current(), + ) as typeof page await onSuccessLoad(result) }) .catch(onFailedLoad) @@ -204,9 +215,22 @@ export const useInfiniteDataLoader = < optimisticIsLoadingRef.current = false }, [needLoad, request]) - const loadMore = useCallback(() => { - setPage(nextPageRef.current) - }, []) + useEffect(() => { + getParamsRef.current = () => ({ + ...baseParams, + [pageParamKey]: nextPageRef.current, + }) + }, [baseParams, pageParamKey]) + useEffect(() => { + getOnSuccessRef.current = (...params) => onSuccess?.(...params) + }, [onSuccess]) + useEffect(() => { + getOnErrorRef.current = err => onError?.(err) ?? onGlobalError?.(err) + }, [onError, onGlobalError]) + useEffect(() => { + getNextPageFnRef.current = (...params) => + getNextPage ? getNextPage(...params) : undefined + }, [getNextPage]) const data = useMemo>( () => ({ From 17aae4865dfa0ff0e5c42eb680c25493fd3c4eb5 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Mon, 14 Apr 2025 16:11:28 +0200 Subject: [PATCH 2/2] fix: add changeset --- .changeset/all-mirrors-win.md | 5 +++++ .changeset/young-ants-rescue.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/all-mirrors-win.md create mode 100644 .changeset/young-ants-rescue.md diff --git a/.changeset/all-mirrors-win.md b/.changeset/all-mirrors-win.md new file mode 100644 index 000000000..7ed3a643d --- /dev/null +++ b/.changeset/all-mirrors-win.md @@ -0,0 +1,5 @@ +--- +"@scaleway/use-dataloader": minor +--- + +Fix: useInfiniteDataloader doesnt update on params change diff --git a/.changeset/young-ants-rescue.md b/.changeset/young-ants-rescue.md new file mode 100644 index 000000000..d21c66a90 --- /dev/null +++ b/.changeset/young-ants-rescue.md @@ -0,0 +1,5 @@ +--- +"@scaleway/use-dataloader": minor +--- + +Feat: Add a default lifetime on the provider