From ea06ffd2e1f2333b50a464dacd11dcff9bea17b5 Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@apollographql.com>
Date: Thu, 9 Jan 2020 17:19:05 -0500
Subject: [PATCH] Make InMemoryCache#evict remove data from all EntityStore
 layers.

Folks testing AC3 have already reported some confusion about evicted data
seeming to reappear in the context of optimistic mutation updates:
https://github.com/apollographql/apollo-client/issues/5746#issuecomment-570693257

To prevent this confusion, I think we need to honor the force implied by
the word "evict" and remove all traces of the given dataId/fieldName from
all layers the cache, rather than only evicting from the top layer of the
cache (that is, this.optimisticData).
---
 src/cache/inmemory/__tests__/entityStore.ts | 30 +++++++++++++++------
 src/cache/inmemory/entityStore.ts           | 12 +++++++++
 src/cache/inmemory/inMemoryCache.ts         | 12 +++------
 src/cache/inmemory/types.ts                 |  2 +-
 4 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/src/cache/inmemory/__tests__/entityStore.ts b/src/cache/inmemory/__tests__/entityStore.ts
index 63a943bfc7b..9d0514d3be5 100644
--- a/src/cache/inmemory/__tests__/entityStore.ts
+++ b/src/cache/inmemory/__tests__/entityStore.ts
@@ -703,10 +703,8 @@ describe('EntityStore', () => {
         },
         title: "The Cuckoo's Calling",
       },
-      "Author:Robert Galbraith": {
-        __typename: "Author",
-        name: "Robert Galbraith",
-      },
+      // The Robert Galbraith Author record is no longer here because
+      // cache.evict evicts data from all EntityStore layers.
     });
 
     cache.writeFragment({
@@ -721,7 +719,25 @@ describe('EntityStore', () => {
       },
     });
 
-    expect(cache.extract(true)).toEqual(snapshotWithBothNames);
+    expect(cache.extract(true)).toEqual({
+      ROOT_QUERY: {
+        __typename: "Query",
+        book: {
+          __ref: "Book:031648637X",
+        },
+      },
+      "Book:031648637X": {
+        __typename: "Book",
+        author: {
+          __ref: "Author:J.K. Rowling",
+        },
+        title: "The Cuckoo's Calling",
+      },
+      "Author:J.K. Rowling": {
+        __typename: "Author",
+        name: "J.K. Rowling",
+      },
+    });
 
     expect(cache.retain("Author:Robert Galbraith")).toBe(2);
 
@@ -730,9 +746,7 @@ describe('EntityStore', () => {
     expect(cache.release("Author:Robert Galbraith")).toBe(1);
     expect(cache.release("Author:Robert Galbraith")).toBe(0);
 
-    expect(cache.gc()).toEqual([
-      "Author:Robert Galbraith",
-    ]);
+    expect(cache.gc()).toEqual([]);
 
     // If you're ever tempted to do this, you probably want to use cache.clear()
     // instead, but evicting the ROOT_QUERY should work at least.
diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts
index 60f38a8b98c..352bc37738d 100644
--- a/src/cache/inmemory/entityStore.ts
+++ b/src/cache/inmemory/entityStore.ts
@@ -149,8 +149,20 @@ export abstract class EntityStore implements NormalizedCache {
             this.group.dirty(dataId, fieldName);
           });
         }
+
+        return true;
       }
     }
+
+    return false;
+  }
+
+  public evict(dataId: string, fieldName?: string): boolean {
+    let evicted = this.delete(dataId, fieldName);
+    if (this instanceof Layer) {
+      evicted = this.parent.evict(dataId, fieldName) || evicted;
+    }
+    return evicted;
   }
 
   public clear(): void {
diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts
index 834801875f7..c139e43e5ad 100644
--- a/src/cache/inmemory/inMemoryCache.ts
+++ b/src/cache/inmemory/inMemoryCache.ts
@@ -202,15 +202,9 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
   }
 
   public evict(dataId: string, fieldName?: string): boolean {
-    if (this.optimisticData.has(dataId)) {
-      // Note that this deletion does not trigger a garbage collection, which
-      // is convenient in cases where you want to evict multiple entities before
-      // performing a single garbage collection.
-      this.optimisticData.delete(dataId, fieldName);
-      this.broadcastWatches();
-      return !this.optimisticData.has(dataId);
-    }
-    return false;
+    const evicted = this.optimisticData.evict(dataId, fieldName);
+    if (evicted) this.broadcastWatches();
+    return evicted;
   }
 
   public reset(): Promise<void> {
diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts
index 9d0255dbdde..549a145c9a9 100644
--- a/src/cache/inmemory/types.ts
+++ b/src/cache/inmemory/types.ts
@@ -22,7 +22,7 @@ export interface NormalizedCache {
   get(dataId: string): StoreObject;
   getFieldValue(dataId: string, storeFieldName: string): StoreValue;
   merge(dataId: string, incoming: StoreObject): void;
-  delete(dataId: string, fieldName?: string): void;
+  delete(dataId: string, fieldName?: string): boolean;
   clear(): void;
 
   // non-Map elements: