-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 useBackgroundQuery: dispose ref after unmount and not used #11696
fix useBackgroundQuery: dispose ref after unmount and not used #11696
Conversation
👷 Deploy request for apollo-client-docs pending review.Visit the deploys page to approve it
|
🦋 Changeset detectedLatest commit: 1083346 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @PiR1 👋
Thanks so much for the contribution! This definitely seems in line with some ideas I had on fixing this, so this change makes sense.
I spotted a couple things that may or may not be problematic. Would you mind taking a look at those comments? Thanks again!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is starting to look really great! I had a few more suggestions on naming and the API itself, but generally I'm pretty happy with this direction. This should be my last round of changes barring any other edge cases I can think of.
Don't worry about the size and API extractor jobs failing. I'll make sure those get updated before this is merged 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 🔥 💯 This looks great! Thanks so much for diving into the the depths of QueryReference
to fix this! This is a really great optimization.
@PiR1 if you wouldn't mind making sure the formatting job passes, I'll get the size/api extractor updated to pass. I'll get this merged in as soon as these are all green. |
@jerelmiller Thank you very much for all the feedback and explanations! |
Absolutely! I hope to see more contributions from you in the future! (if you're interested of course 🙂 ) |
Fixes: #11649, based on tests created in this pull request: #11651