From 05c58f3482cde34491691e9c015649134f033eff Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 20 May 2019 13:05:36 -0400 Subject: [PATCH] Avoid overriding cache-and-network fetchPolicy when refetching. Should help with the problem reported by @supercranky in this comment: https://github.com/apollographql/apollo-client/issues/4636#issuecomment-483218323 The apparent reasoning for preserving the no-cache and network-only fetch policies is that refetching requires a "network" fetch policy, but I believe cache-and-network should also count as a "network" fetch policy, since it always attempts to fetch from the network in addition to fetching from the cache. Even if that network request fails, it may be useful (e.g. for offline usage) to return data from the cache. --- .../apollo-client/src/core/ObservableQuery.ts | 3 +- .../src/core/__tests__/ObservableQuery.ts | 128 +++++++++++++++++- 2 files changed, 129 insertions(+), 2 deletions(-) diff --git a/packages/apollo-client/src/core/ObservableQuery.ts b/packages/apollo-client/src/core/ObservableQuery.ts index be7e3bb9424..a689040a9a3 100644 --- a/packages/apollo-client/src/core/ObservableQuery.ts +++ b/packages/apollo-client/src/core/ObservableQuery.ts @@ -298,7 +298,8 @@ export class ObservableQuery< // Override fetchPolicy for this call only // only network-only and no-cache are safe to use - if (fetchPolicy !== 'no-cache') { + if (fetchPolicy !== 'no-cache' && + fetchPolicy !== 'cache-and-network') { fetchPolicy = 'network-only'; } diff --git a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts b/packages/apollo-client/src/core/__tests__/ObservableQuery.ts index f52cff21814..506fcd1e4f2 100644 --- a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts +++ b/packages/apollo-client/src/core/__tests__/ObservableQuery.ts @@ -18,6 +18,7 @@ import ApolloClient from '../../'; import wrap from '../../util/wrap'; import subscribeAndCount from '../../util/subscribeAndCount'; import { stripSymbols } from 'apollo-utilities'; +import { ApolloError } from '../../errors/ApolloError'; describe('ObservableQuery', () => { // Standard data for all these tests @@ -1070,7 +1071,7 @@ describe('ObservableQuery', () => { const observable = queryManager.watchQuery({ query: firstRequest.query, variables: firstRequest.variables, - fetchPolicy: 'cache-and-network', + fetchPolicy: 'cache-first', }); const origFetchQuery = queryManager.fetchQuery; @@ -1200,6 +1201,131 @@ describe('ObservableQuery', () => { } }); }); + + it('cache-and-network refetch should run @client(always: true) resolvers when network request fails', done => { + const query = gql` + query MixedQuery { + counter @client(always: true) + name + } + `; + + let count = 0; + + let linkObservable = Observable.of({ + data: { + name: 'Ben', + }, + }); + + const intentionalNetworkFailure = new ApolloError({ + networkError: new Error('intentional network failure'), + }); + + const errorObservable: typeof linkObservable = new Observable( + observer => { + observer.error(intentionalNetworkFailure); + }, + ); + + const client = new ApolloClient({ + link: new ApolloLink(request => linkObservable), + cache: new InMemoryCache(), + resolvers: { + Query: { + counter() { + return ++count; + }, + }, + }, + }); + + const observable = client.watchQuery({ + query, + fetchPolicy: 'cache-and-network', + returnPartialData: true, + }); + + let handleCount = 0; + observable.subscribe({ + error(error) { + expect(error).toBe(intentionalNetworkFailure); + }, + + next(result) { + ++handleCount; + + if (handleCount === 1) { + expect(result).toEqual({ + data: {}, + loading: true, + networkStatus: NetworkStatus.loading, + stale: false, + }); + } else if (handleCount === 2) { + expect(result).toEqual({ + data: { + counter: 1, + }, + loading: true, + networkStatus: NetworkStatus.loading, + stale: false, + }); + } else if (handleCount === 3) { + expect(result).toEqual({ + data: { + counter: 2, + name: 'Ben', + }, + loading: false, + networkStatus: NetworkStatus.ready, + stale: false, + }); + + // Make the next network request fail. + linkObservable = errorObservable; + + observable.refetch().then( + result => { + expect(result).toEqual({ + data: { + counter: 3, + name: 'Ben', + }, + }); + }, + error => { + expect(error).toBe(intentionalNetworkFailure); + }, + ); + } else if (handleCount === 4) { + expect(result).toEqual({ + data: { + counter: 2, + name: 'Ben', + }, + loading: true, + networkStatus: NetworkStatus.refetch, + stale: false, + }); + } else if (handleCount === 5) { + expect(result).toEqual({ + data: { + counter: 3, + name: 'Ben', + }, + loading: true, + networkStatus: NetworkStatus.refetch, + stale: false, + }); + + done(); + } else if (handleCount > 5) { + done.fail(new Error('should not get here')); + } + }, + }); + }); }); describe('currentResult', () => {