From 532de448c25db2e3cbcc05675237a0e38e6dc00a Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@apollographql.com>
Date: Tue, 16 Mar 2021 19:27:11 -0400
Subject: [PATCH] Add Cache.WatchOptions["lastDiff"] deduplication to
 broadcastWatch.

The goal of broadcastWatches is to notify cache watchers of any new data
resulting from cache writes.

However, it's possible for cache writes to invalidate watched queries in a
way that does not result in any differences in the resulting data, so this
watch.lastDiff caching saves us from triggering a redundant broadcast of
exactly the same data again.

Note: thanks to #7439, when two result objects are deeply equal to each
another, they will automatically also be === to each other, which is what
allows us to get away with the !== check in this code.
---
 src/cache/core/types/Cache.ts                 |  1 +
 src/cache/inmemory/__tests__/cache.ts         | 27 ++++---------------
 src/cache/inmemory/__tests__/readFromStore.ts | 23 ++++++++--------
 src/cache/inmemory/inMemoryCache.ts           |  4 ++-
 4 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts
index 281e0a1599a..6389d121bd5 100644
--- a/src/cache/core/types/Cache.ts
+++ b/src/cache/core/types/Cache.ts
@@ -28,6 +28,7 @@ export namespace Cache {
   export interface WatchOptions extends ReadOptions {
     immediate?: boolean;
     callback: WatchCallback;
+    lastDiff?: DiffResult<any>;
   }
 
   export interface EvictOptions {
diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts
index 5206b2ff00c..6545a68dba1 100644
--- a/src/cache/inmemory/__tests__/cache.ts
+++ b/src/cache/inmemory/__tests__/cache.ts
@@ -1492,22 +1492,11 @@ describe('Cache', () => {
       expect(dirtied.has(abInfo.watch)).toBe(true);
       expect(dirtied.has(bInfo.watch)).toBe(false);
 
-      expect(last(aInfo.diffs)).toEqual({
-        complete: true,
-        result: {
-          a: "ay",
-        },
-      });
-
-      expect(last(abInfo.diffs)).toEqual({
-        complete: true,
-        result: {
-          a: "ay",
-          b: "bee",
-        },
-      });
-
-      expect(bInfo.diffs.length).toBe(0);
+      // No new diffs should have been generated, since we only invalidated
+      // fields using cache.modify, and did not change any field values.
+      expect(aInfo.diffs).toEqual([]);
+      expect(abInfo.diffs).toEqual([]);
+      expect(bInfo.diffs).toEqual([]);
 
       aInfo.cancel();
       abInfo.cancel();
@@ -1686,7 +1675,6 @@ describe("InMemoryCache#broadcastWatches", function () {
     expect(receivedCallbackResults).toEqual([
       received1,
       // New results:
-      received1,
       received2,
     ]);
 
@@ -1714,7 +1702,6 @@ describe("InMemoryCache#broadcastWatches", function () {
     }];
 
     expect(receivedCallbackResults).toEqual([
-      received1,
       received1,
       received2,
       // New results:
@@ -1734,16 +1721,12 @@ describe("InMemoryCache#broadcastWatches", function () {
     }];
 
     expect(receivedCallbackResults).toEqual([
-      received1,
       received1,
       received2,
       received3,
       received4,
       // New results:
-      received1,
       received2AllCaps,
-      received3,
-      received4,
     ]);
   });
 });
diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts
index b546b4d1f5a..2a25c064df9 100644
--- a/src/cache/inmemory/__tests__/readFromStore.ts
+++ b/src/cache/inmemory/__tests__/readFromStore.ts
@@ -1419,10 +1419,10 @@ describe('reading from the store', () => {
 
     const diffs: Cache.DiffResult<any>[] = [];
 
-    function watch() {
+    function watch(immediate = true) {
       return cache.watch({
         query: rulerQuery,
-        immediate: true,
+        immediate,
         optimistic: true,
         callback(diff) {
           diffs.push(diff);
@@ -1764,14 +1764,10 @@ describe('reading from the store', () => {
       diffWithZeusAsRuler,
     ]);
 
-    // Rewatch the rulerQuery, which will populate the same diffs array
-    // that we were using before.
-    const cancel2 = watch();
-
-    const diffWithApolloAsRuler = {
-      complete: true,
-      result: apolloRulerResult,
-    };
+    // Rewatch the rulerQuery, but avoid delivering an immediate initial
+    // result (by passing false), so that we can use cache.modify to
+    // trigger the delivery of diffWithApolloAsRuler below.
+    const cancel2 = watch(false);
 
     expect(diffs).toEqual([
       initialDiff,
@@ -1779,7 +1775,6 @@ describe('reading from the store', () => {
       diffWithoutDevouredSons,
       diffWithChildrenOfZeus,
       diffWithZeusAsRuler,
-      diffWithApolloAsRuler,
     ]);
 
     cache.modify({
@@ -1797,6 +1792,11 @@ describe('reading from the store', () => {
 
     cancel2();
 
+    const diffWithApolloAsRuler = {
+      complete: true,
+      result: apolloRulerResult,
+    };
+
     // The cache.modify call should have triggered another diff, since we
     // overwrote the ROOT_QUERY.ruler field with a valid Reference to the
     // Apollo entity object.
@@ -1807,7 +1807,6 @@ describe('reading from the store', () => {
       diffWithChildrenOfZeus,
       diffWithZeusAsRuler,
       diffWithApolloAsRuler,
-      diffWithApolloAsRuler,
     ]);
 
     expect(
diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts
index dc62df90e1e..99b0107880b 100644
--- a/src/cache/inmemory/inMemoryCache.ts
+++ b/src/cache/inmemory/inMemoryCache.ts
@@ -446,6 +446,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
       }
     }
 
-    c.callback(diff);
+    if (!c.lastDiff || c.lastDiff.result !== diff.result) {
+      c.callback(c.lastDiff = diff);
+    }
   }
 }