Fix obscure warning in useFragment when cache.identify returns undefined

[jerelmiller]

Fixes #12051

#12020 did some refactoring to useFragment to be more compatible with Rules of React (specifically not writing to ref in render). In the process, useFragment now calls cache.identify directly to ensure that whole data objects passed to from don't trigger rewatches when non key fields change to that object. This change however introduced a regression where invalid identifiers (i.e. cache.identify returns undefined) would show a warning such as:

TypeError: Cannot read properties of undefined (reading '__typename')

This was a result of taking that undefined and passing it as the value to cache.watchFragment, which itself called cache.identify(undefined) and this triggered the warning.

This change ensures that cache.watchFragment forwards undefined to cache.watch as id since it uses ROOT_QUERY as a fallback in that case.

[changeset-bot]

🦋 Changeset detected

Latest commit: e712cc9

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

[github-actions]

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	39.31 KB (+0.01% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	47.99 KB (+0.02% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	45.57 KB (+0.02% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	34.41 KB (+0.02% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	32.29 KB (+0.03% 🔺)
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.21 KB (0%)
import { useQuery } from "dist/react/index.js" (production)	4.3 KB (0%)
import { useLazyQuery } from "dist/react/index.js"	5.69 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production)	4.77 KB (0%)
import { useMutation } from "dist/react/index.js"	3.62 KB (0%)
import { useMutation } from "dist/react/index.js" (production)	2.84 KB (0%)
import { useSubscription } from "dist/react/index.js"	4.42 KB (0%)
import { useSubscription } from "dist/react/index.js" (production)	3.48 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js"	5.49 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production)	4.15 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	4.99 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.64 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.39 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	3.33 KB (0%)
import { useFragment } from "dist/react/index.js"	2.28 KB (0%)
import { useFragment } from "dist/react/index.js" (production)	2.23 KB (0%)

[jerelmiller]

/release:pr

[github-actions]

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

npm i @apollo/[email protected]

[netlify]

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	8fdbb5f
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/66d76ac53cc4c40008d183e3
😎 Deploy Preview	https://deploy-preview-12052--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.

[netlify]

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	b4e5568
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/66d886c0764b1300089d225d
😎 Deploy Preview	https://deploy-preview-12052--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.

[phryneas]

I have to admit, I really don't like the empty string workaround.
Given that watchFragment is still a very young api, could we maybe handle that over there, having it pass on an undefined value?

typeof from === "undefined" ? from : typeof from === "string" ? from : this.identify(from)

[jerelmiller]

Welp, I should have just kept my original changes 🤣. I moved it in 74674ec for a couple reasons:

• Our TypeScript types don't allow for undefined (technically) so this wasn't a supported use case. I believe you would have seen this already if you used cache.watchFragment directly and passed undefined to from (assuming you were using plain JS or ignored the TS errors)
• We were using id! in useFragment which meant we were trying to circumvent what TS was already trying to warn us about (undefined not being supported) so it felt like the fix needed to be on the side of useFragment.

I'm fully willing to admit I'm overthinking this, but felt it a bit odd that our implementation was at odds with our supported types, hence this latest change.

[phryneas]

I see the reasoning - but, tbh., I'd rather add a little comment to the ! - the "special" handling of "" seems more like an accident to me and we might handle that differently at some point in the future, missing that we have to change this code path again.
That might be caused by something as trivial as replacing a || with a (logically much more fitting) ?? and trickle down in very weird ways.

[jerelmiller]

Sounds reasonable enough. Will revert the last commit and add an additional comment for the !

[phryneas]

Looks good to me now!