Merge pull request #7439 from apollographql/canonical-cache-results

Efficiently canonicalize InMemoryCache result objects.

CHANGELOG.md

@@ -5,6 +5,9 @@ TBD
 
 ### Improvements
 
+- `InMemoryCache` now _guarantees_ that any two result objects returned by the cache (from `readQuery`, `readFragment`, etc.) will be referentially equal (`===`) if they are deeply equal. Previously, `===` equality was often achievable for results for the same query, on a best-effort basis. Now, equivalent result objects will be automatically shared among the result trees of completely different queries. This guarantee is important for taking full advantage of optimistic updates that correctly guess the final data, and for "pure" UI components that can skip re-rendering when their input data are unchanged. <br/>
+  [@benjamn](https://github.com/benjamn) in [#7439](https://github.com/apollographql/apollo-client/pull/7439)
+
 - Support `client.refetchQueries` as an imperative way to refetch queries, without having to pass `options.refetchQueries` to `client.mutate`. <br/>
   [@dannycochran](https://github.com/dannycochran) in [#7431](https://github.com/apollographql/apollo-client/pull/7431)
 

package.json

@@ -56,7 +56,7 @@
     {
       "name": "apollo-client",
       "path": "./dist/apollo-client.cjs.min.js",
-      "maxSize": "25.5 kB"
+      "maxSize": "25.9 kB"
     }
   ],
   "peerDependencies": {
@@ -77,10 +77,11 @@
     "@types/zen-observable": "^0.8.0",
     "@wry/context": "^0.5.2",
     "@wry/equality": "^0.3.0",
+    "@wry/trie": "^0.2.1",
     "fast-json-stable-stringify": "^2.0.0",
     "graphql-tag": "^2.11.0",
     "hoist-non-react-statics": "^3.3.2",
-    "optimism": "^0.13.1",
+    "optimism": "^0.14.0",
     "prop-types": "^15.7.2",
     "symbol-observable": "^2.0.0",
     "ts-invariant": "^0.6.0",

src/__tests__/client.ts

@@ -3037,11 +3037,9 @@ describe('@connection', () => {
     client.cache.evict({ fieldName: "a" });
     await wait();
 
-    // The results are structurally the same, but the result objects have
-    // been recomputed for queries that involved the ROOT_QUERY.a field.
-    expect(checkLastResult(aResults, a456)).not.toBe(a456);
+    expect(checkLastResult(aResults, a456)).toBe(a456);
     expect(checkLastResult(bResults, bOyez)).toBe(bOyez);
-    expect(checkLastResult(abResults, a456bOyez)).not.toBe(a456bOyez);
+    expect(checkLastResult(abResults, a456bOyez)).toBe(a456bOyez);
 
     const cQuery = gql`{ c }`;
     // Passing cache-only as the fetchPolicy allows the { c: "see" }
@@ -3090,25 +3088,19 @@ describe('@connection', () => {
       { a: 123 },
       { a: 234 },
       { a: 456 },
-      // Delivered again because we explicitly called resetLastResults.
-      { a: 456 },
     ]);
 
     expect(bResults).toEqual([
       { b: "asdf" },
       { b: "ASDF" },
       { b: "oyez" },
-      // Delivered again because we explicitly called resetLastResults.
-      { b: "oyez" },
     ]);
 
     expect(abResults).toEqual([
       { a: 123, b: "asdf" },
       { a: 234, b: "asdf" },
       { a: 234, b: "ASDF" },
       { a: 456, b: "oyez" },
-      // Delivered again because we explicitly called resetLastResults.
-      { a: 456, b: "oyez" },
     ]);
 
     expect(cResults).toEqual([

src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap

@@ -0,0 +1,18 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`reading from the store returns === results for different queries 1`] = `
+Object {
+  "ROOT_QUERY": Object {
+    "__typename": "Query",
+    "a": Array [
+      "a",
+      "y",
+      "y",
+    ],
+    "b": Object {
+      "c": "C",
+      "d": "D",
+    },
+  },
+}
+`;

src/cache/inmemory/__tests__/cache.ts

@@ -1728,8 +1728,8 @@ describe("InMemoryCache#modify", () => {
     })).toBe(false); // Nothing actually modified.
 
     const resultAfterAuthorInvalidation = read();
-    expect(resultAfterAuthorInvalidation).not.toBe(initialResult);
     expect(resultAfterAuthorInvalidation).toEqual(initialResult);
+    expect(resultAfterAuthorInvalidation).toBe(initialResult);
 
     expect(cache.modify({
       id: cache.identify({
@@ -1743,8 +1743,8 @@ describe("InMemoryCache#modify", () => {
     })).toBe(false); // Nothing actually modified.
 
     const resultAfterBookInvalidation = read();
-    expect(resultAfterBookInvalidation).not.toBe(resultAfterAuthorInvalidation);
     expect(resultAfterBookInvalidation).toEqual(resultAfterAuthorInvalidation);
+    expect(resultAfterBookInvalidation).toBe(resultAfterAuthorInvalidation);
     expect(resultAfterBookInvalidation.currentlyReading.author).toEqual({
       __typename: "Author",
       name: "Maria Dahvana Headley",
@@ -2591,9 +2591,8 @@ describe("ReactiveVar and makeVar", () => {
     });
 
     const result2 = cache.readQuery({ query });
-    // Without resultCaching, equivalent results will not be ===.
-    expect(result2).not.toBe(result1);
     expect(result2).toEqual(result1);
+    expect(result2).toBe(result1);
 
     expect(nameVar()).toBe("Ben");
     expect(nameVar("Hugh")).toBe("Hugh");

src/cache/inmemory/__tests__/object-canon.ts

@@ -0,0 +1,108 @@
+import { ObjectCanon } from "../object-canon";
+
+describe("ObjectCanon", () => {
+  it("can canonicalize objects and arrays", () => {
+    const canon = new ObjectCanon;
+
+    const obj1 = {
+      a: [1, 2],
+      b: {
+        c: [{
+          d: "dee",
+          e: "ee",
+        }, "f"],
+        g: "gee",
+      },
+    };
+
+    const obj2 = {
+      b: {
+        g: "gee",
+        c: [{
+          e: "ee",
+          d: "dee",
+        }, "f"],
+      },
+      a: [1, 2],
+    };
+
+    expect(obj1).toEqual(obj2);
+    expect(obj1).not.toBe(obj2);
+
+    const c1 = canon.admit(obj1);
+    const c2 = canon.admit(obj2);
+
+    expect(c1).toBe(c2);
+    expect(c1).toEqual(obj1);
+    expect(c1).toEqual(obj2);
+    expect(c2).toEqual(obj1);
+    expect(c2).toEqual(obj2);
+    expect(c1).not.toBe(obj1);
+    expect(c1).not.toBe(obj2);
+    expect(c2).not.toBe(obj1);
+    expect(c2).not.toBe(obj2);
+
+    expect(canon.admit(c1)).toBe(c1);
+    expect(canon.admit(c2)).toBe(c2);
+  });
+
+  it("preserves custom prototypes", () => {
+    const canon = new ObjectCanon;
+
+    class Custom {
+      constructor(public value: any) {}
+      getValue() { return this.value }
+    }
+
+    const customs = [
+      new Custom("oyez"),
+      new Custom(1234),
+      new Custom(true),
+    ];
+
+    const admitted = canon.admit(customs);
+    expect(admitted).not.toBe(customs);
+    expect(admitted).toEqual(customs);
+
+    function check(i: number) {
+      expect(admitted[i]).toEqual(customs[i]);
+      expect(admitted[i]).not.toBe(customs[i]);
+      expect(admitted[i].getValue()).toBe(customs[i].getValue());
+      expect(Object.getPrototypeOf(admitted[i])).toBe(Custom.prototype);
+      expect(admitted[i]).toBeInstanceOf(Custom);
+    }
+    check(0);
+    check(1);
+    check(2);
+
+    expect(canon.admit(customs)).toBe(admitted);
+  });
+
+  it("unwraps Pass wrappers as-is", () => {
+    const canon = new ObjectCanon;
+
+    const cd = {
+      c: "see",
+      d: "dee",
+    };
+
+    const obj = {
+      a: cd,
+      b: canon.pass(cd),
+      e: cd,
+    };
+
+    function check() {
+      const admitted = canon.admit(obj);
+      expect(admitted).not.toBe(obj);
+      expect(admitted.b).toBe(cd);
+      expect(admitted.e).toEqual(cd);
+      expect(admitted.e).not.toBe(cd);
+      expect(admitted.e).toEqual(admitted.b);
+      expect(admitted.e).not.toBe(admitted.b);
+      expect(admitted.e).toBe(admitted.a);
+    }
+    check();
+    check();
+  });
+});

src/cache/inmemory/__tests__/optimistic.ts

@@ -431,7 +431,7 @@ describe('optimistic cache layers', () => {
 
     const resultAfterRemovingBuzzLayer = readWithAuthors();
     expect(resultAfterRemovingBuzzLayer).toEqual(resultWithBuzz);
-    expect(resultAfterRemovingBuzzLayer).not.toBe(resultWithBuzz);
+    expect(resultAfterRemovingBuzzLayer).toBe(resultWithBuzz);
     resultWithTwoAuthors.books.forEach((book, i) => {
       expect(book).toEqual(resultAfterRemovingBuzzLayer.books[i]);
       expect(book).toBe(resultAfterRemovingBuzzLayer.books[i]);

src/cache/inmemory/__tests__/policies.ts

@@ -4692,19 +4692,8 @@ describe("type policies", function () {
     });
 
     const thirdFirstBookResult = readFirstBookResult();
-
-    // A change in VW's books field triggers rereading of result objects
-    // that previously involved her books field.
-    expect(thirdFirstBookResult).not.toBe(secondFirstBookResult);
-
-    // However, since the new Book was not the earliest published, the
-    // second and third results are structurally the same.
     expect(thirdFirstBookResult).toEqual(secondFirstBookResult);
-
-    // In fact, the original author.firstBook object has been reused!
-    expect(thirdFirstBookResult.author.firstBook).toBe(
-      secondFirstBookResult.author.firstBook,
-    );
+    expect(thirdFirstBookResult).toBe(secondFirstBookResult);
   });
 
   it("readField can read fields with arguments", function () {