Make InMemoryCache#evict remove data from all EntityStore layers. (#5773

)

Folks testing AC3 (👋 @ahrnee) have already reported some confusion about
evicted data seeming to reappear in the context of optimistic mutation updates:
#5746 (comment)

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
(namely, `this.optimisticData`).

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.

src/cache/inmemory/entityStore.ts

@@ -154,8 +154,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 {

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> {

src/cache/inmemory/types.ts

@@ -22,7 +22,7 @@ export interface NormalizedCache {
   get(dataId: string): StoreObject;
   get(dataId: string, fieldName: 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: