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 issue where a query may not stop polling after unmount when in Strict mode with cache-and-network fetch policy #11837

Merged
merged 11 commits into from
May 13, 2024

Conversation

jerelmiller
Copy link
Member

Fixes #9431
Fixes #11750

When using useQuery in React's strict mode, 2 ObservableQuery instances are created due to how React's strict mode works with refs and rendering the component twice. The 2 ObservableQuery objects are created because the useInternalState hook runs twice in strict mode and the ref.current property is not set until after the component renders the second time. Because of this, a cache broadcast would happen on teardown of the 2nd observable query (the one actually used by useQuery) and this would cause the 1st observable query to start polling itself because it was never torn down.

This change updates the updatePolling function to only run polling if there are active subscribers to the ObservableQuery.

Copy link

changeset-bot bot commented May 9, 2024

🦋 Changeset detected

Latest commit: 75bce0d

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

@jerelmiller jerelmiller requested review from phryneas and alessbell May 9, 2024 17:25
@jerelmiller
Copy link
Member Author

/release:pr

Copy link
Contributor

github-actions bot commented May 9, 2024

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 38.65 KB (+0.02% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 47.06 KB (+0.02% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 44.69 KB (+0.02% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 34.18 KB (+0.02% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 32.06 KB (+0.02% 🔺)
import { ApolloProvider } from "dist/react/index.js" 1.23 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.22 KB (0%)
import { useQuery } from "dist/react/index.js" 5.28 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.36 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 5.52 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.59 KB (0%)
import { useMutation } from "dist/react/index.js" 3.52 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.74 KB (0%)
import { useSubscription } from "dist/react/index.js" 3.21 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.4 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 5.44 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.1 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 4.96 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.61 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" 5.07 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.72 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3.33 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 3.27 KB (0%)
import { useFragment } from "dist/react/index.js" 2.29 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.23 KB (0%)

Copy link
Contributor

github-actions bot commented May 9, 2024

A new release has been made for this PR. You can install it with:

npm i @apollo/[email protected]

Copy link

netlify bot commented May 9, 2024

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit 57515f3
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/663d06e1a76bd60008bb7a25
😎 Deploy Preview https://deploy-preview-11837--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

netlify bot commented May 9, 2024

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit 75bce0d
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/66423c5c4fc98a0008007abb
😎 Deploy Preview https://deploy-preview-11837--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.

@jerelmiller jerelmiller force-pushed the jerel/cache-and-network-polling branch 2 times, most recently from a61042c to c9bdf40 Compare May 9, 2024 18:42
@@ -781,7 +781,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
options: { pollInterval },
} = this;

if (!pollInterval) {
if (!pollInterval || !this.hasObservers()) {
Copy link
Member

@phryneas phryneas May 13, 2024

Choose a reason for hiding this comment

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

Are we sure that polling is started correctly in all cases now, even if a subscription starts delayed?

Ah, it is called from reobserveAsConcast.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. Thats when this polling would kick in. When the component unmounted, it triggered a broadcast, which ended up calling this updatePolling function and starting a new poll interval from the observable that wasn't subscribed to.

@github-actions github-actions bot added the auto-cleanup 🤖 label May 13, 2024
@jerelmiller jerelmiller merged commit dff15b1 into main May 13, 2024
33 checks passed
@jerelmiller jerelmiller deleted the jerel/cache-and-network-polling branch May 13, 2024 16:25
@github-actions github-actions bot mentioned this pull request May 13, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants