From 5ed5b7b33a6d9f0320997b3c624b0ae393c3f3dd Mon Sep 17 00:00:00 2001 From: Dave Bauman Date: Wed, 3 Jan 2024 10:16:13 -0500 Subject: [PATCH] fix: Ignore old requests in useActivities hook --- packages/frontend/src/shared/useActivities.ts | 135 ++++++++++++------ 1 file changed, 89 insertions(+), 46 deletions(-) diff --git a/packages/frontend/src/shared/useActivities.ts b/packages/frontend/src/shared/useActivities.ts index d547e9e3f..3d97daced 100644 --- a/packages/frontend/src/shared/useActivities.ts +++ b/packages/frontend/src/shared/useActivities.ts @@ -182,12 +182,55 @@ export interface UseActivitiesProps { paused?: boolean; } -// https://github.com/typescript-eslint/typescript-eslint/issues/2446 -// type PaginationReturn = [{}, () => void]; +export type ActivityResultState = { + data: { + activityConnection?: ActivityConnection; + suggestedFilters?: AutocompleteResults; + }; + error?: CombinedError | string; + fetching: boolean; + after?: string | null; + hasMore: boolean; + total: number; +}; + +export type UseActivitiesReturnType = [ + ActivityResultState, + () => Promise, + ({ activityId: string, liked: boolean }) => Promise +]; -export function useActivities({ query = '', first = 10, after = null, sort, paused = false }: UseActivitiesProps): any { +/** + * Custom hook for performing a search for activities. + * + * This hook is designed to be used with an infinite scroll component, + * so it returns both the search result state and a function to fetch more results. + * The search results include the aggregated results from all pages that have been + * loaded so far. + * + * Any changes to the query or sort options will reset the infinite scroll state + * back to the first page of results. + * + * If multiple requests are made before the first one finishes, only the latest + * request will be used. Urql does not support cancelling requests, so this + * is the best we can do. + * + * @param {UseActivitiesProps} options - The search options. + * @param {string} options.query - The search query. + * @param {number} options.first - The number of results to fetch per page. + * @param {string} options.after - The cursor to start fetching results from. + * @param {string} options.sort - The sort option. + * @param {boolean} [options.paused=false] - Whether the search is paused. + * @returns {UseActivitiesReturnType} - The search result state and a function to fetch more results. + */ +export function useActivities({ + query = '', + first = 10, + after = null, + sort, + paused = false +}: UseActivitiesProps): UseActivitiesReturnType { const [fetching, setFetching] = useState(true); - const loadingNextPage = useRef(false); const [error, setError] = useState(); const total = useRef(0); @@ -196,7 +239,8 @@ export function useActivities({ query = '', first = 10, after = null, sort, paus const afterRef = useRef(after); const hasMoreRef = useRef(true); const [edges, setEdges] = useState([]); - const pending = useRef(false); + + const latestRequest = useRef(); const [, onLikeActivity] = useMutation(LIKE_ACTIVITY_MUTATION); @@ -211,61 +255,60 @@ export function useActivities({ query = '', first = 10, after = null, sort, paus return; } - if (!loadingNextPage.current) { - //console.log(`Loading next page (after = ${afterRef.current})`); - loadingNextPage.current = true; + // Generate a unique ID for this request so we can ignore old responses + const requestId = `r||${query}||${sort}||${first}||${afterRef.current}`; + if (latestRequest.current === requestId) { + // Ignore duplicate requests + // The infinite scroll component may trigger multiple requests for the same page + return; + } - const { data, error } = await urqlClient - .query(ACTIVITIES_QUERY, { - search: query, - first, - after: afterRef.current, - sort: sort && [sort] - }) - .toPromise(); + latestRequest.current = requestId; - setError(error); + const { data, error } = await urqlClient + .query(ACTIVITIES_QUERY, { + search: query, + first, + after: afterRef.current, + sort: sort && [sort] + }) + .toPromise(); - const nextPage = data?.activities; + if (latestRequest.current !== requestId) { + // Ignore this response, it's for an old request + return; + } - if (nextPage != null) { - if (afterRef.current === null) { - // Replace edges with the first page of a new search - setEdges(nextPage.edges); + setError(error); - // Only use total and suggested filters from the first page of results - total.current = nextPage.pageInfo.total; - setSuggestedFilters(nextPage.suggestedFilters); - } else { - setEdges((existing) => [...existing, ...nextPage.edges]); - } + const nextPage = data?.activities; - //console.log('Finished loading page ', nextPage.pageInfo); - afterRef.current = nextPage.pageInfo.endCursor; + if (nextPage != null) { + if (afterRef.current === null) { + // Replace edges with the first page of a new search + setEdges(nextPage.edges); - // If the endCursor is null, there are no results after the previous cursor - hasMoreRef.current = nextPage.pageInfo.endCursor !== null; + // Only use total and suggested filters from the first page of results + total.current = nextPage.pageInfo.total; + setSuggestedFilters(nextPage.suggestedFilters); + } else { + setEdges((existing) => [...existing, ...nextPage.edges]); } - // Done! - loadingNextPage.current = false; - setFetching(false); + //console.log('Finished loading page ', nextPage.pageInfo); + afterRef.current = nextPage.pageInfo.endCursor; - // If a request is pending, process it now - if (pending.current) { - pending.current = false; - fetchMore(); - } - } else { - // If a fetchMore request came while we were loading, pending is set true - // This is here to trigger a subsequent page load to avoid cases where the - // front-end component already triggered fetchMore(), but it was already loading. - pending.current = true; + // If the endCursor is null, there are no results after the previous cursor + hasMoreRef.current = nextPage.pageInfo.endCursor !== null; } + + // Done! + + setFetching(false); }, [paused, query, sort, first]); useEffect(() => { - //console.log('Resetting infinite scroll'); + // Reset the scroll state whenever query/sort changes afterRef.current = null; hasMoreRef.current = true; setEdges([]);