feat: add a new datalifetime on provider and fix useinfinitedataloader (#2548)

* fix: use infinite and add defaut lifetime

* fix: add changeset

Author: DorianMaliszewski

.changeset/all-mirrors-win.md

@@ -0,0 +1,5 @@
+---
+"@scaleway/use-dataloader": minor
+---
+
+Fix: useInfiniteDataloader doesnt update on params change

.changeset/young-ants-rescue.md

@@ -0,0 +1,5 @@
+---
+"@scaleway/use-dataloader": minor
+---
+
+Feat: Add a default lifetime on the provider

packages/use-dataloader/src/DataLoaderProvider.tsx

@@ -14,10 +14,6 @@ type Requests = Record<string, DataLoader<unknown, unknown>>
 
 type UseDataLoaderInitializerArgs<ResultType = unknown> = {
   method: () => PromiseType<ResultType>
-  /**
-   * 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<ResultType, ErrorType>
   computeKey: (key: string) => string
   cacheKeyPrefix?: string
+  defaultDatalifetime?: number
   onError?: (error: Error) => void | Promise<void>
   clearAllCachedData: () => void
   clearCachedData: (key: string) => void
@@ -63,13 +60,18 @@ type DataLoaderProviderProps = {
   cacheKeyPrefix?: string
   onError?: OnErrorFn
   maxConcurrentRequests?: number
+  /**
+   * Default request lifetime in milliseconds. It doesnt override values passed to hooks
+   */
+  defaultDatalifetime?: number
 }
 
 const DataLoaderProvider = ({
   children,
   cacheKeyPrefix,
   onError,
   maxConcurrentRequests = DEFAULT_MAX_CONCURRENT_REQUESTS,
+  defaultDatalifetime,
 }: DataLoaderProviderProps): ReactElement => {
   const requestsRef = useRef<Requests>({})
 
@@ -204,6 +206,7 @@ const DataLoaderProvider = ({
       reloadAll,
       reloadGroup,
       computeKey,
+      defaultDatalifetime,
     }),
     [
       addRequest,
@@ -219,6 +222,7 @@ const DataLoaderProvider = ({
       reloadAll,
       reloadGroup,
       computeKey,
+      defaultDatalifetime,
     ],
   )
 

packages/use-dataloader/src/useDataLoader.ts

@@ -23,7 +23,12 @@ export const useDataLoader = <ResultType = unknown, ErrorType = Error>(
     dataLifetime,
   }: UseDataLoaderConfig<ResultType, ErrorType> = {},
 ): UseDataLoaderResult<ResultType, ErrorType> => {
-  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 = <ResultType = unknown, ErrorType = Error>(
       !!(
         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)

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<DataLoader<ResultType, ErrorType>[]>([])
   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<NonNullable<typeof getNextPage>>) =>
+      getNextPage ? getNextPage(...params) : undefined,
+  )
+
+  const getParamsRef = useRef(() => ({
+    ...baseParams,
+    [pageParamKey]: nextPageRef.current,
+  }))
+
+  const getMethodRef = useRef(() => method(getParamsRef.current()))
+
+  const getOnSuccessRef = useRef(
+    (...params: Parameters<NonNullable<typeof onSuccess>>) =>
+      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<ResultType, ErrorType>(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,64 +158,79 @@ 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) {
       previousDataRef.current = request.data
     }
   }, [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)
     }
     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<UseInfiniteDataLoaderResult<ResultType, ErrorType>>(
     () => ({