don't cancel existing subscriptions when updating cache from another source

.changeset/swift-mails-search.md.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix a bug where an incoming cache update could prevent future updates from the active link.

.git-blame-ignore-revs

@@ -0,0 +1 @@
+78809ce69e2ce8dece8d7cd414756189a23b872f

config/bundlesize.ts

@@ -3,7 +3,7 @@ import { join } from "path";
 import { gzipSync } from "zlib";
 import bytes from "bytes";
 
-const gzipBundleByteLengthLimit = bytes("32.65KB");
+const gzipBundleByteLengthLimit = bytes("32.75KB");
 const minFile = join("dist", "apollo-client.min.cjs");
 const minPath = join(__dirname, "..", minFile);
 const gzipByteLen = gzipSync(readFileSync(minPath)).byteLength;

src/core/ObservableQuery.ts

@@ -692,11 +692,11 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
   private fetch(
     options: WatchQueryOptions<TVariables, TData>,
     newNetworkStatus?: NetworkStatus,
-  ): Concast<ApolloQueryResult<TData>> {
+  ) {
     // TODO Make sure we update the networkStatus (and infer fetchVariables)
     // before actually committing to the fetch.
     this.queryManager.setObservableQuery(this);
-    return this.queryManager.fetchQueryObservable(
+    return this.queryManager['fetchConcastWithInfo'](
       this.queryId,
       options,
       newNetworkStatus,
@@ -835,7 +835,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
     }
 
     const variables = options.variables && { ...options.variables };
-    const concast = this.fetch(options, newNetworkStatus);
+    const { concast, fromLink } = this.fetch(options, newNetworkStatus);
     const observer: Observer<ApolloQueryResult<TData>> = {
       next: result => {
         this.reportResult(result, variables);
@@ -845,7 +845,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
       },
     };
 
-    if (!useDisposableConcast) {
+    if (!useDisposableConcast && fromLink) {
       // We use the {add,remove}Observer methods directly to avoid wrapping
       // observer with an unnecessary SubscriptionObserver object.
       if (this.concast && this.observer) {

src/core/QueryManager.ts

@@ -1159,8 +1159,23 @@ export class QueryManager<TStore> {
     // The initial networkStatus for this fetch, most often
     // NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
     // or setVariables.
-    networkStatus = NetworkStatus.loading,
+    networkStatus?: NetworkStatus,
   ): Concast<ApolloQueryResult<TData>> {
+    return this.fetchConcastWithInfo(
+      queryId,
+      options,
+      networkStatus,
+    ).concast;
+  }
+
+  private fetchConcastWithInfo<TData, TVars extends OperationVariables>(
+    queryId: string,
+    options: WatchQueryOptions<TVars, TData>,
+    // The initial networkStatus for this fetch, most often
+    // NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
+    // or setVariables.
+    networkStatus = NetworkStatus.loading
+  ): ConcastAndInfo<TData> {
     const query = this.transform(options.query).document;
     const variables = this.getVariables(query, options.variables) as TVars;
     const queryInfo = this.getQuery(queryId);
@@ -1190,7 +1205,7 @@ export class QueryManager<TStore> {
       // WatchQueryOptions object.
       normalized.variables = variables;
 
-      const concastSources = this.fetchQueryByPolicy<TData, TVars>(
+      const sourcesWithInfo = this.fetchQueryByPolicy<TData, TVars>(
         queryInfo,
         normalized,
         networkStatus,
@@ -1202,13 +1217,13 @@ export class QueryManager<TStore> {
         normalized.fetchPolicy !== "standby" &&
         // The "standby" policy currently returns [] from fetchQueryByPolicy, so
         // this is another way to detect when nothing was done/fetched.
-        concastSources.length > 0 &&
+        sourcesWithInfo.sources.length > 0 &&
         queryInfo.observableQuery
       ) {
         queryInfo.observableQuery["applyNextFetchPolicy"]("after-fetch", options);
       }
 
-      return concastSources;
+      return sourcesWithInfo;
     };
 
     // This cancel function needs to be set before the concast is created,
@@ -1220,29 +1235,39 @@ export class QueryManager<TStore> {
       setTimeout(() => concast.cancel(reason));
     });
 
-    // A Concast<T> can be created either from an Iterable<Observable<T>>
-    // or from a PromiseLike<Iterable<Observable<T>>>, where T in this
-    // case is ApolloQueryResult<TData>.
-    const concast = new Concast(
-      // If the query has @export(as: ...) directives, then we need to
-      // process those directives asynchronously. When there are no
-      // @export directives (the common case), we deliberately avoid
-      // wrapping the result of this.fetchQueryByPolicy in a Promise,
-      // since the timing of result delivery is (unfortunately) important
-      // for backwards compatibility. TODO This code could be simpler if
-      // we deprecated and removed LocalState.
-      this.transform(normalized.query).hasClientExports
-        ? this.localState.addExportedVariables(
-          normalized.query,
-          normalized.variables,
-          normalized.context,
-        ).then(fromVariables)
-        : fromVariables(normalized.variables!)
-    );
+    let concast: Concast<ApolloQueryResult<TData>>, 
+        containsDataFromLink: boolean;
+    // If the query has @export(as: ...) directives, then we need to
+    // process those directives asynchronously. When there are no
+    // @export directives (the common case), we deliberately avoid
+    // wrapping the result of this.fetchQueryByPolicy in a Promise,
+    // since the timing of result delivery is (unfortunately) important
+    // for backwards compatibility. TODO This code could be simpler if
+    // we deprecated and removed LocalState.
+    if (this.transform(normalized.query).hasClientExports) {
+      concast = new Concast(
+        this.localState
+          .addExportedVariables(normalized.query, normalized.variables, normalized.context)
+          .then(fromVariables).then(sourcesWithInfo => sourcesWithInfo.sources),
+      );
+      // there is just no way we can synchronously get the *right* value here,
+      // so we will assume `true`, which is the behaviour before the bug fix in
+      // #10597. This means that bug is not fixed in that case, and is probably
+      // un-fixable with reasonable effort for the edge case of @export as
+      // directives.
+      containsDataFromLink = true;
+    } else {
+      const sourcesWithInfo = fromVariables(normalized.variables);
+      containsDataFromLink = sourcesWithInfo.fromLink;
+      concast = new Concast(sourcesWithInfo.sources);
+    }
 
     concast.promise.then(cleanupCancelFn, cleanupCancelFn);
 
-    return concast;
+    return {
+      concast,
+      fromLink: containsDataFromLink,
+    };
   }
 
   public refetchQueries<TResult>({
@@ -1415,8 +1440,8 @@ export class QueryManager<TStore> {
     // The initial networkStatus for this fetch, most often
     // NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
     // or setVariables.
-    networkStatus: NetworkStatus,
-  ): ConcastSourcesArray<ApolloQueryResult<TData>> {
+    networkStatus: NetworkStatus
+  ): SourcesAndInfo<TData> {
     const oldNetworkStatus = queryInfo.networkStatus;
 
     queryInfo.init({
@@ -1498,72 +1523,58 @@ export class QueryManager<TStore> {
       isNetworkRequestInFlight(networkStatus);
 
     switch (fetchPolicy) {
-    default: case "cache-first": {
-      const diff = readCache();
+      default:  case "cache-first": {
+        const diff = readCache();
 
-      if (diff.complete) {
-        return [
-          resultsFromCache(diff, queryInfo.markReady()),
-        ];
-      }
+        if (diff.complete) {
+          return { fromLink: false, sources: [resultsFromCache(diff, queryInfo.markReady())] };
+        }
+
+        if (returnPartialData || shouldNotify) {
+          return { fromLink: true, sources: [resultsFromCache(diff), resultsFromLink()] };
+        }
 
-      if (returnPartialData || shouldNotify) {
-        return [
-          resultsFromCache(diff),
-          resultsFromLink(),
-        ];
+        return { fromLink: true, sources: [resultsFromLink()] };
       }
 
-      return [
-        resultsFromLink(),
-      ];
-    }
+      case "cache-and-network": {
+        const diff = readCache();
 
-    case "cache-and-network": {
-      const diff = readCache();
+        if (diff.complete || returnPartialData || shouldNotify) {
+          return { fromLink: true, sources: [resultsFromCache(diff), resultsFromLink()] };
+        }
 
-      if (diff.complete || returnPartialData || shouldNotify) {
-        return [
-          resultsFromCache(diff),
-          resultsFromLink(),
-        ];
+        return { fromLink: true, sources: [resultsFromLink()] };
       }
 
-      return [
-        resultsFromLink(),
-      ];
-    }
+      case "cache-only":
+        return { fromLink: false, sources: [resultsFromCache(readCache(), queryInfo.markReady())] };
 
-    case "cache-only":
-      return [
-        resultsFromCache(readCache(), queryInfo.markReady()),
-      ];
-
-    case "network-only":
-      if (shouldNotify) {
-        return [
-          resultsFromCache(readCache()),
-          resultsFromLink(),
-        ];
-      }
+      case "network-only":
+        if (shouldNotify) {
+          return { fromLink: true, sources: [resultsFromCache(readCache()), resultsFromLink()] };
+        }
 
-      return [resultsFromLink()];
-
-    case "no-cache":
-      if (shouldNotify) {
-        return [
-          // Note that queryInfo.getDiff() for no-cache queries does not call
-          // cache.diff, but instead returns a { complete: false } stub result
-          // when there is no queryInfo.diff already defined.
-          resultsFromCache(queryInfo.getDiff()),
-          resultsFromLink(),
-        ];
-      }
+        return { fromLink: true, sources: [resultsFromLink()] };
+
+      case "no-cache":
+        if (shouldNotify) {
+          return {
+            fromLink: true,
+            // Note that queryInfo.getDiff() for no-cache queries does not call
+            // cache.diff, but instead returns a { complete: false } stub result
+            // when there is no queryInfo.diff already defined.
+            sources: [
+              resultsFromCache(queryInfo.getDiff()),
+              resultsFromLink(),
+            ],
+          };
+        }
 
-      return [resultsFromLink()];
+        return { fromLink: true, sources: [resultsFromLink()] };
 
-    case "standby":
-      return [];
+      case "standby":
+        return { fromLink: false, sources: [] };
     }
   }
 
@@ -1582,3 +1593,15 @@ export class QueryManager<TStore> {
     };
   }
 }
+
+// Return types used by fetchQueryByPolicy and other private methods above.
+interface FetchConcastInfo {
+  // Metadata properties that can be returned in addition to the Concast.
+  fromLink: boolean;
+}
+interface SourcesAndInfo<TData> extends FetchConcastInfo {
+  sources: ConcastSourcesArray<ApolloQueryResult<TData>>;
+}
+interface ConcastAndInfo<TData> extends FetchConcastInfo {
+  concast: Concast<ApolloQueryResult<TData>>;
+}