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: abort stale connection on reobserve (#9532) #9791

Conversation

javier-garcia-meteologica
Copy link
Contributor

@javier-garcia-meteologica javier-garcia-meteologica commented Jun 7, 2022

It makes sure that after a ObservableQuery is reboserved (e.g. variables are changed), it unsubscribes from stale connections (those using old variables) so they can be cancelled by the browser.

Problem example: Use watchQuery with an interactive searchValue as input. When the user types too fast and the query take longer than the debounce limit (300ms), apollo client makes new connections to the bakend with the new searchValue. However old connections, which are a burden to the backend, are not aborted.

Fixes #9532

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@javier-garcia-meteologica javier-garcia-meteologica force-pushed the fix_abort_stale_connection_reobserve branch 3 times, most recently from 54dae34 to 4881f03 Compare June 7, 2022 08:49
@jpvajda jpvajda added the 🏓 awaiting-team-response requires input from the apollo team label Jun 9, 2022
@jpvajda
Copy link
Contributor

jpvajda commented Jun 9, 2022

@javier-garcia-meteologica Thank you for contribution, could you give us a little more detail of what you are submitting this PR?

@javier-garcia-meteologica
Copy link
Contributor Author

@jpvajda I've added a description to this PR, which I had previously forgotten.

@benjamn benjamn force-pushed the fix_abort_stale_connection_reobserve branch from eadf0b6 to 182b3e7 Compare June 10, 2022 23:16
@benjamn benjamn added this to the v3.6.x patch releases milestone Jun 10, 2022
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing (and testing) this @javier-garcia-meteologica!

this.concast.removeObserver(this.observer, true);
this.concast.removeObserver(this.observer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been testing this change in the v3.7 betas thanks to #9718, with no problems so far.

@benjamn benjamn added 🧞 fixed-by-contributor issues where the contributor went the extra mile and fixed the problem with a PR and removed 🏓 awaiting-team-response requires input from the apollo team labels Jun 10, 2022
@benjamn benjamn merged commit 3d039f8 into apollographql:main Jun 10, 2022
@benjamn
Copy link
Member

benjamn commented Jun 10, 2022

@javier-garcia-meteologica These changes have been soft-published in @apollo/client@3.6.8, meaning you can install that version directly to test the changes, and we will promote it to latest probably this coming Monday, if everything looks good. Thanks again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧞 fixed-by-contributor issues where the contributor went the extra mile and fixed the problem with a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

watchQuery + reobserve no longer aborts stale queries
3 participants