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 useSubscription restart was not cached #12044

Conversation

DoctorJohn
Copy link
Contributor

@DoctorJohn DoctorJohn commented Aug 28, 2024

This PR fixes that the useSubscription hook's definition of the recently added (#11927) restart function was not cached between rerenders (unlike refetch and fetchMore returned by similar hooks). This made the restart function unsuitable to be included in the dependency array of React.useEffect.

My use case is restarting a subscription when a React Native app is resumed from the background. @phryneas I assume it's evident from the code that restart is redefined every render. However, if you would like a reproducible, please let me know.

@apollo-cla
Copy link

@DoctorJohn: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

Copy link

netlify bot commented Aug 28, 2024

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit 5842753
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/66d739d3fda26200085f14ba
😎 Deploy Preview https://deploy-preview-12044--apollo-client-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

changeset-bot bot commented Aug 28, 2024

🦋 Changeset detected

Latest commit: 5842753

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@apollo/client Patch

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

@phryneas
Copy link
Member

phryneas commented Sep 3, 2024

Great catch, thank you for the contribution!
I'll clean that up a bit and get it released in a few minutes :)

@phryneas phryneas merged commit 04462a2 into apollographql:main Sep 3, 2024
41 of 42 checks passed
@github-actions github-actions bot mentioned this pull request Sep 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants