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

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ### Bug Fixes
 
+- Disable `InMemoryCache` [result object canonization](https://github.com/apollographql/apollo-client/pull/7439) by default, to prevent unexpected memory growth and/or reuse of object references, with multiple ways to reenable it (per-cache, per-query, or a mixture of both). <br/>
+  [@benjamn](https://github.com/benjamn) in [#8822](https://github.com/apollographql/apollo-client/pull/8822)
+
 - Clear `InMemoryCache` `watches` set when `cache.reset()` called. <br/>
   [@benjamn](https://github.com/benjamn) in [#8826](https://github.com/apollographql/apollo-client/pull/8826)
 

scripts/memory/tests.js

@@ -155,13 +155,18 @@ describe("garbage collection", () => {
           },
         },
       },
+      // Explicitly disable canonization to test that it can be overridden.
+      canonizeResults: false,
     });
 
     const client = new ApolloClient({ cache });
 
     (function () {
       const query = gql`query { local }`;
-      const obsQuery = client.watchQuery({ query });
+      const obsQuery = client.watchQuery({
+        query,
+        canonizeResults: true,
+      });
 
       function register(suffix) {
         const reader = cache["storeReader"];
@@ -177,16 +182,18 @@ describe("garbage collection", () => {
             local: "hello",
           });
 
-          assert.strictEqual(
-            cache.readQuery({ query }),
-            result.data,
-          );
+          const read = () => cache.readQuery({
+            query,
+            canonizeResults: true,
+          });
+
+          assert.strictEqual(read(), result.data);
 
           assert.deepStrictEqual(cache.gc(), []);
 
           // Nothing changes because we merely called cache.gc().
           assert.strictEqual(
-            cache.readQuery({ query }),
+            read(),
             result.data,
           );
 
@@ -199,7 +206,7 @@ describe("garbage collection", () => {
 
           register(2);
 
-          const dataAfterResetWithSameCanon = cache.readQuery({ query });
+          const dataAfterResetWithSameCanon = read();
           assert.strictEqual(dataAfterResetWithSameCanon, result.data);
 
           assert.deepStrictEqual(cache.gc({
@@ -211,7 +218,7 @@ describe("garbage collection", () => {
 
           register(3);
 
-          const dataAfterFullReset = cache.readQuery({ query });
+          const dataAfterFullReset = read();
           assert.notStrictEqual(dataAfterFullReset, result.data);
           assert.deepStrictEqual(dataAfterFullReset, result.data);
 

src/cache/core/types/DataProxy.ts

@@ -70,7 +70,7 @@ export namespace DataProxy {
     /**
      * Whether to canonize cache results before returning them. Canonization
      * takes some extra time, but it speeds up future deep equality comparisons.
-     * Defaults to true.
+     * Defaults to false.
      */
     canonizeResults?: boolean;
   }
@@ -91,7 +91,7 @@ export namespace DataProxy {
     /**
      * Whether to canonize cache results before returning them. Canonization
      * takes some extra time, but it speeds up future deep equality comparisons.
-     * Defaults to true.
+     * Defaults to false.
      */
     canonizeResults?: boolean;
   }

src/cache/inmemory/__tests__/cache.ts

@@ -2013,6 +2013,7 @@ describe("InMemoryCache#modify", () => {
 
   it("should allow invalidation using details.INVALIDATE", () => {
     const cache = new InMemoryCache({
+      canonizeResults: true,
       typePolicies: {
         Book: {
           keyFields: ["isbn"],
@@ -2946,22 +2947,31 @@ describe("ReactiveVar and makeVar", () => {
   it("should work with resultCaching disabled (unusual)", () => {
     const { cache, nameVar, query } = makeCacheAndVar(false);
 
-    const result1 = cache.readQuery({ query });
+    const result1 = cache.readQuery({
+      query,
+      canonizeResults: true,
+    });
     expect(result1).toEqual({
       onCall: {
         __typename: "Person",
         name: "Ben",
       },
     });
 
-    const result2 = cache.readQuery({ query });
+    const result2 = cache.readQuery({
+      query,
+      canonizeResults: true,
+    });
     expect(result2).toEqual(result1);
     expect(result2).toBe(result1);
 
     expect(nameVar()).toBe("Ben");
     expect(nameVar("Hugh")).toBe("Hugh");
 
-    const result3 = cache.readQuery({ query });
+    const result3 = cache.readQuery({
+      query,
+      canonizeResults: true,
+    });
     expect(result3).toEqual({
       onCall: {
         __typename: "Person",

src/cache/inmemory/__tests__/entityStore.ts

@@ -170,14 +170,18 @@ describe('EntityStore', () => {
       },
     });
 
-    const resultBeforeGC = cache.readQuery({ query });
+    function read() {
+      return cache.readQuery({ query, canonizeResults: true });
+    }
+
+    const resultBeforeGC = read();
 
     expect(cache.gc().sort()).toEqual([
       'Author:Ray Bradbury',
       'Book:9781451673319',
     ]);
 
-    const resultAfterGC = cache.readQuery({ query });
+    const resultAfterGC = read();
     expect(resultBeforeGC).toBe(resultAfterGC);
 
     expect(cache.extract()).toEqual({
@@ -207,7 +211,7 @@ describe('EntityStore', () => {
       resetResultCache: true,
     })).toEqual([]);
     expect(cache["storeReader"]).not.toBe(originalReader);
-    const resultAfterResetResultCache = cache.readQuery({ query });
+    const resultAfterResetResultCache = read();
     expect(resultAfterResetResultCache).toBe(resultBeforeGC);
     expect(resultAfterResetResultCache).toBe(resultAfterGC);
 
@@ -217,15 +221,15 @@ describe('EntityStore', () => {
       resetResultIdentities: true,
     })).toEqual([]);
 
-    const resultAfterFullGC = cache.readQuery({ query });
+    const resultAfterFullGC = read();
     expect(resultAfterFullGC).toEqual(resultBeforeGC);
     expect(resultAfterFullGC).toEqual(resultAfterGC);
     // These !== relations are triggered by passing resetResultIdentities:true
     // to cache.gc, above.
     expect(resultAfterFullGC).not.toBe(resultBeforeGC);
     expect(resultAfterFullGC).not.toBe(resultAfterGC);
     // Result caching immediately begins working again after the intial reset.
-    expect(cache.readQuery({ query })).toBe(resultAfterFullGC);
+    expect(read()).toBe(resultAfterFullGC);
 
     // Go back to the pre-GC snapshot.
     cache.restore(snapshot);
@@ -1049,6 +1053,7 @@ describe('EntityStore', () => {
     `;
 
     const cache = new InMemoryCache({
+      canonizeResults: true,
       typePolicies: {
         Query: {
           fields: {
@@ -2391,6 +2396,7 @@ describe('EntityStore', () => {
     const isbnsWeHaveRead: string[] = [];
 
     const cache = new InMemoryCache({
+      canonizeResults: true,
       typePolicies: {
         Query: {
           fields: {

src/cache/inmemory/__tests__/optimistic.ts

@@ -6,6 +6,7 @@ describe('optimistic cache layers', () => {
   it('return === results for repeated reads', () => {
     const cache = new InMemoryCache({
       resultCaching: true,
+      canonizeResults: true,
       dataIdFromObject(value: any) {
         switch (value && value.__typename) {
           case 'Book':
@@ -204,6 +205,7 @@ describe('optimistic cache layers', () => {
   it('dirties appropriate IDs when optimistic layers are removed', () => {
     const cache = new InMemoryCache({
       resultCaching: true,
+      canonizeResults: true,
       dataIdFromObject(value: any) {
         switch (value && value.__typename) {
           case 'Book':

src/cache/inmemory/__tests__/policies.ts

@@ -4947,6 +4947,7 @@ describe("type policies", function () {
     function readFirstBookResult() {
       return cache.readQuery<{ author: any }>({
         query: firstBookQuery,
+        canonizeResults: true,
       })!;
     }
 

src/cache/inmemory/__tests__/readFromStore.ts

@@ -1377,6 +1377,7 @@ describe('reading from the store', () => {
 
   withErrorSpy(it, "propagates eviction signals to parent queries", () => {
     const cache = new InMemoryCache({
+      canonizeResults: true,
       typePolicies: {
         Deity: {
           keyFields: ["name"],
@@ -1866,7 +1867,9 @@ describe('reading from the store', () => {
   });
 
   it("returns === results for different queries", function () {
-    const cache = new InMemoryCache;
+    const cache = new InMemoryCache({
+      canonizeResults: true,
+    });
 
     const aQuery: TypedDocumentNode<{
       a: string[];

src/cache/inmemory/inMemoryCache.ts

@@ -13,6 +13,7 @@ import {
   StoreObject,
   Reference,
   isReference,
+  compact,
 } from '../../utilities';
 import {
   ApolloReducerConfig,
@@ -36,6 +37,7 @@ export interface InMemoryCacheConfig extends ApolloReducerConfig {
   possibleTypes?: PossibleTypesMap;
   typePolicies?: TypePolicies;
   resultCacheMaxSize?: number;
+  canonizeResults?: boolean;
 }
 
 type BroadcastOptions = Pick<
@@ -44,13 +46,22 @@ type BroadcastOptions = Pick<
   | "onWatchUpdated"
 >
 
-const defaultConfig: InMemoryCacheConfig = {
+const defaultConfig = {
   dataIdFromObject: defaultDataIdFromObject,
   addTypename: true,
   resultCaching: true,
-  typePolicies: {},
+  // Thanks to the shouldCanonizeResults helper, this should be the only line
+  // you have to change to reenable canonization by default in the future.
+  canonizeResults: false,
 };
 
+export function shouldCanonizeResults(
+  config: Pick<InMemoryCacheConfig, "canonizeResults">,
+): boolean {
+  const value = config.canonizeResults;
+  return value === void 0 ? defaultConfig.canonizeResults : value;
+}
+
 export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
   private data: EntityStore;
   private optimisticData: EntityStore;
@@ -77,7 +88,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
 
   constructor(config: InMemoryCacheConfig = {}) {
     super();
-    this.config = { ...defaultConfig, ...config };
+    this.config = compact(defaultConfig, config);
     this.addTypename = !!this.config.addTypename;
 
     this.policies = new Policies({
@@ -121,6 +132,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
         cache: this,
         addTypename: this.addTypename,
         resultCacheMaxSize: this.config.resultCacheMaxSize,
+        canonizeResults: shouldCanonizeResults(this.config),
         canon: resetResultIdentities
           ? void 0
           : previousReader && previousReader.canon,

src/cache/inmemory/readFromStore.ts

@@ -27,6 +27,7 @@ import {
   maybeDeepFreeze,
   isNonNullObject,
   canUseWeakMap,
+  compact,
 } from '../../utilities';
 import { Cache } from '../core/types/Cache';
 import {
@@ -37,7 +38,7 @@ import {
 import { maybeDependOnExistenceOfEntity, supportsResultCaching } from './entityStore';
 import { getTypenameFromStoreObject } from './helpers';
 import { Policies } from './policies';
-import { InMemoryCache } from './inMemoryCache';
+import { shouldCanonizeResults, InMemoryCache } from './inMemoryCache';
 import { MissingFieldError } from '../core/types/common';
 import { canonicalStringify, ObjectCanon } from './object-canon';
 
@@ -86,6 +87,7 @@ export interface StoreReaderConfig {
   cache: InMemoryCache,
   addTypename?: boolean;
   resultCacheMaxSize?: number;
+  canonizeResults?: boolean;
   canon?: ObjectCanon;
 }
 
@@ -128,6 +130,7 @@ export class StoreReader {
     cache: InMemoryCache,
     addTypename: boolean;
     resultCacheMaxSize?: number;
+    canonizeResults: boolean;
   };
 
   private knownResults = new (
@@ -140,10 +143,10 @@ export class StoreReader {
   }
 
   constructor(config: StoreReaderConfig) {
-    this.config = {
-      ...config,
+    this.config = compact(config, {
       addTypename: config.addTypename !== false,
-    };
+      canonizeResults: shouldCanonizeResults(config),
+    });
 
     this.canon = config.canon || new ObjectCanon;
 
@@ -231,7 +234,7 @@ export class StoreReader {
     rootId = 'ROOT_QUERY',
     variables,
     returnPartialData = true,
-    canonizeResults = true,
+    canonizeResults = this.config.canonizeResults,
   }: DiffQueryAgainstStoreOptions): Cache.DiffResult<T> {
     const policies = this.config.cache.policies;
 

src/core/QueryInfo.ts

@@ -192,13 +192,12 @@ export class QueryInfo {
   }
 
   private getDiffOptions(variables = this.variables): Cache.DiffOptions {
-    const oq = this.observableQuery;
     return {
       query: this.document!,
       variables,
       returnPartialData: true,
       optimistic: true,
-      canonizeResults: !oq || oq.options.canonizeResults !== false,
+      canonizeResults: this.observableQuery?.options.canonizeResults,
     };
   }
 

src/core/__tests__/QueryManager/index.ts

@@ -834,7 +834,7 @@ describe('QueryManager', () => {
   });
 
   itAsync('will return referentially equivalent data if nothing changed in a refetch', (resolve, reject) => {
-    const request = {
+    const request: WatchQueryOptions = {
       query: gql`
         {
           a
@@ -850,6 +850,7 @@ describe('QueryManager', () => {
         }
       `,
       notifyOnNetworkStatusChange: false,
+      canonizeResults: true,
     };
 
     const data1 = {

src/core/watchQueryOptions.ts

@@ -106,7 +106,7 @@ export interface QueryOptions<TVariables = OperationVariables, TData = any> {
   /**
    * Whether to canonize cache results before returning them. Canonization
    * takes some extra time, but it speeds up future deep equality comparisons.
-   * Defaults to true.
+   * Defaults to false.
    */
   canonizeResults?: boolean;
 }