From a16205e42866987dbbea1e5d065dab5f7e500454 Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@apollographql.com>
Date: Mon, 8 Mar 2021 18:58:39 -0500
Subject: [PATCH] Make refetch results overwrite existing data.

This is how I would ideally like to fix #7491, but I'm worried it could be
a breaking change for any application code that relies on refetch results
getting merged with existing data, rather than replacing it (which is what
refetch was originally meant to do, since AC2).

I will follow this commit with one that makes this the overwrite behavior
opt-in (and thus backwards compatible).
---
 src/core/QueryInfo.ts    | 11 +++++++++--
 src/core/QueryManager.ts | 36 +++++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts
index 7d8ae8ac8d0..671a0261b18 100644
--- a/src/core/QueryInfo.ts
+++ b/src/core/QueryInfo.ts
@@ -25,6 +25,12 @@ export type QueryStoreValue = Pick<QueryInfo,
   | "graphQLErrors"
   >;
 
+export const enum CacheWriteBehavior {
+  FORBID,
+  OVERWRITE,
+  MERGE,
+};
+
 const destructiveMethodCounts = new (
   canUseWeakMap ? WeakMap : Map
 )<ApolloCache<any>, number>();
@@ -307,7 +313,7 @@ export class QueryInfo {
       | "variables"
       | "fetchPolicy"
       | "errorPolicy">,
-    allowCacheWrite: boolean,
+    cacheWriteBehavior: CacheWriteBehavior,
   ) {
     this.graphQLErrors = isNonEmptyArray(result.errors) ? result.errors : [];
 
@@ -318,7 +324,7 @@ export class QueryInfo {
     if (options.fetchPolicy === 'no-cache') {
       this.diff = { result: result.data, complete: true };
 
-    } else if (!this.stopped && allowCacheWrite) {
+    } else if (!this.stopped && cacheWriteBehavior !== CacheWriteBehavior.FORBID) {
       if (shouldWriteResult(result, options.errorPolicy)) {
         // Using a transaction here so we have a chance to read the result
         // back from the cache before the watch callback fires as a result
@@ -330,6 +336,7 @@ export class QueryInfo {
               query: this.document!,
               data: result.data as T,
               variables: options.variables,
+              overwrite: cacheWriteBehavior === CacheWriteBehavior.OVERWRITE,
             });
 
             this.lastWrite = {
diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
index 04f3392e28c..985953a3690 100644
--- a/src/core/QueryManager.ts
+++ b/src/core/QueryManager.ts
@@ -38,7 +38,12 @@ import {
 } from './types';
 import { LocalState } from './LocalState';
 
-import { QueryInfo, QueryStoreValue, shouldWriteResult } from './QueryInfo';
+import {
+  QueryInfo,
+  QueryStoreValue,
+  shouldWriteResult,
+  CacheWriteBehavior,
+} from './QueryInfo';
 
 const { hasOwnProperty } = Object.prototype;
 
@@ -822,7 +827,7 @@ export class QueryManager<TStore> {
 
   private getResultsFromLink<TData, TVars>(
     queryInfo: QueryInfo,
-    allowCacheWrite: boolean,
+    cacheWriteBehavior: CacheWriteBehavior,
     options: Pick<WatchQueryOptions<TVars, TData>,
       | "variables"
       | "context"
@@ -848,7 +853,7 @@ export class QueryManager<TStore> {
               graphQLErrors: result.errors,
             }));
           }
-          queryInfo.markResult(result, options, allowCacheWrite);
+          queryInfo.markResult(result, options, cacheWriteBehavior);
           queryInfo.markReady();
         }
 
@@ -1076,8 +1081,13 @@ export class QueryManager<TStore> {
       return fromData(data);
     };
 
-    const resultsFromLink = (allowCacheWrite: boolean) =>
-      this.getResultsFromLink<TData, TVars>(queryInfo, allowCacheWrite, {
+    const cacheWriteBehavior =
+      fetchPolicy === "no-cache" ? CacheWriteBehavior.FORBID :
+      networkStatus === NetworkStatus.refetch ? CacheWriteBehavior.OVERWRITE :
+      CacheWriteBehavior.MERGE;
+
+    const resultsFromLink = () =>
+      this.getResultsFromLink<TData, TVars>(queryInfo, cacheWriteBehavior, {
         variables,
         context,
         fetchPolicy,
@@ -1103,12 +1113,12 @@ export class QueryManager<TStore> {
       if (returnPartialData || shouldNotify) {
         return [
           resultsFromCache(diff),
-          resultsFromLink(true),
+          resultsFromLink(),
         ];
       }
 
       return [
-        resultsFromLink(true),
+        resultsFromLink(),
       ];
     }
 
@@ -1118,12 +1128,12 @@ export class QueryManager<TStore> {
       if (diff.complete || returnPartialData || shouldNotify) {
         return [
           resultsFromCache(diff),
-          resultsFromLink(true),
+          resultsFromLink(),
         ];
       }
 
       return [
-        resultsFromLink(true),
+        resultsFromLink(),
       ];
     }
 
@@ -1136,11 +1146,11 @@ export class QueryManager<TStore> {
       if (shouldNotify) {
         return [
           resultsFromCache(readCache()),
-          resultsFromLink(true),
+          resultsFromLink(),
         ];
       }
 
-      return [resultsFromLink(true)];
+      return [resultsFromLink()];
 
     case "no-cache":
       if (shouldNotify) {
@@ -1149,11 +1159,11 @@ export class QueryManager<TStore> {
           // cache.diff, but instead returns a { complete: false } stub result
           // when there is no queryInfo.diff already defined.
           resultsFromCache(queryInfo.getDiff()),
-          resultsFromLink(false),
+          resultsFromLink(),
         ];
       }
 
-      return [resultsFromLink(false)];
+      return [resultsFromLink()];
 
     case "standby":
       return [];