diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 3af192b809f..60a34b7878d 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -836,6 +836,24 @@ export class QueryManager { context = {}, } = options; + if (fetchPolicy === "cache-and-network" || + fetchPolicy === "network-only") { + // When someone chooses cache-and-network or network-only as their + // initial FetchPolicy, they almost certainly do not want future cache + // updates to trigger unconditional network requests, which is what + // repeatedly applying the cache-and-network or network-only policies + // would seem to require. Instead, when the cache reports an update + // after the initial network request, subsequent network requests should + // be triggered only if the cache result is incomplete. This behavior + // corresponds exactly to switching to a cache-first FetchPolicy, so we + // modify options.fetchPolicy here for the next fetchQueryObservable + // call, using the same options object that the Reobserver always passes + // to fetchQueryObservable. Note: if these FetchPolicy transitions get + // much more complicated, we might consider using some sort of state + // machine to capture the transition rules. + options.fetchPolicy = "cache-first"; + } + const mightUseNetwork = fetchPolicy === "cache-first" || fetchPolicy === "cache-and-network" || diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index 941dacb657f..c48419558d4 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -933,15 +933,29 @@ describe('ObservableQuery', () => { }); describe('refetch', () => { - type TFQO = QueryManager["fetchQueryObservable"]; function mockFetchQuery(queryManager: QueryManager) { - const origFetchQuery: TFQO = (queryManager as any).fetchQueryObservable; - return (queryManager as any).fetchQueryObservable = jest.fn< - ReturnType, - Parameters + const fetchQueryObservable = queryManager.fetchQueryObservable; + const fetchQueryByPolicy: QueryManager["fetchQueryByPolicy"] = + (queryManager as any).fetchQueryByPolicy; + + const mock = (original: T) => jest.fn< + ReturnType, + Parameters >(function () { - return origFetchQuery.apply(queryManager, arguments); + return original.apply(queryManager, arguments); }); + + const mocks = { + fetchQueryObservable: mock(fetchQueryObservable), + fetchQueryByPolicy: mock(fetchQueryByPolicy), + }; + + Object.assign(queryManager, mocks); + + return mocks; } itAsync('calls fetchRequest with fetchPolicy `network-only` when using a non-networked fetch policy', (resolve, reject) => { @@ -964,15 +978,24 @@ describe('ObservableQuery', () => { fetchPolicy: 'cache-first', }); - const mocked = mockFetchQuery(queryManager); + const mocks = mockFetchQuery(queryManager); subscribeAndCount(reject, observable, handleCount => { if (handleCount === 1) { observable.refetch(differentVariables); } else if (handleCount === 2) { - expect(mocked.mock.calls[1][1].fetchPolicy).toEqual( - 'network-only', - ); + const fqbpCalls = mocks.fetchQueryByPolicy.mock.calls; + expect(fqbpCalls.length).toBe(2); + expect(fqbpCalls[1][1].fetchPolicy).toEqual('network-only'); + // Although the options.fetchPolicy we passed just now to + // fetchQueryByPolicy should have been network-only, + // observable.options.fetchPolicy should now be updated to + // cache-first, since network-only (and cache-and-network) fetch + // policies fall back to cache-first after the first request. + expect(observable.options.fetchPolicy).toBe('cache-first'); + const fqoCalls = mocks.fetchQueryObservable.mock.calls; + expect(fqoCalls.length).toBe(2); + expect(fqoCalls[1][1].fetchPolicy).toEqual('cache-first'); resolve(); } }); @@ -1000,15 +1023,24 @@ describe('ObservableQuery', () => { fetchPolicy: 'no-cache', }); - const mocked = mockFetchQuery(queryManager); + const mocks = mockFetchQuery(queryManager); subscribeAndCount(reject, observable, handleCount => { if (handleCount === 1) { observable.refetch(differentVariables); } else if (handleCount === 2) { - expect( - mocked.mock.calls[1][1].fetchPolicy, - ).toEqual('no-cache'); + const fqbpCalls = mocks.fetchQueryByPolicy.mock.calls; + expect(fqbpCalls.length).toBe(2); + expect(fqbpCalls[1][1].fetchPolicy).toBe('no-cache'); + + // Unlike network-only or cache-and-network, the no-cache + // FetchPolicy does not switch to cache-first after the first + // network request. + expect(observable.options.fetchPolicy).toBe('no-cache'); + const fqoCalls = mocks.fetchQueryObservable.mock.calls; + expect(fqoCalls.length).toBe(2); + expect(fqoCalls[1][1].fetchPolicy).toBe('no-cache'); + resolve(); } }); @@ -1159,34 +1191,35 @@ describe('ObservableQuery', () => { networkStatus: NetworkStatus.ready, }); + const oldLinkObs = linkObservable; // Make the next network request fail. linkObservable = errorObservable; observable.refetch().then( - result => { - expect(result).toEqual({ - data: { - counter: 3, - name: 'Ben', - }, - }); + () => { + reject(new Error('should have gotten an error')); }, + error => { expect(error).toBe(intentionalNetworkFailure); + + // Switch back from errorObservable. + linkObservable = oldLinkObs; + + observable.refetch().then(result => { + expect(result).toEqual({ + data: { + counter: 3, + name: 'Ben', + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + resolve(); + }, reject); }, ); - } else if (handleCount === 3) { - expect(result).toEqual({ - data: { - counter: 3, - name: 'Ben', - }, - loading: true, - networkStatus: NetworkStatus.refetch, - }); - - resolve(); - } else if (handleCount > 4) { + } else if (handleCount > 2) { reject(new Error('should not get here')); } }, @@ -1545,38 +1578,38 @@ describe('ObservableQuery', () => { }, ); - queryManager.query({ query, variables }).then(() => { + queryManager.query({ query, variables }).then(result => { + expect(result).toEqual({ + data: dataOne, + loading: false, + networkStatus: NetworkStatus.ready, + }); + const observable = queryManager.watchQuery({ query, variables, fetchPolicy: 'network-only', }); - expect(stripSymbols(observable.getCurrentResult())).toEqual({ - data: undefined, + + expect(observable.getCurrentResult()).toEqual({ + data: void 0, loading: true, networkStatus: 1, partial: false, }); subscribeAndCount(reject, observable, (handleCount, subResult) => { - const { - data, - loading, - networkStatus, - } = observable.getCurrentResult(); - if (handleCount === 1) { expect(subResult).toEqual({ - data, - loading, - networkStatus, + loading: true, + networkStatus: NetworkStatus.loading, partial: false, }); } else if (handleCount === 2) { - expect(stripSymbols(subResult)).toEqual({ + expect(subResult).toEqual({ data: dataTwo, loading: false, - networkStatus: 7, + networkStatus: NetworkStatus.ready, }); resolve(); }