From add15f540348a3bb529c66c038c3945b4bd77c04 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Jul 2021 17:50:28 -0400 Subject: [PATCH 01/11] Avoid deleting options.nextFetchPolicy after applying it. Despite my vague suggestion in the removed comments that deleting options.nextFetchPolicy somehow helped make the fetchPolicy transition idempotent (a concern that only matters when nextFetchPolicy is a function), leaving options.nextFetchPolicy intact should also be safe/idempotent in virtually all cases, including every case where nextFetchPolicy is just a string, rather than a function. More importantly, leaving options.nextFetchPolicy intact allows it to be applied more than once, which seems to fix issue #6839. --- src/__tests__/client.ts | 4 +--- src/core/ObservableQuery.ts | 5 ----- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index c810a705fad..a0f822d3e1b 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -3319,7 +3319,6 @@ describe('@connection', () => { // again to perform an additional transition. this.nextFetchPolicy = fetchPolicy => { ++nextFetchPolicyCallCount; - expect(fetchPolicy).toBe("cache-and-network"); return "cache-first"; }; return "cache-and-network"; @@ -3372,9 +3371,8 @@ describe('@connection', () => { client.cache.evict({ fieldName: "count" }); } else if (handleCount === 6) { expect(result.data).toEqual({ count: 2 }); - expect(nextFetchPolicyCallCount).toBe(3); + expect(nextFetchPolicyCallCount).toBe(4); expect(obs.options.fetchPolicy).toBe("cache-first"); - expect(obs.options.nextFetchPolicy).toBeUndefined(); setTimeout(resolve, 50); } else { reject("too many results"); diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index a0825f65888..25bb97179e5 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -732,11 +732,6 @@ export function applyNextFetchPolicy( } = options; if (nextFetchPolicy) { - // The options.nextFetchPolicy transition should happen only once, but it - // should also be possible (though uncommon) for a nextFetchPolicy function - // to set options.nextFetchPolicy to perform an additional transition. - options.nextFetchPolicy = void 0; - // When someone chooses "cache-and-network" or "network-only" as their // initial FetchPolicy, they often do not want future cache updates to // trigger unconditional network requests, which is what repeatedly From 143e2ea29c0f1eeffbee7decc5778291ab6bd770 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Jul 2021 18:52:07 -0400 Subject: [PATCH 02/11] Specify which ObservableQuery operations run independently. ObservableQuery reobserve operations can either modify this.options and replace this.concast with a Concast derived from those modified options, or they can use a (shallow) copy of this.options to create a disposable Concast just for the current reobserve operation, without permanently altering the configuration of the ObservableQuery. This commit clarifies which operations receive fresh options (and do not modify this.concast): NetworkStatus.{refetch,poll,fetchMore}, as decided in one place, by the ObservableQuery#reobserve method. This list is subject to change, and I suspect we may end up wanting to use criteria besides NetworkStatus in some cases. For now, NetworkStatus seems to capture the cases we care about. Since NetworkStatus.poll operations "run independently" now, with their own shallow copy of this.options, the forced fetchPolicy: "network-only" no longer needs to be reset by nextFetchPolicy. More generally, since polling no longer modifies the ObservableQuery options at all, the interaction betwee polling and other kinds of network fetches should be less complicated after this commit. --- src/core/ObservableQuery.ts | 44 +++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 25bb97179e5..8cdcc9a7551 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -586,7 +586,6 @@ once, rather than every time you call fetchMore.`); if (!isNetworkRequestInFlight(this.queryInfo.networkStatus)) { this.reobserve({ fetchPolicy: "network-only", - nextFetchPolicy: this.options.fetchPolicy || "cache-first", }, NetworkStatus.poll).then(poll, poll); } else { poll(); @@ -622,26 +621,38 @@ once, rather than every time you call fetchMore.`); newNetworkStatus?: NetworkStatus, ): Promise> { this.isTornDown = false; - let options: WatchQueryOptions; - if (newNetworkStatus === NetworkStatus.refetch) { - options = Object.assign({}, this.options, compact(newOptions)); - } else { - if (newOptions) { - Object.assign(this.options, compact(newOptions)); - } + const useDisposableConcast = + // Refetching uses a disposable Concast to allow refetches using different + // options/variables, without permanently altering the options of the + // original ObservableQuery. + newNetworkStatus === NetworkStatus.refetch || + // The fetchMore method does not actually call the reobserve method, but, + // if it did, it would definitely use a disposable Concast. + newNetworkStatus === NetworkStatus.fetchMore || + // Polling uses a disposable Concast so the polling options (which force + // fetchPolicy to be "network-only") won't override the original options. + newNetworkStatus === NetworkStatus.poll; + + const options = useDisposableConcast + // Disposable Concast fetches receive a shallow copy of this.options + // (merged with newOptions), leaving this.options unmodified. + ? compact(this.options, newOptions) + : Object.assign(this.options, compact(newOptions)); + + // We can skip calling updatePolling if we're not changing this.options. + if (!useDisposableConcast) { this.updatePolling(); - options = this.options; } const concast = this.fetch(options, newNetworkStatus); - if (newNetworkStatus !== NetworkStatus.refetch) { - // We use the {add,remove}Observer methods directly to avoid - // wrapping observer with an unnecessary SubscriptionObserver - // object, in part so that we can remove it here without triggering - // any unsubscriptions, because we just want to ignore the old - // observable, not prematurely shut it down, since other consumers - // may be awaiting this.concast.promise. + + if (!useDisposableConcast) { + // We use the {add,remove}Observer methods directly to avoid wrapping + // observer with an unnecessary SubscriptionObserver object, in part so + // that we can remove it here without triggering any unsubscriptions, + // because we just want to ignore the old observable, not prematurely shut + // it down, since other consumers may be awaiting this.concast.promise. if (this.concast) { this.concast.removeObserver(this.observer, true); } @@ -650,6 +661,7 @@ once, rather than every time you call fetchMore.`); } concast.addObserver(this.observer); + return concast.promise; } From 231ffbd204650b255fc62ca7dbd1bfe76202d4b6 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Jul 2021 19:16:52 -0400 Subject: [PATCH 03/11] Reset options.fetchPolicy in ObservableQuery#setVariables. This change seems consistent with the goals of #7437, though variables can be changed without calling ObservableQuery#setVariables. --- src/core/ObservableQuery.ts | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 8cdcc9a7551..53839e2708a 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -21,6 +21,7 @@ import { WatchQueryOptions, FetchMoreQueryOptions, SubscribeToMoreOptions, + WatchQueryFetchPolicy, } from './watchQueryOptions'; import { QueryInfo } from './QueryInfo'; @@ -57,6 +58,10 @@ export class ObservableQuery< return this.options.variables; } + // Original value of this.options.fetchPolicy (defaulting to "cache-first"), + // from whenever the ObservableQuery was first created. + public initialFetchPolicy: WatchQueryFetchPolicy; + private isTornDown: boolean; private queryManager: QueryManager; private observers = new Set>>(); @@ -129,6 +134,8 @@ export class ObservableQuery< const opDef = getOperationDefinition(options.query); this.queryName = opDef && opDef.name && opDef.name.value; + this.initialFetchPolicy = options.fetchPolicy || "cache-first"; + // related classes this.queryManager = queryManager; this.queryInfo = queryInfo; @@ -475,23 +482,11 @@ once, rather than every time you call fetchMore.`); return Promise.resolve(); } - let { fetchPolicy = 'cache-first' } = this.options; - const reobserveOptions: Partial> = { - fetchPolicy, + return this.reobserve({ + // Reset options.fetchPolicy to its original value. + fetchPolicy: this.initialFetchPolicy, variables, - }; - - if (fetchPolicy !== 'cache-first' && - fetchPolicy !== 'no-cache' && - fetchPolicy !== 'network-only') { - reobserveOptions.fetchPolicy = 'cache-and-network'; - reobserveOptions.nextFetchPolicy = fetchPolicy; - } - - return this.reobserve( - reobserveOptions, - NetworkStatus.setVariables, - ); + }, NetworkStatus.setVariables); } public updateQuery( From 1ab0161871b3bdafb6b9c0feedf852e395edaad0 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Jul 2021 19:32:57 -0400 Subject: [PATCH 04/11] Improve ObservableQuery nextFetchPolicy test. --- src/core/__tests__/ObservableQuery.ts | 35 +++++++++++++++++++++------ 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index d0a2b27175c..d2399db82aa 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -978,22 +978,43 @@ describe('ObservableQuery', () => { const mocks = mockFetchQuery(queryManager); - subscribeAndCount(reject, observable, handleCount => { - if (handleCount === 1) { + subscribeAndCount(reject, observable, (count, result) => { + if (count === 1) { + expect(result).toEqual({ + loading: false, + networkStatus: NetworkStatus.ready, + data: dataOne, + }); + observable.refetch(differentVariables); - } else if (handleCount === 2) { + + } else if (count === 2) { + expect(result).toEqual({ + loading: false, + networkStatus: NetworkStatus.ready, + data: dataTwo, + }); + const fqbpCalls = mocks.fetchQueryByPolicy.mock.calls; expect(fqbpCalls.length).toBe(2); + expect(fqbpCalls[0][1].fetchPolicy).toEqual('cache-first'); expect(fqbpCalls[1][1].fetchPolicy).toEqual('network-only'); + + const fqoCalls = mocks.fetchQueryObservable.mock.calls; + expect(fqoCalls.length).toBe(2); + expect(fqoCalls[0][1].fetchPolicy).toEqual('cache-first'); + expect(fqoCalls[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, thanks to options.nextFetchPolicy. 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(); + + // Give the test time to fail if more results are delivered. + setTimeout(resolve, 50); + } else { + reject(new Error(`too many results (${count}, ${result})`)); } }); }); From 1dcc2e8a1a32aed3e4fd7cb431bcc4633a9fe0df Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Jul 2021 19:30:53 -0400 Subject: [PATCH 05/11] Avoid specifying options.nextFetchPolicy for refetch operations. Specifying options.nextFetchPolicy is unnecessary because refetch operations receive a (disposable) copy of the ObservableQuery options, so options.fetchPolicy does not need to be reset. --- src/core/ObservableQuery.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 53839e2708a..298483a82a4 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -290,8 +290,6 @@ export class ObservableQuery< reobserveOptions.fetchPolicy = 'no-cache'; } else if (fetchPolicy !== 'cache-and-network') { reobserveOptions.fetchPolicy = 'network-only'; - // Go back to the original options.fetchPolicy after this refetch. - reobserveOptions.nextFetchPolicy = fetchPolicy || "cache-first"; } if (variables && !equal(this.options.variables, variables)) { From 4bd46edeea9ec2cfacfed29491a44abb7e0aa2be Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 29 Jun 2021 16:36:04 -0400 Subject: [PATCH 06/11] Avoid calling applyNextFetchPolicy in prepareObservableQueryOptions. https://github.com/apollographql/apollo-client/issues/8426#issuecomment-869937683 --- src/react/data/QueryData.ts | 3 -- src/react/hoc/__tests__/queries/skip.test.tsx | 47 ++++++++++++++----- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/react/data/QueryData.ts b/src/react/data/QueryData.ts index 9d97f7283e5..8014c13a989 100644 --- a/src/react/data/QueryData.ts +++ b/src/react/data/QueryData.ts @@ -8,7 +8,6 @@ import { FetchMoreQueryOptions, SubscribeToMoreOptions, ObservableQuery, - applyNextFetchPolicy, FetchMoreOptions, UpdateQueryOptions, DocumentNode, @@ -199,8 +198,6 @@ export class QueryData extends OperationData< options.fetchPolicy === 'cache-and-network') ) { options.fetchPolicy = 'cache-first'; - } else if (options.nextFetchPolicy && this.currentObservable) { - applyNextFetchPolicy(options); } return { diff --git a/src/react/hoc/__tests__/queries/skip.test.tsx b/src/react/hoc/__tests__/queries/skip.test.tsx index ff0d0b1a3dc..8b6a960da18 100644 --- a/src/react/hoc/__tests__/queries/skip.test.tsx +++ b/src/react/hoc/__tests__/queries/skip.test.tsx @@ -589,6 +589,7 @@ describe('[queries] skip', () => { const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } }; const nextData = { allPeople: { people: [{ name: 'Anakin Skywalker' }] } }; + const finalData = { allPeople: { people: [{ name: 'Darth Vader' }] } }; let ranQuery = 0; @@ -605,6 +606,14 @@ describe('[queries] skip', () => { request: { query }, result: { data: nextData }, }, + { + request: { query }, + result: { data: nextData }, + }, + { + request: { query }, + result: { data: finalData }, + }, ) ); @@ -625,6 +634,8 @@ describe('[queries] skip', () => { })( class extends React.Component { render() { + expect(this.props.data?.error).toBeUndefined(); + switch (++count) { case 1: expect(this.props.data.loading).toBe(true); @@ -639,40 +650,52 @@ describe('[queries] skip', () => { expect(ranQuery).toBe(1); setTimeout(() => { this.props.setSkip(true); - }); + }, 10); break; case 3: // This render is triggered after setting skip to true. Now // let's set skip to false to re-trigger the query. + expect(this.props.skip).toBe(true); expect(this.props.data).toBeUndefined(); expect(ranQuery).toBe(1); setTimeout(() => { this.props.setSkip(false); - }); + }, 10); break; case 4: + expect(this.props.skip).toBe(false); + expect(this.props.data!.loading).toBe(true); + expect(this.props.data.allPeople).toEqual(data.allPeople); + expect(ranQuery).toBe(2); + break; + case 5: + expect(this.props.skip).toBe(false); + expect(this.props.data!.loading).toBe(false); + expect(this.props.data.allPeople).toEqual(nextData.allPeople); + expect(ranQuery).toBe(3); // Since the `nextFetchPolicy` was set to `cache-first`, our // query isn't loading as it's able to find the result of the // query directly from the cache. Let's trigger a refetch // to manually load the next batch of data. - expect(this.props.data!.loading).toBe(false); - expect(this.props.data.allPeople).toEqual(data.allPeople); - expect(ranQuery).toBe(1); setTimeout(() => { this.props.data.refetch(); - }); + }, 10); break; - case 5: + case 6: + expect(this.props.skip).toBe(false); expect(this.props.data!.loading).toBe(true); - expect(ranQuery).toBe(2); + expect(this.props.data.allPeople).toEqual(nextData.allPeople); + expect(ranQuery).toBe(4); break; - case 6: + case 7: // The next batch of data has loaded. + expect(this.props.skip).toBe(false); expect(this.props.data!.loading).toBe(false); - expect(this.props.data.allPeople).toEqual(nextData.allPeople); - expect(ranQuery).toBe(2); + expect(this.props.data.allPeople).toEqual(finalData.allPeople); + expect(ranQuery).toBe(4); break; default: + throw new Error(`too many renders (${count})`); } return null; } @@ -698,7 +721,7 @@ describe('[queries] skip', () => { ); await wait(() => { - expect(count).toEqual(6); + expect(count).toEqual(7); }); }); From bdac212592472dd040d6e7a902d052de26793ce7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 7 Jul 2021 19:41:05 -0400 Subject: [PATCH 07/11] Improve type for QueryData["previous"]["observableQueryOptions"]. --- src/react/data/QueryData.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/react/data/QueryData.ts b/src/react/data/QueryData.ts index 8014c13a989..b6d05cde5ea 100644 --- a/src/react/data/QueryData.ts +++ b/src/react/data/QueryData.ts @@ -28,6 +28,9 @@ import { } from '../types/types'; import { OperationData } from './OperationData'; +type ObservableQueryOptions = + ReturnType["prepareObservableQueryOptions"]>; + export class QueryData extends OperationData< QueryDataOptions > { @@ -39,7 +42,7 @@ export class QueryData extends OperationData< private previous: { client?: ApolloClient; query?: DocumentNode | TypedDocumentNode; - observableQueryOptions?: {}; + observableQueryOptions?: ObservableQueryOptions; result?: QueryResult; loading?: boolean; options?: QueryDataOptions; @@ -222,7 +225,7 @@ export class QueryData extends OperationData< this.previous.observableQueryOptions = { ...observableQueryOptions, - children: null + children: void 0, }; this.currentObservable = this.refreshClient().client.watchQuery({ ...observableQueryOptions @@ -246,7 +249,7 @@ export class QueryData extends OperationData< const newObservableQueryOptions = { ...this.prepareObservableQueryOptions(), - children: null + children: void 0, }; if (this.getOptions().skip) { From 83703c2bebe483a29161d284f8b4ca2d44e81c4a Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 7 Jul 2021 19:39:23 -0400 Subject: [PATCH 08/11] Reliably reset fetchPolicy to original value when variables change. --- src/core/ObservableQuery.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 298483a82a4..d32122ee0b5 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -627,15 +627,32 @@ once, rather than every time you call fetchMore.`); // fetchPolicy to be "network-only") won't override the original options. newNetworkStatus === NetworkStatus.poll; + // Save the old variables, since Object.assign may modify them below. + const oldVariables = this.options.variables; + const options = useDisposableConcast // Disposable Concast fetches receive a shallow copy of this.options // (merged with newOptions), leaving this.options unmodified. ? compact(this.options, newOptions) : Object.assign(this.options, compact(newOptions)); - // We can skip calling updatePolling if we're not changing this.options. if (!useDisposableConcast) { + // We can skip calling updatePolling if we're not changing this.options. this.updatePolling(); + + // Reset options.fetchPolicy to its original value when variables change, + // unless a new fetchPolicy was provided by newOptions. + if ( + newOptions && + newOptions.variables && + !newOptions.fetchPolicy && + !equal(newOptions.variables, oldVariables) + ) { + options.fetchPolicy = this.initialFetchPolicy; + if (newNetworkStatus === void 0) { + newNetworkStatus = NetworkStatus.setVariables; + } + } } const concast = this.fetch(options, newNetworkStatus); From 5d47432181fce23c3d89f6c0a15baa09014c7201 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 8 Jul 2021 15:45:16 -0400 Subject: [PATCH 09/11] Test that fetchPolicy is reset when variables change. --- src/core/__tests__/ObservableQuery.ts | 112 ++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index d2399db82aa..d1b585fe71e 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -1138,6 +1138,118 @@ describe('ObservableQuery', () => { }); }); + itAsync('resets fetchPolicy when variables change when using nextFetchPolicy', (resolve, reject) => { + // This query and variables are copied from react-apollo + const queryWithVars = gql` + query people($first: Int) { + allPeople(first: $first) { + people { + name + } + } + } + `; + + const data = { allPeople: { people: [{ name: 'Luke Skywalker' }] } }; + const variables1 = { first: 0 }; + + const data2 = { allPeople: { people: [{ name: 'Leia Skywalker' }] } }; + const variables2 = { first: 1 }; + + const queryManager = mockQueryManager( + reject, + { + request: { + query: queryWithVars, + variables: variables1, + }, + result: { data }, + }, + { + request: { + query: queryWithVars, + variables: variables2, + }, + result: { data: data2 }, + }, + { + request: { + query: queryWithVars, + variables: variables1, + }, + result: { data }, + }, + { + request: { + query: queryWithVars, + variables: variables2, + }, + result: { data: data2 }, + }, + ); + + const observable = queryManager.watchQuery({ + query: queryWithVars, + variables: variables1, + fetchPolicy: 'cache-and-network', + nextFetchPolicy: 'cache-first', + notifyOnNetworkStatusChange: true, + }); + + expect(observable.options.fetchPolicy).toBe('cache-and-network'); + expect(observable.initialFetchPolicy).toBe('cache-and-network'); + + subscribeAndCount(reject, observable, (handleCount, result) => { + expect(result.error).toBeUndefined(); + + if (handleCount === 1) { + expect(result.data).toEqual(data); + expect(result.loading).toBe(false); + expect(observable.options.fetchPolicy).toBe('cache-first'); + observable.refetch(variables2); + } else if (handleCount === 2) { + expect(result.loading).toBe(true); + expect(result.networkStatus).toBe(NetworkStatus.setVariables); + expect(observable.options.fetchPolicy).toBe('cache-first'); + } else if (handleCount === 3) { + expect(result.data).toEqual(data2); + expect(result.loading).toBe(false); + expect(observable.options.fetchPolicy).toBe('cache-first'); + observable.setOptions({ + variables: variables1, + }).then(result => { + expect(result.data).toEqual(data); + }).catch(reject); + expect(observable.options.fetchPolicy).toBe('cache-and-network'); + } else if (handleCount === 4) { + expect(result.loading).toBe(true); + expect(result.networkStatus).toBe(NetworkStatus.setVariables); + expect(observable.options.fetchPolicy).toBe('cache-first'); + } else if (handleCount === 5) { + expect(result.data).toEqual(data); + expect(result.loading).toBe(false); + expect(observable.options.fetchPolicy).toBe('cache-first'); + observable.reobserve({ + variables: variables2, + }).then(result => { + expect(result.data).toEqual(data2); + }).catch(reject); + expect(observable.options.fetchPolicy).toBe('cache-and-network'); + } else if (handleCount === 6) { + expect(result.data).toEqual(data2); + expect(result.loading).toBe(true); + expect(observable.options.fetchPolicy).toBe('cache-first'); + } else if (handleCount === 7) { + expect(result.data).toEqual(data2); + expect(result.loading).toBe(false); + expect(observable.options.fetchPolicy).toBe('cache-first'); + setTimeout(resolve, 10); + } else { + reject(`too many renders (${handleCount})`); + } + }); + }); + itAsync('cache-and-network refetch should run @client(always: true) resolvers when network request fails', (resolve, reject) => { const query = gql` query MixedQuery { From 89126b625b17b5ad38c647e91a901f92d4cc4f05 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 9 Jul 2021 11:44:15 -0400 Subject: [PATCH 10/11] Keep ObservableQuery initialFetchPolicy member private for now. https://github.com/apollographql/apollo-client/pull/8465#discussion_r666566193 --- src/core/ObservableQuery.ts | 2 +- src/core/__tests__/ObservableQuery.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index d32122ee0b5..0ca64231bb8 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -60,7 +60,7 @@ export class ObservableQuery< // Original value of this.options.fetchPolicy (defaulting to "cache-first"), // from whenever the ObservableQuery was first created. - public initialFetchPolicy: WatchQueryFetchPolicy; + private initialFetchPolicy: WatchQueryFetchPolicy; private isTornDown: boolean; private queryManager: QueryManager; diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index d1b585fe71e..33a2ad5bbbb 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -1197,7 +1197,7 @@ describe('ObservableQuery', () => { }); expect(observable.options.fetchPolicy).toBe('cache-and-network'); - expect(observable.initialFetchPolicy).toBe('cache-and-network'); + expect(observable["initialFetchPolicy"]).toBe('cache-and-network'); subscribeAndCount(reject, observable, (handleCount, result) => { expect(result.error).toBeUndefined(); From 8a98eb95b62efe18c8708e5a3eb2bd508d4544cb Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 9 Jul 2021 11:57:29 -0400 Subject: [PATCH 11/11] Mention PR #8465 in CHANGELOG.md, as a "potentially disruptive" change. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ec48754a2b..ac9171a2e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ - Log non-fatal `invariant.error` message when fields are missing from result objects written into `InMemoryCache`, rather than throwing an exception. While this change relaxes an exception to be merely an error message, which is usually a backwards-compatible change, the error messages are logged in more cases now than the exception was previously thrown, and those new error messages may be worth investigating to discover potential problems in your application. The errors are not displayed for `@client`-only fields, so adding `@client` is one way to handle/hide the errors for local-only fields. Another general strategy is to use a more precise query to write specific subsets of data into the cache, rather than reusing a larger query that contains fields not present in the written `data`.
[@benjamn](https://github.com/benjamn) in [#8416](https://github.com/apollographql/apollo-client/pull/8416) +- The [`nextFetchPolicy`](https://github.com/apollographql/apollo-client/pull/6893) option for `client.watchQuery` and `useQuery` will no longer be removed from the `options` object after it has been applied, and instead will continue to be applied any time `options.fetchPolicy` is reset to another value, until/unless the `options.nextFetchPolicy` property is removed from `options`.
+ [@benjamn](https://github.com/benjamn) in [#8465](https://github.com/apollographql/apollo-client/pull/8465) + ### Improvements - `InMemoryCache` now _guarantees_ that any two result objects returned by the cache (from `readQuery`, `readFragment`, etc.) will be referentially equal (`===`) if they are deeply equal. Previously, `===` equality was often achievable for results for the same query, on a best-effort basis. Now, equivalent result objects will be automatically shared among the result trees of completely different queries. This guarantee is important for taking full advantage of optimistic updates that correctly guess the final data, and for "pure" UI components that can skip re-rendering when their input data are unchanged.