Prevent optimistic cache evictions from evicting non-optimistic data.

CHANGELOG.md

@@ -8,6 +8,9 @@
 - Stop excluding observerless queries from `refetchQueries: [...]` selection. <br/>
   [@benjamn](https://github.com/benjamn) in [#8825](https://github.com/apollographql/apollo-client/pull/8825)
 
+- Prevent optimistic cache evictions from evicting non-optimistic data. <br/>
+  [@benjamn](https://github.com/benjamn) in [#8829](https://github.com/apollographql/apollo-client/pull/8829)
+
 ## Apollo Client 3.4.13
 
 ### Bug Fixes

src/cache/inmemory/__tests__/optimistic.ts

@@ -443,4 +443,161 @@ describe('optimistic cache layers', () => {
     const resultWithoutOptimisticLayers = readWithAuthors();
     expect(resultWithoutOptimisticLayers).toBe(nonOptimisticResult);
   });
+
+  describe("eviction during optimistic updates", function () {
+    it("should not evict from lower layers", function () {
+      const cache = new InMemoryCache();
+
+      const query = gql`
+        query {
+          counter {
+            value
+          }
+        }
+      `;
+
+      function write(value: number) {
+        cache.writeQuery({
+          query,
+          data: {
+            counter: {
+              value,
+            },
+          },
+        });
+      }
+
+      function expectOptimisticCount(value: number) {
+        expect(cache.readQuery({
+          query,
+          optimistic: true,
+        })).toEqual({
+          counter: {
+            value,
+          },
+        });
+
+        expect(cache.extract(true)).toEqual({
+          ROOT_QUERY: {
+            __typename: "Query",
+            counter: {
+              value,
+            },
+          },
+        });
+      }
+
+      function expectNonOptimisticCount(value: number) {
+        // Reading non-optimistically returns the original non-optimistic data.
+        expect(cache.readQuery({
+          query,
+          optimistic: false,
+        })).toEqual({
+          counter: {
+            value,
+          },
+        });
+
+        // Extracting non-optimistically shows Query.counter === 0 again.
+        expect(cache.extract(false)).toEqual({
+          ROOT_QUERY: {
+            __typename: "Query",
+            counter: {
+              value,
+            },
+          },
+        });
+      }
+
+      write(0);
+      expectOptimisticCount(0);
+      expectNonOptimisticCount(0);
+
+      cache.batch({
+        optimistic: "layer 1",
+        update() {
+          write(1);
+          expectOptimisticCount(1);
+          // Within the update function, non-optimistic cache reads come from
+          // the current optimistic layer, so we read 1 here instead of 0.
+          expectNonOptimisticCount(1);
+        },
+      });
+      expectOptimisticCount(1);
+      // Now that we're out of the update function, the non-optimistic data is
+      // back to looking as it always did.
+      expectNonOptimisticCount(0);
+
+      cache.batch({
+        optimistic: "layer 2",
+        update() {
+          write(2);
+          expectOptimisticCount(2);
+        },
+      });
+      expectOptimisticCount(2);
+
+      cache.batch({
+        optimistic: "layer 3",
+        update() {
+          write(3);
+          expectOptimisticCount(3);
+
+          expect(cache.evict({
+            fieldName: "counter",
+          })).toBe(true);
+
+          expectOptimisticEviction();
+        },
+      });
+
+      function expectOptimisticEviction() {
+        // Reading optimistically fails because the data have been evicted,
+        // though only optimistically.
+        expect(cache.readQuery({
+          query,
+          optimistic: true,
+        })).toBe(null);
+
+        // Extracting optimistically shows Query.counter undefined.
+        expect(cache.extract(true)).toEqual({
+          ROOT_QUERY: {
+            __typename: "Query",
+            counter: void 0,
+          },
+        });
+      }
+
+      expectOptimisticEviction();
+
+      cache.removeOptimistic("layer 2");
+
+      // Nothing changes because "layer 2" was not the top layer.
+      expectOptimisticEviction();
+
+      // Original data still intact, of course.
+      expectNonOptimisticCount(0);
+
+      cache.removeOptimistic("layer 3");
+
+      // Since we removed layers 2 and then 3, only layer 1 is left.
+      expectOptimisticCount(1);
+      expectNonOptimisticCount(0);
+
+      // Since this eviction is not happening inside an optimistic update
+      // function, it evicts the Query.counter field from both the optimistic
+      // layer (1) and the root layer.
+      expect(cache.evict({
+        fieldName: "counter",
+      })).toBe(true);
+
+      expectOptimisticEviction();
+
+      cache.removeOptimistic("layer 1");
+
+      // There are no optimistic layers now, but the root/non-optimistic layer
+      // also exhibits the eviction.
+      expectOptimisticEviction();
+    });
+  });
 });

src/cache/inmemory/entityStore.ts

@@ -291,14 +291,17 @@ export abstract class EntityStore implements NormalizedCache {
     return false;
   }
 
-  public evict(options: Cache.EvictOptions): boolean {
+  public evict(
+    options: Cache.EvictOptions,
+    limit: EntityStore,
+  ): boolean {
     let evicted = false;
     if (options.id) {
       if (hasOwn.call(this.data, options.id)) {
         evicted = this.delete(options.id, options.fieldName, options.args);
       }
-      if (this instanceof Layer) {
-        evicted = this.parent.evict(options) || evicted;
+      if (this instanceof Layer && this !== limit) {
+        evicted = this.parent.evict(options, limit) || evicted;
       }
       // Always invalidate the field to trigger rereading of watched
       // queries, even if no cache data was modified by the eviction,

src/cache/inmemory/inMemoryCache.ts

@@ -353,7 +353,10 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
       // this.txCount still seems like a good idea, for uniformity with
       // the other update methods.
       ++this.txCount;
-      return this.optimisticData.evict(options);
+      // Pass this.data as a limit on the depth of the eviction, so evictions
+      // during optimistic updates (when this.data is temporarily set equal to
+      // this.optimisticData) do not escape their optimistic Layer.
+      return this.optimisticData.evict(options, this.data);
     } finally {
       if (!--this.txCount && options.broadcast !== false) {
         this.broadcastWatches();