Disable InMemoryCache canonization by default to prevent unexpected memory growth/sharing, with options to reenable

[benjamn]

Making InMemoryCache result canonization (#7439) opt-in instead of opt-out (as it currently is) should prevent the surprises of shared object references reported in #8717, and fixes the apparent memory leak in #8784, as confirmed using @CollinMonahanLab49's reproduction.

Given that canonization was a headline feature of v3.4, shipping this PR would be something of a retraction, but I believe it's the simplest way to prevent surprise and encourage using canonization (only) where it's actually useful.

Code that benefits from canonization can continue using it! In fact, there are multiple ways to override the new default behavior, with different scopes:

// This cache will canonize everything by default, unless
// specific reads request canonizeResults:false.
const cache = new InMemoryCache({
  canonizeResults: true,
});

// Override the setting for individual cache reads.
cache.readQuery({ query, canonizeResults: true });
cache.readFragment({ id, fragment, canonizeResults: false });

// Results for this query will be canonized even if canonization
// is disabled elsewhere.
const result = useQuery(query, {
  canonizeResults: true,
});

// This ObservableQuery's results will always be canonized.
const observableQuery = client.watchQuery({
  query,
  canonizeResults: true,
});

// Set the default canonizeResults option for all watchQuery
// calls (including useQuery). Passing canonizeResults:true
// to InMemoryCache may be more reliable than this.
const client = new ApolloClient({
  defaultOptions: {
    watchQuery: {
      canonizeResults: true,
    },
  },
});

I had to update a number of tests after changing the default canonization behavior, but I was able to fix every one of them using one of the tricks demonstrated above, which is reassuring.

Though the consequences of canonization (memory usage and poor fit with certain query workloads) all have good explanations, I would rather folks engage with those tradeoffs because they want to optimize specific parts of their application, rather than inheriting the consequences by default (without asking for them).

[brainkim]

Lessons learned!

[KeithGillette]

I don't have any deep insight into this, but having been bitten by the switch to default canonization, I like this change, as having to opt-in would have probably inspired the scrutiny to surface the issue (#8717) it caused us before production release. The global opt-in with individual query query opt-in/out makes it easy to take advantage of the benefits of canonization and flexible enough to avoid the errors it can cause in individual cases.

[jheysen]

Same as above, I like the new default and I could even argue that the previous implementation was a semver-major change, with this reverting the change to semver-minor. An example of this is the transition that node-postgres implemented between v7 and v8, regarding SSL cert validation.

A suggestion to analyze could be to canonize objects, but not arrays. Since object arrays are only a list of references to the objects living elsewhere on the heap, if the same object is referenced more than once in the response, the memory benefits could be largely obtained while not breaking many applications that seek to work with the arrays in the responses.

[brainkim]

LGTM

[matekb]

Hello,
I too have problems with the new behavior. Do you know when a patch release containing this fix will be released?
Thanks for the good work with Apollo!
Mattias

[benjamn]

@matekb Today!