Merge pull request #5153 from apollographql/always-freeze-cache-results

Remove freezeResults option, assuming always true.
apollographql · Aug 21, 2019 · ad1fedb · ad1fedb
2 parents 33d9dcc + af94014
commit ad1fedb
Show file tree

Hide file tree

Showing 10 changed files with 109 additions and 182 deletions.
diff --git a/packages/apollo-cache-inmemory/src/__tests__/__snapshots__/cache.ts.snap b/packages/apollo-cache-inmemory/src/__tests__/__snapshots__/cache.ts.snap
@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/3) 1`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 1`] = `
 Object {
   "bar": Object {
     "i": 7,
@@ -14,7 +14,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/3) 2`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 2`] = `
 Object {
   "bar": Object {
     "i": 7,
@@ -32,7 +32,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/3) 3`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 3`] = `
 Object {
   "bar": Object {
     "i": 10,
@@ -50,7 +50,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/3) 4`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 4`] = `
 Object {
   "bar": Object {
     "i": 10,
@@ -68,7 +68,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/3) 5`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 5`] = `
 Object {
   "bar": Object {
     "i": 7,
@@ -86,7 +86,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/3) 6`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 6`] = `
 Object {
   "bar": Object {
     "i": 10,
@@ -104,7 +104,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/3) 1`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 1`] = `
 Object {
   "bar": Object {
     "i": 7,
@@ -118,7 +118,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/3) 2`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 2`] = `
 Object {
   "bar": Object {
     "i": 7,
@@ -136,7 +136,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/3) 3`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 3`] = `
 Object {
   "bar": Object {
     "i": 10,
@@ -154,7 +154,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/3) 4`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 4`] = `
 Object {
   "bar": Object {
     "i": 10,
@@ -172,7 +172,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/3) 5`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 5`] = `
 Object {
   "bar": Object {
     "i": 7,
@@ -190,111 +190,7 @@ Object {
 }
 `;
 
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/3) 6`] = `
-Object {
-  "bar": Object {
-    "i": 10,
-    "j": 11,
-    "k": 12,
-  },
-  "foo": Object {
-    "e": 4,
-    "f": 5,
-    "g": 6,
-    "h": Object {
-      "__ref": "bar",
-    },
-  },
-}
-`;
-
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (3/3) 1`] = `
-Object {
-  "bar": Object {
-    "i": 7,
-  },
-  "foo": Object {
-    "e": 4,
-    "h": Object {
-      "__ref": "bar",
-    },
-  },
-}
-`;
-
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (3/3) 2`] = `
-Object {
-  "bar": Object {
-    "i": 7,
-    "j": 8,
-    "k": 9,
-  },
-  "foo": Object {
-    "e": 4,
-    "f": 5,
-    "g": 6,
-    "h": Object {
-      "__ref": "bar",
-    },
-  },
-}
-`;
-
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (3/3) 3`] = `
-Object {
-  "bar": Object {
-    "i": 10,
-    "j": 8,
-    "k": 9,
-  },
-  "foo": Object {
-    "e": 4,
-    "f": 5,
-    "g": 6,
-    "h": Object {
-      "__ref": "bar",
-    },
-  },
-}
-`;
-
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (3/3) 4`] = `
-Object {
-  "bar": Object {
-    "i": 10,
-    "j": 11,
-    "k": 12,
-  },
-  "foo": Object {
-    "e": 4,
-    "f": 5,
-    "g": 6,
-    "h": Object {
-      "__ref": "bar",
-    },
-  },
-}
-`;
-
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (3/3) 5`] = `
-Object {
-  "bar": Object {
-    "i": 7,
-    "j": 8,
-    "k": 9,
-  },
-  "foo": Object {
-    "e": 4,
-    "f": 5,
-    "g": 6,
-    "h": Object {
-      "__ref": "bar",
-    },
-  },
-}
-`;
-
-exports[`Cache writeFragment will write some deeply nested data into the store at any id (3/3) 6`] = `
+exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 6`] = `
 Object {
   "bar": Object {
     "i": 10,

diff --git a/packages/apollo-cache-inmemory/src/__tests__/cache.ts b/packages/apollo-cache-inmemory/src/__tests__/cache.ts
@@ -24,12 +24,6 @@ describe('Cache', () => {
           resultCaching: false,
         }).restore(cloneDeep(data)),
       ),
-      initialDataForCaches.map(data =>
-        new InMemoryCache({
-          addTypename: false,
-          freezeResults: true,
-        }).restore(cloneDeep(data)),
-      ),
     ];
 
     cachesList.forEach((caches, i) => {
@@ -55,11 +49,6 @@ describe('Cache', () => {
         ...config,
         resultCaching: false,
       }),
-      new InMemoryCache({
-        addTypename: false,
-        ...config,
-        freezeResults: true,
-      }),
     ];
 
     caches.forEach((cache, i) => {

diff --git a/packages/apollo-cache-inmemory/src/__tests__/roundtrip.ts b/packages/apollo-cache-inmemory/src/__tests__/roundtrip.ts
@@ -22,7 +22,6 @@ function assertDeeplyFrozen(value: any, stack: any[] = []) {
 
 function storeRoundtrip(query: DocumentNode, result: any, variables = {}) {
   const reader = new StoreReader();
-  const immutableReader = new StoreReader({ freezeResults: true });
   const writer = new StoreWriter();
 
   const store = writer.writeQueryToStore({
@@ -45,9 +44,9 @@ function storeRoundtrip(query: DocumentNode, result: any, variables = {}) {
   expect(store).toBeInstanceOf(EntityCache);
   expect(reader.readQueryFromStore(readOptions)).toBe(reconstructedResult);
 
-  const immutableResult = immutableReader.readQueryFromStore(readOptions);
+  const immutableResult = reader.readQueryFromStore(readOptions);
   expect(immutableResult).toEqual(reconstructedResult);
-  expect(immutableReader.readQueryFromStore(readOptions)).toBe(immutableResult);
+  expect(reader.readQueryFromStore(readOptions)).toBe(immutableResult);
   if (process.env.NODE_ENV !== 'production') {
     try {
       // Note: this illegal assignment will only throw in strict mode, but that's
@@ -234,8 +233,8 @@ describe('roundtrip', () => {
       },
     );
 
-    // Just because we read from the store using { freezeResults: true }, the
-    // original data should not be frozen.
+    // Reading immutable results from the store does not mean the original
+    // data should get frozen.
     expect(Object.isExtensible(updateClub)).toBe(true);
     expect(Object.isFrozen(updateClub)).toBe(false);
   });

diff --git a/packages/apollo-cache-inmemory/src/__tests__/writeToStore.ts b/packages/apollo-cache-inmemory/src/__tests__/writeToStore.ts
@@ -20,6 +20,7 @@ import { StoreWriter } from '../writeToStore';
 import { defaultNormalizedCacheFactory } from '../entityCache';
 
 import { makeReference } from '../helpers';
+import { InMemoryCache } from '../inMemoryCache';
 
 export function withWarning(func: Function, regex?: RegExp) {
   let message: string = null as never;
@@ -1862,4 +1863,40 @@ describe('writing to the store', () => {
       },
     });
   });
+
+  it('should not deep-freeze scalar objects', () => {
+    const query = gql`
+      query {
+        scalarFieldWithObjectValue
+      }
+    `;
+
+    const scalarObject = {
+      a: 1,
+      b: [2, 3],
+      c: {
+        d: 4,
+        e: 5,
+      },
+    };
+
+    const cache = new InMemoryCache();
+
+    cache.writeQuery({
+      query,
+      data: {
+        scalarFieldWithObjectValue: scalarObject,
+      },
+    });
+
+    expect(Object.isFrozen(scalarObject)).toBe(false);
+    expect(Object.isFrozen(scalarObject.b)).toBe(false);
+    expect(Object.isFrozen(scalarObject.c)).toBe(false);
+
+    const result = cache.readQuery<any>({ query });
+    expect(result.scalarFieldWithObjectValue).not.toBe(scalarObject);
+    expect(Object.isFrozen(result.scalarFieldWithObjectValue)).toBe(true);
+    expect(Object.isFrozen(result.scalarFieldWithObjectValue.b)).toBe(true);
+    expect(Object.isFrozen(result.scalarFieldWithObjectValue.c)).toBe(true);
+  });
 });
diff --git a/packages/apollo-cache-inmemory/src/inMemoryCache.ts b/packages/apollo-cache-inmemory/src/inMemoryCache.ts
@@ -20,14 +20,12 @@ import { KeyTrie } from 'optimism';
 
 export interface InMemoryCacheConfig extends ApolloReducerConfig {
   resultCaching?: boolean;
-  freezeResults?: boolean;
 }
 
 const defaultConfig: InMemoryCacheConfig = {
   dataIdFromObject: defaultDataIdFromObject,
   addTypename: true,
   resultCaching: true,
-  freezeResults: false,
 };
 
 export function defaultDataIdFromObject(result: any): string | null {
@@ -92,7 +90,6 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
 
     this.storeReader = new StoreReader({
       cacheKeyRoot: this.cacheKeyRoot,
-      freezeResults: config.freezeResults,
       possibleTypes: this.possibleTypes,
     });
 

diff --git a/packages/apollo-cache-inmemory/src/readFromStore.ts b/packages/apollo-cache-inmemory/src/readFromStore.ts
@@ -98,17 +98,14 @@ type ExecSubSelectedArrayOptions = {
 type PossibleTypes = import('./inMemoryCache').InMemoryCache['possibleTypes'];
 export interface StoreReaderConfig {
   cacheKeyRoot?: KeyTrie<object>;
-  freezeResults?: boolean;
   possibleTypes?: PossibleTypes;
 }
 
 export class StoreReader {
-  private freezeResults: boolean;
   private possibleTypes?: PossibleTypes;
 
   constructor({
     cacheKeyRoot = new KeyTrie<object>(canUseWeakMap),
-    freezeResults = false,
     possibleTypes,
   }: StoreReaderConfig = {}) {
     const {
@@ -117,7 +114,6 @@ export class StoreReader {
       executeSubSelectedArray,
     } = this;
 
-    this.freezeResults = freezeResults;
     this.possibleTypes = possibleTypes;
 
     this.executeStoreQuery = wrap((options: ExecStoreQueryOptions) => {
@@ -391,7 +387,7 @@ export class StoreReader {
     // defensive shallow copies than necessary.
     finalResult.result = mergeDeepArray(objectsToMerge);
 
-    if (this.freezeResults && process.env.NODE_ENV !== 'production') {
+    if (process.env.NODE_ENV !== 'production') {
       Object.freeze(finalResult.result);
     }
 
@@ -437,9 +433,7 @@ export class StoreReader {
     if (!field.selectionSet) {
       if (process.env.NODE_ENV !== 'production') {
         assertSelectionSetForIdValue(contextValue.store, field, readStoreResult.result);
-        if (this.freezeResults) {
-          maybeDeepFreeze(readStoreResult);
-        }
+        maybeDeepFreeze(readStoreResult);
       }
       return readStoreResult;
     }
@@ -525,7 +519,7 @@ export class StoreReader {
       return item;
     });
 
-    if (this.freezeResults && process.env.NODE_ENV !== 'production') {
+    if (process.env.NODE_ENV !== 'production') {
       Object.freeze(array);
     }