Disable feud-stopping logic after any cache eviction. (#6817)

When application code evicts an object or its fields from the cache, and
those evictions trigger network requests, the network data is often vital
for repairing the damage done by the eviction, even if the network data
look exactly the same as the previously-written data.

In other words, after any cache eviction, we should disable the logic
introduced in PR #6448 to stop query feuds. This exception is reasonable
because feuding queries only write additional data into the cache, rather
than evicting anything, so we can safely assume evictions do not
contribute to a cycle of competing cache updates.

I freely admit the method of counting evictions that I've implemented here
is a bit of a hack, but it works for any ApolloCache implementation,
without requiring the public ApolloCache API to support any new
functionality. If we identify any other potential consumers of this
information, we might consider a more official API.

CHANGELOG.md

@@ -5,6 +5,9 @@
 - Substantially improve type inference for data and variables types using `TypedDocumentNode<Data, Variables>` type instead of `DocumentNode`. <br/>
   [@dotansimha](https://github.com/dotansimha) in [#6720](https://github.com/apollographql/apollo-client/pull/6720)
 
+- Disable feud-stopping logic after any cache eviction. <br/>
+  [@benjamn](https://github.com/benjamn) in [#6817](https://github.com/apollographql/apollo-client/pull/6817)
+
 ## Apollo Client 3.1.3
 
 ## Bug Fixes

src/core/QueryInfo.ts

@@ -10,6 +10,7 @@ import {
   ObservableSubscription,
   isNonEmptyArray,
   graphQLResultHasError,
+  canUseWeakMap,
 } from '../utilities';
 import {
   NetworkStatus,
@@ -24,6 +25,10 @@ export type QueryStoreValue = Pick<QueryInfo,
   | "graphQLErrors"
   >;
 
+const cacheEvictCounts = new (
+  canUseWeakMap ? WeakMap : Map
+)<ApolloCache<any>, number>();
+
 // 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
@@ -46,7 +51,28 @@ export class QueryInfo {
   networkError?: Error | null;
   graphQLErrors?: ReadonlyArray<GraphQLError>;
 
-  constructor(private cache: ApolloCache<any>) {}
+  constructor(private cache: ApolloCache<any>) {
+    // Track how often cache.evict is called, since we want eviction to
+    // override the feud-stopping logic in the markResult method, by
+    // 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);
+      };
+    }
+  }
 
   public init(query: {
     document: DocumentNode;
@@ -204,8 +230,27 @@ export class QueryInfo {
     }
   }
 
-  private lastWrittenResult?: FetchResult<any>;
-  private lastWrittenVars?: WatchQueryOptions["variables"];
+  private lastWrite?: {
+    result: FetchResult<any>;
+    variables: WatchQueryOptions["variables"];
+    evictCount: number | undefined;
+  };
+
+  private shouldWrite(
+    result: FetchResult<any>,
+    variables: WatchQueryOptions["variables"],
+  ) {
+    const { lastWrite } = this;
+    return !(
+      lastWrite &&
+      // 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) &&
+      equal(variables, lastWrite.variables) &&
+      equal(result.data, lastWrite.result.data)
+    );
+  }
 
   public markResult<T>(
     result: FetchResult<T>,
@@ -235,9 +280,19 @@ export class QueryInfo {
         // of writeQuery, so we can store the new diff quietly and ignore
         // it when we receive it redundantly from the watch callback.
         this.cache.performTransaction(cache => {
-          if (this.lastWrittenResult &&
-              equal(result.data, this.lastWrittenResult.data) &&
-              equal(options.variables, this.lastWrittenVars)) {
+          if (this.shouldWrite(result, options.variables)) {
+            cache.writeQuery({
+              query: this.document!,
+              data: result.data as T,
+              variables: options.variables,
+            });
+
+            this.lastWrite = {
+              result,
+              variables: options.variables,
+              evictCount: cacheEvictCounts.get(this.cache),
+            };
+          } else {
             // If result is the same as the last result we received from
             // the network (and the variables match too), avoid writing
             // result into the cache again. The wisdom of skipping this
@@ -278,14 +333,6 @@ export class QueryInfo {
             }
             // If the previous this.diff was incomplete, fall through to
             // re-reading the latest data with cache.diff, below.
-          } else {
-            cache.writeQuery({
-              query: this.document!,
-              data: result.data as T,
-              variables: options.variables,
-            });
-            this.lastWrittenResult = result;
-            this.lastWrittenVars = options.variables;
           }
 
           const diff = cache.diff<T>({
@@ -311,7 +358,7 @@ export class QueryInfo {
         });
 
       } else {
-        this.lastWrittenResult = this.lastWrittenVars = void 0;
+        this.lastWrite = void 0;
       }
     }
   }
@@ -323,7 +370,7 @@ export class QueryInfo {
 
   public markError(error: ApolloError) {
     this.networkStatus = NetworkStatus.error;
-    this.lastWrittenResult = this.lastWrittenVars = void 0;
+    this.lastWrite = void 0;
 
     if (error.graphQLErrors) {
       this.graphQLErrors = error.graphQLErrors;