Revert "Merge pull request #7761 from apollographql/7611-preserve-no-…

…cache-fetchpolicy" This reverts commit 319993e, reversing changes made to 6120401. Because #7611 could be a visible change for some applications, we've decided to relocate these changes onto the release-3.4 branch, rather than main, so we revert the PR on main, merge main into release-3.4, then un-revert the PR on the release-3.4 branch.
apollographql · Mar 15, 2021 · 49b45e3 · 49b45e3
1 parent 834c6ff
commit 49b45e3
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 190 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,8 +5,6 @@
 - Maintain serial ordering of `asyncMap` mapping function calls, and prevent potential unhandled `Promise` rejection errors. <br/>
   [@benjamn](https://github.com/benjamn) in [#7818](https://github.com/apollographql/apollo-client/pull/7818)
 
-- Preserve fetch policy even when `notifyOnNetworkStatusChange` is set <br />
-  [@jcreighton](https://github.com/jcreighton) in [#7761](https://github.com/apollographql/apollo-client/pull/7761)
 ## Apollo Client 3.3.11
 
 ### Bug fixes

diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
@@ -914,6 +914,7 @@ export class QueryManager<TStore> {
     const query = this.transform(options.query).document;
     const variables = this.getVariables(query, options.variables) as TVars;
     const queryInfo = this.getQuery(queryId);
+    const oldNetworkStatus = queryInfo.networkStatus;
 
     let {
       fetchPolicy = "cache-first" as WatchQueryFetchPolicy,
@@ -923,6 +924,26 @@ export class QueryManager<TStore> {
       context = {},
     } = options;
 
+    const mightUseNetwork =
+      fetchPolicy === "cache-first" ||
+      fetchPolicy === "cache-and-network" ||
+      fetchPolicy === "network-only" ||
+      fetchPolicy === "no-cache";
+
+    if (mightUseNetwork &&
+        notifyOnNetworkStatusChange &&
+        typeof oldNetworkStatus === "number" &&
+        oldNetworkStatus !== networkStatus &&
+        isNetworkRequestInFlight(networkStatus)) {
+      // In order to force delivery of an incomplete cache result with
+      // loading:true, we tweak the fetchPolicy to include the cache, and
+      // pretend that returnPartialData was enabled.
+      if (fetchPolicy !== "cache-first") {
+        fetchPolicy = "cache-and-network";
+      }
+      returnPartialData = true;
+    }
+
     const normalized = Object.assign({}, options, {
       query,
       variables,
@@ -1017,11 +1038,8 @@ export class QueryManager<TStore> {
       errorPolicy,
       returnPartialData,
       context,
-      notifyOnNetworkStatusChange,
     } = options;
 
-    const oldNetworkStatus = queryInfo.networkStatus;
-
     queryInfo.init({
       document: query,
       variables,
@@ -1074,13 +1092,6 @@ export class QueryManager<TStore> {
         errorPolicy,
       });
 
-    const shouldNotifyOnNetworkStatusChange = () => (
-      notifyOnNetworkStatusChange &&
-      typeof oldNetworkStatus === "number" &&
-      oldNetworkStatus !== networkStatus &&
-      isNetworkRequestInFlight(networkStatus)
-    );
-
     switch (fetchPolicy) {
     default: case "cache-first": {
       const diff = readCache();
@@ -1098,13 +1109,6 @@ export class QueryManager<TStore> {
         ];
       }
 
-      if (shouldNotifyOnNetworkStatusChange()) {
-        return [
-          resultsFromCache(diff),
-          resultsFromLink(true),
-        ];
-      }
-
       return [
         resultsFromLink(true),
       ];
@@ -1113,7 +1117,7 @@ export class QueryManager<TStore> {
     case "cache-and-network": {
       const diff = readCache();
 
-      if (diff.complete || returnPartialData || shouldNotifyOnNetworkStatusChange()) {
+      if (diff.complete || returnPartialData) {
         return [
           resultsFromCache(diff),
           resultsFromLink(true),
@@ -1131,22 +1135,9 @@ export class QueryManager<TStore> {
       ];
 
     case "network-only":
-      if (shouldNotifyOnNetworkStatusChange()) {
-        const diff = readCache();
-
-        return [
-          resultsFromCache(diff),
-          resultsFromLink(true),
-        ];
-      }
-
       return [resultsFromLink(true)];
 
     case "no-cache":
-      if (shouldNotifyOnNetworkStatusChange()) {
-          return [resultsFromCache(queryInfo.getDiff()), resultsFromLink(false)];
-      }
-
       return [resultsFromLink(false)];
 
     case "standby":

diff --git a/src/core/__tests__/fetchPolicies.ts b/src/core/__tests__/fetchPolicies.ts
@@ -324,163 +324,6 @@ describe('no-cache', () => {
       })
       .then(resolve, reject);
   });
-
-  describe('when notifyOnNetworkStatusChange is set', () => {
-    itAsync('does not save the data to the cache on success', (resolve, reject) => {
-      let called = 0;
-      const inspector = new ApolloLink((operation, forward) => {
-        called++;
-        return forward(operation).map(result => {
-          called++;
-          return result;
-        });
-      });
-
-      const client = new ApolloClient({
-        link: inspector.concat(createLink(reject)),
-        cache: new InMemoryCache({ addTypename: false }),
-      });
-
-      return client.query({ query, fetchPolicy: 'no-cache', notifyOnNetworkStatusChange: true }).then(
-        () => client.query({ query }).then(actualResult => {
-          expect(stripSymbols(actualResult.data)).toEqual(result);
-          // the second query couldn't read anything from the cache
-          expect(called).toBe(4);
-        }),
-      ).then(resolve, reject);
-    });
-
-    itAsync('does not save data to the cache on failure', (resolve, reject) => {
-      let called = 0;
-      const inspector = new ApolloLink((operation, forward) => {
-        called++;
-        return forward(operation).map(result => {
-          called++;
-          return result;
-        });
-      });
-
-      const client = new ApolloClient({
-        link: inspector.concat(createFailureLink()),
-        cache: new InMemoryCache({ addTypename: false }),
-      });
-
-      let didFail = false;
-      return client
-        .query({ query, fetchPolicy: 'no-cache', notifyOnNetworkStatusChange: true })
-        .catch(e => {
-          expect(e.message).toMatch('query failed');
-          didFail = true;
-        })
-        .then(() => client.query({ query }).then(actualResult => {
-          expect(stripSymbols(actualResult.data)).toEqual(result);
-          // the first error doesn't call .map on the inspector
-          expect(called).toBe(3);
-          expect(didFail).toBe(true);
-        }))
-        .then(resolve, reject);
-    });
-
-    itAsync('gives appropriate networkStatus for watched queries', (resolve, reject) => {
-      const client = new ApolloClient({
-        link: ApolloLink.empty(),
-        cache: new InMemoryCache(),
-        resolvers: {
-          Query: {
-            hero(_data, args) {
-              return {
-                __typename: 'Hero',
-                ...args,
-                name: 'Luke Skywalker',
-              };
-            },
-          },
-        },
-      });
-
-      const observable = client.watchQuery({
-        query: gql`
-          query FetchLuke($id: String) {
-            hero(id: $id) @client {
-              id
-              name
-            }
-          }
-        `,
-        fetchPolicy: 'no-cache',
-        variables: { id: '1' },
-        notifyOnNetworkStatusChange: true,
-      });
-
-      function dataWithId(id: number | string) {
-        return {
-          hero: {
-            __typename: 'Hero',
-            id: String(id),
-            name: 'Luke Skywalker',
-          },
-        };
-      }
-
-      subscribeAndCount(reject, observable, (count, result) => {
-        if (count === 1) {
-          expect(result).toEqual({
-            data: dataWithId(1),
-            loading: false,
-            networkStatus: NetworkStatus.ready,
-          });
-          expect(client.cache.extract(true)).toEqual({});
-          return observable.setVariables({ id: '2' });
-        } else if (count === 2) {
-          expect(result).toEqual({
-            data: {},
-            loading: true,
-            networkStatus: NetworkStatus.setVariables,
-            partial: true,
-          });
-        } else if (count === 3) {
-          expect(result).toEqual({
-            data: dataWithId(2),
-            loading: false,
-            networkStatus: NetworkStatus.ready,
-          });
-          expect(client.cache.extract(true)).toEqual({});
-          return observable.refetch();
-        } else if (count === 4) {
-          expect(result).toEqual({
-            data: dataWithId(2),
-            loading: true,
-            networkStatus: NetworkStatus.refetch,
-          });
-          expect(client.cache.extract(true)).toEqual({});
-        } else if (count === 5) {
-          expect(result).toEqual({
-            data: dataWithId(2),
-            loading: false,
-            networkStatus: NetworkStatus.ready,
-          });
-          expect(client.cache.extract(true)).toEqual({});
-          return observable.refetch({ id: '3' });
-        } else if (count === 6) {
-          expect(result).toEqual({
-            data: {},
-            loading: true,
-            networkStatus: NetworkStatus.setVariables,
-            partial: true,
-          });
-          expect(client.cache.extract(true)).toEqual({});
-        } else if (count === 7) {
-          expect(result).toEqual({
-            data: dataWithId(3),
-            loading: false,
-            networkStatus: NetworkStatus.ready,
-          });
-          expect(client.cache.extract(true)).toEqual({});
-          resolve();
-        }
-      });
-    });
-  });
 });
 
 describe('cache-first', () => {