documentTransform: use optimism and WeakCache

[phryneas]

This goes one step further than #11388 and swaps a lot of the logic for a call to optimism (assuming the version allowing to swap out the Cache class releases soon)

Checklist:

• If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests

[changeset-bot]

🦋 Changeset detected

Latest commit: baafd93

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	37.73 KB (-0.04% 🔽)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	44.22 KB (+0.01% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	42.71 KB (+0.03% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	32.87 KB (+0.04% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	31.56 KB (+0.05% 🔺)
import { ApolloProvider } from "dist/react/index.js"	1.23 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production)	1.21 KB (0%)
import { useQuery } from "dist/react/index.js"	4.33 KB (0%)
import { useQuery } from "dist/react/index.js" (production)	4.15 KB (0%)
import { useLazyQuery } from "dist/react/index.js"	4.63 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production)	4.46 KB (0%)
import { useMutation } from "dist/react/index.js"	2.6 KB (0%)
import { useMutation } from "dist/react/index.js" (production)	2.58 KB (0%)
import { useSubscription } from "dist/react/index.js"	2.29 KB (0%)
import { useSubscription } from "dist/react/index.js" (production)	2.24 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js"	4.29 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production)	3.7 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	3.8 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.2 KB (0%)
import { useLoadableQuery } from "dist/react/index.js"	4.07 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production)	3.47 KB (0%)
import { useReadQuery } from "dist/react/index.js"	3 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	2.95 KB (0%)
import { useFragment } from "dist/react/index.js"	2.1 KB (0%)
import { useFragment } from "dist/react/index.js" (production)	2.05 KB (0%)

[phryneas]

The big downside of this PR: this method vanishes. I'm not sure how much we did consider it part of our external API.

@jerelmiller what are your thoughts?

[jerelmiller]

Ah this should be marked as a private method and I think was just an oversight when we released this.

The getCacheKey option passed to the constructor should be the public API in this case, so this should be fine to remove.

[jerelmiller]

I want to put out a word of caution with this based on a previous experience I had using `wrap.

When developing the SuspenseCache in the initial version of useSuspenseQuery, I used optimism's wrap function in order to handle the caching of the (what now is) query references. I ended up with a lot of issues due to the way dirtying a wrapped function would invalidate wrapped function further down the call stack, of which the suspense cache function was one of them (even though they were very distant from each other). There would be an operation in InMemoryCache that would be executed, which would dirty a function, then cause my SuspenseCache entries to be lost since it thought it needed to recompute everything.

This change may very well be fine, but I think we need to make sure that we don't inadvertently invalidate the internal cache. I think most of my tests around the document transforms are verifying the operation was sent to the server correctly, but I don't know that I have any test that does something with the in memory cache that would cause it to recompute wrapped functions. Perhaps you can add a case here for that?

[jerelmiller]

Will do a full review, but chatted in person and decided optimism is fine moving forward.

[jerelmiller]

You can use the stable 0.18.0 version that is now released. This was merged into main already though so merging main -> release-3.9 -> this branch should resolve this.

[jerelmiller]

Perhaps we just need to merge release-3.9 back into this branch? Odd this made it into the diff.

[jerelmiller]

Changes look great! I think some of the weird stuff will get auto fixed by merging release-3.9 into this one, so I'm not concerned.