Consider cache.modify a destructive method, like cache.evict.

CHANGELOG.md

@@ -16,8 +16,10 @@
 - Allow `options.nextFetchPolicy` to be a function that takes the current `FetchPolicy` and returns a new (or the same) `FetchPolicy`, making `nextFetchPolicy` more suitable for global use in `defaultOptions.watchQuery`. <br/>
   [@benjamn](https://github.com/benjamn) in [#6893](https://github.com/apollographql/apollo-client/pull/6893)
 
-- Disable feud-stopping logic after any cache eviction. <br/>
-  [@benjamn](https://github.com/benjamn) in [#6817](https://github.com/apollographql/apollo-client/pull/6817)
+- Disable feud-stopping logic after any `cache.evict` or `cache.modify` operation. <br/>
+  [@benjamn](https://github.com/benjamn) in
+  [#6817](https://github.com/apollographql/apollo-client/pull/6817) and
+  [#6898](https://github.com/apollographql/apollo-client/pull/6898)
 
 - Prevent full reobservation of queries affected by optimistic mutation updates, while still delivering results from the cache. <br/>
   [@benjamn](https://github.com/benjamn) in [#6854](https://github.com/apollographql/apollo-client/pull/6854)

src/core/QueryInfo.ts

@@ -25,10 +25,30 @@ export type QueryStoreValue = Pick<QueryInfo,
   | "graphQLErrors"
   >;
 
-const cacheEvictCounts = new (
+const destructiveMethodCounts = new (
   canUseWeakMap ? WeakMap : Map
 )<ApolloCache<any>, number>();
 
+function wrapDestructiveCacheMethod(
+  cache: ApolloCache<any>,
+  methodName: keyof ApolloCache<any>,
+) {
+  const original = cache[methodName];
+  if (typeof original === "function") {
+    cache[methodName] = function () {
+      destructiveMethodCounts.set(
+        cache,
+        // The %1e15 allows the count to wrap around to 0 safely every
+        // quadrillion evictions, so there's no risk of overflow. To be
+        // clear, this is more of a pedantic principle than something
+        // that matters in any conceivable practical scenario.
+        (destructiveMethodCounts.get(cache)! + 1) % 1e15,
+      );
+      return original.apply(this, arguments);
+    };
+  }
+}
+
 // A QueryInfo object represents a single query managed by the
 // QueryManager, which tracks all QueryInfo objects by queryId in its
 // this.queries Map. QueryInfo objects store the latest results and errors
@@ -57,20 +77,9 @@ export class QueryInfo {
     // causing shouldWrite to return true. Wrapping the cache.evict method
     // is a bit of a hack, but it saves us from having to make eviction
     // counting an official part of the ApolloCache API.
-    if (!cacheEvictCounts.has(cache) && cache.evict) {
-      cacheEvictCounts.set(cache, 0);
-      const originalEvict = cache.evict;
-      cache.evict = function evict() {
-        cacheEvictCounts.set(
-          cache,
-          // The %1e15 allows the count to wrap around to 0 safely every
-          // quadrillion evictions, so there's no risk of overflow. To be
-          // clear, this is more of a pedantic principle than something
-          // that matters in any conceivable practical scenario.
-          (cacheEvictCounts.get(cache)! + 1) % 1e15,
-        );
-        return originalEvict.apply(this, arguments);
-      };
+    if (!destructiveMethodCounts.has(cache)) {
+      wrapDestructiveCacheMethod(cache, "evict");
+      wrapDestructiveCacheMethod(cache, "modify");
     }
   }
 
@@ -246,7 +255,7 @@ export class QueryInfo {
   private lastWrite?: {
     result: FetchResult<any>;
     variables: WatchQueryOptions["variables"];
-    evictCount: number | undefined;
+    dmCount: number | undefined;
   };
 
   private shouldWrite(
@@ -259,7 +268,7 @@ export class QueryInfo {
       // If cache.evict has been called since the last time we wrote this
       // data into the cache, there's a chance writing this result into
       // the cache will repair what was evicted.
-      lastWrite.evictCount === cacheEvictCounts.get(this.cache) &&
+      lastWrite.dmCount === destructiveMethodCounts.get(this.cache) &&
       equal(variables, lastWrite.variables) &&
       equal(result.data, lastWrite.result.data)
     );
@@ -303,7 +312,7 @@ export class QueryInfo {
             this.lastWrite = {
               result,
               variables: options.variables,
-              evictCount: cacheEvictCounts.get(this.cache),
+              dmCount: destructiveMethodCounts.get(this.cache),
             };
           } else {
             // If result is the same as the last result we received from

src/core/__tests__/QueryManager/index.ts

@@ -2288,6 +2288,112 @@ describe('QueryManager', () => {
     });
   });
 
+  itAsync("should disable feud-stopping logic after evict or modify", (resolve, reject) => {
+    const cache = new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            info: {
+              merge: false,
+            },
+          },
+        },
+      },
+    });
+
+    const client = new ApolloClient({
+      cache,
+      link: new ApolloLink(operation => new Observable((observer: Observer<FetchResult>) => {
+        observer.next!({ data: { info: { c: "see" }}});
+        observer.complete!();
+      })),
+    });
+
+    const query = gql`query { info { c } }`;
+
+    const obs = client.watchQuery({
+      query,
+      returnPartialData: true,
+    });
+
+    subscribeAndCount(reject, obs, (count, result) => {
+      if (count === 1) {
+        expect(result).toEqual({
+          loading: true,
+          networkStatus: NetworkStatus.loading,
+          data: {},
+          partial: true,
+        });
+
+      } else if (count === 2) {
+        expect(result).toEqual({
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          data: {
+            info: {
+              c: "see",
+            },
+          },
+        });
+
+        cache.evict({
+          fieldName: "info",
+        });
+
+      } else if (count === 3) {
+        expect(result).toEqual({
+          loading: true,
+          networkStatus: NetworkStatus.loading,
+          data: {},
+          partial: true,
+        });
+
+      } else if (count === 4) {
+        expect(result).toEqual({
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          data: {
+            info: {
+              c: "see",
+            },
+          },
+        });
+
+        cache.modify({
+          fields: {
+            info(_, { DELETE }) {
+              return DELETE;
+            },
+          },
+        });
+
+      } else if (count === 5) {
+        expect(result).toEqual({
+          loading: true,
+          networkStatus: NetworkStatus.loading,
+          data: {},
+          partial: true,
+        });
+
+      } else if (count === 6) {
+        expect(result).toEqual({
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          data: {
+            info: {
+              c: "see",
+            },
+          },
+        });
+
+        setTimeout(resolve, 100);
+
+      } else {
+        reject(new Error(`Unexpected ${JSON.stringify({count,result})}`));
+      }
+    });
+  });
+
   itAsync('should not error when replacing unidentified data with a normalized ID', (resolve, reject) => {
     const queryWithoutId = gql`
       query {