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.

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

Conversation

baumandm
Copy link
Contributor

@baumandm baumandm commented Jan 2, 2024

Previously, search-as-you-type could result in a scenario where the results displayed didn't match the user's search. This occurred because of a bug in the pending.current logic that delayed subsequent searches and called fetchMore() after the pending query completed. The issue is that fetchMore() runs in the closure of the previous search, so it actually just repeated the old search instead of performing a new search with new variables.

This PR addresses this issue by removing the pending.current check, firing all requests when they come in. When requests complete, they are ignored if they are not the most-recently triggered. Urql doesn't support AbortController, so we can't cancel the in-progress requests. If necessary, we can increase the debounce delay to reduce load.

The infinite scroll controller tends to trigger multiple duplicate requests, so these are also being filtered out before sending using similar logic.

@baumandm baumandm requested a review from a team as a code owner January 2, 2024 21:28
Previously, search-as-you-type could result in a scenario where the results displayed didn't match the user's search.  This occurred because of a bug in the `pending.current` logic that delayed subsequent searches and called `fetchMore()` after the pending query completed.  The issue is that `fetchMore()` runs in the closure of the previous search, so it actually just repeated the old search instead of performing a new search with new variables.

This PR addresses this issue by removing the `pending.current` check, firing all requests when they come in.  When requests complete, they are ignored if they are not the most-recently triggered.  Urql doesn't support AbortController, so we can't cancel the in-progress requests.  If necessary, we can increase the debounce delay to reduce load.

The infinite scroll controller tends to trigger multiple duplicate requests, so these are also being filtered out before sending using similar logic.
@baumandm baumandm force-pushed the fix/use-search-hook branch from ad7df0d to 10632c1 Compare January 3, 2024 15:17
@baumandm baumandm force-pushed the fix/use-search-hook branch from 10632c1 to 5ed5b7b Compare January 3, 2024 15:33
@baumandm baumandm merged commit 7eb0a18 into main Jan 3, 2024
3 checks passed
@baumandm baumandm deleted the fix/use-search-hook branch January 3, 2024 15:39
@eg-oss-ci
Copy link
Contributor

🎉 This PR is included in version 3.25.14 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants