From 49b45e311987cb73b7be038c43acd9a761c6ac46 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 15 Mar 2021 12:50:01 -0400 Subject: [PATCH] Revert "Merge pull request #7761 from apollographql/7611-preserve-no-cache-fetchpolicy" This reverts commit 319993eac4b5ef5fad82994c76e3e7fff6a30dcb, reversing changes made to 6120401181612e9245873cb4735d9fe0098e70a0. 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. --- CHANGELOG.md | 2 - src/core/QueryManager.ts | 53 ++++------ src/core/__tests__/fetchPolicies.ts | 157 ---------------------------- 3 files changed, 22 insertions(+), 190 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 854f2185e66..c18c8d5a647 100644 --- 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.
[@benjamn](https://github.com/benjamn) in [#7818](https://github.com/apollographql/apollo-client/pull/7818) -- Preserve fetch policy even when `notifyOnNetworkStatusChange` is set
- [@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 index f41e6f001e2..827b91b7128 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -914,6 +914,7 @@ export class QueryManager { 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 { 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 { errorPolicy, returnPartialData, context, - notifyOnNetworkStatusChange, } = options; - const oldNetworkStatus = queryInfo.networkStatus; - queryInfo.init({ document: query, variables, @@ -1074,13 +1092,6 @@ export class QueryManager { 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 { ]; } - if (shouldNotifyOnNetworkStatusChange()) { - return [ - resultsFromCache(diff), - resultsFromLink(true), - ]; - } - return [ resultsFromLink(true), ]; @@ -1113,7 +1117,7 @@ export class QueryManager { 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 { ]; 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 index f05b56cfe3d..b67128eb701 100644 --- 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', () => {