useSubscription: fix rules of React violations

[phryneas]

Another one for https://github.com/apollographql/apollo-client/pull/11511 - this one is essentially a rewrite.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 33829ecc5946fef5c8cff06880aa61b2221d7f57

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	Minor

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

[github-actions[bot]]

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	38.69 KB (+0.04% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	47.48 KB (+0.01% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	45.08 KB (+0.07% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	34.22 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	32.08 KB (0%)
import { ApolloProvider } from "dist/react/index.js"	1.26 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production)	1.24 KB (0%)
import { useQuery } from "dist/react/index.js"	5.32 KB (0%)
import { useQuery } from "dist/react/index.js" (production)	4.39 KB (0%)
import { useLazyQuery } from "dist/react/index.js"	5.6 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production)	4.67 KB (0%)
import { useMutation } from "dist/react/index.js"	3.59 KB (0%)
import { useMutation } from "dist/react/index.js" (production)	2.81 KB (0%)
import { useSubscription } from "dist/react/index.js"	3.63 KB (+12.24% 🔺)
import { useSubscription } from "dist/react/index.js" (production)	2.78 KB (+14.49% 🔺)
import { useSuspenseQuery } from "dist/react/index.js"	5.47 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production)	4.12 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	4.98 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.63 KB (0%)
import { useLoadableQuery } from "dist/react/index.js"	5.04 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production)	3.69 KB (0%)
import { useReadQuery } from "dist/react/index.js"	3.35 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	3.3 KB (0%)
import { useFragment } from "dist/react/index.js"	2.32 KB (0%)
import { useFragment } from "dist/react/index.js" (production)	2.27 KB (0%)

[netlify[bot]]

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	33829ecc5946fef5c8cff06880aa61b2221d7f57
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/668534061444cb00082c8652
😎 Deploy Preview	https://deploy-preview-11863--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle 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]

Out of curiosity, does this fix https://github.com/apollographql/apollo-client/issues/11165?

[kjhuang-db]

when will this be merged? can we integrate this https://github.com/apollographql/apollo-client/issues/10216 to it as it is critical to decouple streaming data update vs react-render in a lot of use case

[jerelmiller]

@kjhuang-db we won't release this PR until 3.11 which is scheduled for early July. With a big refactor like this, we tend to want to put this in a minor release.

The work to integrate the new option should likely be done in a separate PR to ensure those changes don't get buried along with the rest of this code, but it should absolutely start with the changes from this branch to avoid conflicts.

[kjhuang-db]

@kjhuang-db we won't release this PR until 3.11 which is scheduled for early July. With a big refactor like this, we tend to want to put this in a minor release.

The work to integrate the new option should likely be done in a separate PR to ensure those changes don't get buried along with the rest of this code, but it should absolutely start with the changes from this branch to avoid conflicts.

@jerelmiller sounds good. curious since this change seems switching the state from local to externalStore, with such big refactor why it is not a major release?

• will check out the PR locally, the goal is still to address the issue of re-rendering issue https://github.com/apollographql/react-apollo/issues/3549
  • TLDR, there is a need to decouple react rendering from streaming data update
• the solution is either
  1. follow useMutation to accept ignoreResults https://github.com/apollographql/apollo-client/issues/10216 --- preferred
  2. provide a customized hook useSubscriptionWithCallback.

will branch out from this PR and see if I can add on it: I would prefer this in OSS apollo to avoid a local "copy" in my org's codebase, thanks

---

also a qq for @phryneas: as discussed on Discord, one concern of current useSubscription declarative api (return state data directly) is batching behavior with react 18: the previous usage of useState within useSubscription might get batched (and break consumers' update based on useEffect(, [state]) usage)

1. do you foresee putting return in externalSyncStore will address this issue? from the change in doc, it seems still an issue in 18?

related: https://github.com/facebook/react/issues/25191#issuecomment-1237456448 (what a small world)

[jerelmiller]

@kjhuang-db answered in discord :)

[phryneas]

Let's address #10216 and #11165 in separate follow-up PRs. I've added both to the 3.11.0 milestone.