diff --git a/CHANGELOG.md b/CHANGELOG.md index bd8f16d1fcc..45f2369efa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ TBD - 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 fe84f4a7a62..eb10408da84 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -890,7 +890,6 @@ 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, @@ -900,26 +899,6 @@ 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, @@ -1047,8 +1026,11 @@ export class QueryManager { errorPolicy, returnPartialData, context, + notifyOnNetworkStatusChange, } = options; + const oldNetworkStatus = queryInfo.networkStatus; + queryInfo.init({ document: query, variables, @@ -1101,6 +1083,13 @@ export class QueryManager { errorPolicy, }); + const shouldNotifyOnNetworkStatusChange = () => ( + notifyOnNetworkStatusChange && + typeof oldNetworkStatus === "number" && + oldNetworkStatus !== networkStatus && + isNetworkRequestInFlight(networkStatus) + ); + switch (fetchPolicy) { default: case "cache-first": { const diff = readCache(); @@ -1118,6 +1107,13 @@ export class QueryManager { ]; } + if (shouldNotifyOnNetworkStatusChange()) { + return [ + resultsFromCache(diff), + resultsFromLink(true), + ]; + } + return [ resultsFromLink(true), ]; @@ -1126,7 +1122,7 @@ export class QueryManager { case "cache-and-network": { const diff = readCache(); - if (diff.complete || returnPartialData) { + if (diff.complete || returnPartialData || shouldNotifyOnNetworkStatusChange()) { return [ resultsFromCache(diff), resultsFromLink(true), @@ -1144,9 +1140,22 @@ 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 b67128eb701..f05b56cfe3d 100644 --- a/src/core/__tests__/fetchPolicies.ts +++ b/src/core/__tests__/fetchPolicies.ts @@ -324,6 +324,163 @@ 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', () => {