Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ignore old requests in useSearch hook #1862

Merged
merged 2 commits into from
Jan 3, 2024
Merged

fix: Ignore old requests in useSearch hook #1862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78df5f0
fix: Ignore old requests in useSearch hook
baumandm Jan 2, 2024
5ed5b7b
fix: Ignore old requests in useActivities hook
baumandm Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 89 additions & 46 deletions packages/frontend/src/shared/useActivities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>,
({ activityId: string, liked: boolean }) => Promise<any>
];

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<CombinedError | string | undefined>();
const total = useRef(0);
Expand All @@ -196,7 +239,8 @@ export function useActivities({ query = '', first = 10, after = null, sort, paus
const afterRef = useRef<string | null>(after);
const hasMoreRef = useRef<boolean>(true);
const [edges, setEdges] = useState<ActivityEdge[]>([]);
const pending = useRef(false);

const latestRequest = useRef<string | undefined>();

const [, onLikeActivity] = useMutation(LIKE_ACTIVITY_MUTATION);

Expand All @@ -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([]);
Expand Down
126 changes: 78 additions & 48 deletions packages/frontend/src/shared/useSearch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,45 @@ export interface UseSearchProps {
paused?: boolean;
}

// https://github.com/typescript-eslint/typescript-eslint/issues/2446
// type PaginationReturn = [{}, () => void];
export type SearchResultState = {
data: {
insights: {
suggestedFilters?: AutocompleteResults;
results: SearchResult[];
};
};
error?: CombinedError | string;
fetching: boolean;
from: React.MutableRefObject<number>;
hasMore: boolean;
total: number;
};

export type UseSearchReturnType = [SearchResultState, () => Promise<void>];

export function useSearch({ query, sort, paused = false }: UseSearchProps): any {
/**
* Custom hook for performing a search.
*
* 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 {UseSearchProps} options - The search options.
* @param {string} options.query - The search query.
* @param {string} options.sort - The sort option.
* @param {boolean} [options.paused=false] - Whether the search is paused.
* @returns {UseSearchReturnType} - The search result state and a function to fetch more results.
*/
export function useSearch({ query, sort, paused = false }: UseSearchProps): UseSearchReturnType {
const [fetching, setFetching] = useState(true);
const loadingNextPage = useRef(false);

const [suggestedFilters, setSuggestedFilters] = useState<AutocompleteResults | undefined>();
const [error, setError] = useState<CombinedError | string | undefined>();
Expand All @@ -99,69 +132,66 @@ export function useSearch({ query, sort, paused = false }: UseSearchProps): any
const size = 20; // Fixed value for now
const from = useRef<number>(0);
const [results, setResults] = useState<SearchResult[]>([]);
const pending = useRef(false);

const latestRequest = useRef<string | undefined>();

const fetchMore = useCallback(async () => {
if (paused) {
// Don't fetch when paused
return;
}

if (!loadingNextPage.current) {
// console.log(`Loading next page (from = ${from.current})`);
loadingNextPage.current = true;

const { data: nextPage, error } = await urqlClient
.query(INSIGHTS_QUERY, {
search: {
query: query || '',
sort: sort && [sort],
paging: {
from: from.current,
size
}
}
})
.toPromise();

setError(error);
// Generate a unique ID for this request so we can ignore old responses
const requestId = `r||${query}||${sort}||${from.current}`;
if (latestRequest.current === requestId) {
// Ignore duplicate requests
// The infinite scroll component may trigger multiple requests for the same page
return;
}

if (nextPage != null) {
if (from.current === 0) {
// Replace results with the first page of a new search
setResults(nextPage.insights.results);
latestRequest.current = requestId;

// Only use total and suggested filters from the first page of results
total.current = nextPage.insights.pageInfo.total;
setSuggestedFilters(nextPage.insights.suggestedFilters);
} else {
setResults((existing) => [...existing, ...nextPage.insights.results]);
const { data: nextPage, error } = await urqlClient
.query(INSIGHTS_QUERY, {
search: {
query: query || '',
sort: sort && [sort],
paging: {
from: from.current,
size
}
}
})
.toPromise();

// console.log('Finished loading page ' + from.current);
from.current += size;
}
if (latestRequest.current !== requestId) {
// Ignore this response, it's for an old request
return;
}

setError(error);

// Done!
loadingNextPage.current = false;
setFetching(false);
if (nextPage != null) {
if (from.current === 0) {
// Replace results with the first page of a new search
setResults(nextPage.insights.results);

// If a request is pending, process it now
if (pending.current) {
pending.current = false;
fetchMore();
// Only use total and suggested filters from the first page of results
total.current = nextPage.insights.pageInfo.total;
setSuggestedFilters(nextPage.insights.suggestedFilters);
} else {
setResults((existing) => [...existing, ...nextPage.insights.results]);
}
} 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;

from.current += size;
}

// Done!
setFetching(false);
}, [paused, query, sort]);

useEffect(() => {
// Reset the scroll state whenever query/sort changes
// console.log('Resetting infinite scroll');
from.current = 0;
setResults([]);
setFetching(true);
Expand Down
Loading