diff --git a/CHANGELOG.md b/CHANGELOG.md
index 22f294a0a3c..6a79744fb8d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## Apollo Client 3.6.9 (unreleased)
+
+### Bug Fixes
+
+- Leave `fetchPolicy` unchanged when `skip: true` (or in standby) and `nextFetchPolicy` is available, even if `variables` change.
+ [@benjamn](https://github.com/benjamn) in [#9823](https://github.com/apollographql/apollo-client/pull/9823)
+
## Apollo Client 3.6.8 (2022-06-10)
### Bug Fixes
diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
index 4e4f92846e0..b6db88b8a34 100644
--- a/src/core/ObservableQuery.ts
+++ b/src/core/ObservableQuery.ts
@@ -653,18 +653,19 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
initialFetchPolicy = fetchPolicy,
} = options;
- // 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
- // applying the "cache-and-network" or "network-only" policies would seem
- // to imply. Instead, when the cache reports an update after the initial
- // network request, it may be desirable for subsequent network requests to
- // be triggered only if the cache result is incomplete. To that end, the
- // options.nextFetchPolicy option provides an easy way to update
- // options.fetchPolicy after the initial network request, without having to
- // call observableQuery.setOptions.
-
- if (typeof options.nextFetchPolicy === "function") {
+ if (fetchPolicy === "standby") {
+ // Do nothing, leaving options.fetchPolicy unchanged.
+ } else if (typeof options.nextFetchPolicy === "function") {
+ // 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
+ // applying the "cache-and-network" or "network-only" policies would
+ // seem to imply. Instead, when the cache reports an update after the
+ // initial network request, it may be desirable for subsequent network
+ // requests to be triggered only if the cache result is incomplete. To
+ // that end, the options.nextFetchPolicy option provides an easy way to
+ // update options.fetchPolicy after the initial network request, without
+ // having to call observableQuery.setOptions.
options.fetchPolicy = options.nextFetchPolicy(fetchPolicy, {
reason,
options,
@@ -809,7 +810,11 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
newOptions &&
newOptions.variables &&
!equal(newOptions.variables, oldVariables) &&
- (!newOptions.fetchPolicy || newOptions.fetchPolicy === oldFetchPolicy)
+ // Don't mess with the fetchPolicy if it's currently "standby".
+ options.fetchPolicy !== "standby" &&
+ // If we're changing the fetchPolicy anyway, don't try to change it here
+ // using applyNextFetchPolicy. The explicit options.fetchPolicy wins.
+ options.fetchPolicy === oldFetchPolicy
) {
this.applyNextFetchPolicy("variables-changed", options);
if (newNetworkStatus === void 0) {
diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
index bf9c6613d4d..f8aaafa1af1 100644
--- a/src/core/QueryManager.ts
+++ b/src/core/QueryManager.ts
@@ -21,7 +21,7 @@ import {
asyncMap,
isNonEmptyArray,
Concast,
- ConcastSourcesIterable,
+ ConcastSourcesArray,
makeUniqueId,
isDocumentNode,
isNonNullObject,
@@ -1122,16 +1122,33 @@ export class QueryManager {
// modify its properties here, rather than creating yet another new
// WatchQueryOptions object.
normalized.variables = variables;
- return this.fetchQueryByPolicy(
+
+ const concastSources = this.fetchQueryByPolicy(
queryInfo,
normalized,
networkStatus,
);
+
+ if (
+ // If we're in standby, postpone advancing options.fetchPolicy using
+ // applyNextFetchPolicy.
+ normalized.fetchPolicy !== "standby" &&
+ // The "standby" policy currently returns [] from fetchQueryByPolicy, so
+ // this is another way to detect when nothing was done/fetched.
+ concastSources.length > 0 &&
+ queryInfo.observableQuery
+ ) {
+ queryInfo.observableQuery["applyNextFetchPolicy"]("after-fetch", options);
+ }
+
+ return concastSources;
};
// This cancel function needs to be set before the concast is created,
// in case concast creation synchronously cancels the request.
+ const cleanupCancelFn = () => this.fetchCancelFns.delete(queryId);
this.fetchCancelFns.set(queryId, reason => {
+ cleanupCancelFn();
// This delay ensures the concast variable has been initialized.
setTimeout(() => concast.cancel(reason));
});
@@ -1156,13 +1173,7 @@ export class QueryManager {
: fromVariables(normalized.variables!)
);
- concast.cleanup(() => {
- this.fetchCancelFns.delete(queryId);
-
- if (queryInfo.observableQuery) {
- queryInfo.observableQuery["applyNextFetchPolicy"]("after-fetch", options);
- }
- });
+ concast.promise.then(cleanupCancelFn, cleanupCancelFn);
return concast;
}
@@ -1338,7 +1349,7 @@ export class QueryManager {
// NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
// or setVariables.
networkStatus: NetworkStatus,
- ): ConcastSourcesIterable> {
+ ): ConcastSourcesArray> {
const oldNetworkStatus = queryInfo.networkStatus;
queryInfo.init({
diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts
index a13bdce94c4..9424a64df6b 100644
--- a/src/core/__tests__/ObservableQuery.ts
+++ b/src/core/__tests__/ObservableQuery.ts
@@ -2,7 +2,11 @@ import gql from 'graphql-tag';
import { GraphQLError } from 'graphql';
import { TypedDocumentNode } from '@graphql-typed-document-node/core';
-import { ApolloClient, NetworkStatus } from '../../core';
+import {
+ ApolloClient,
+ NetworkStatus,
+ WatchQueryFetchPolicy,
+} from '../../core';
import { ObservableQuery } from '../ObservableQuery';
import { QueryManager } from '../QueryManager';
@@ -1190,11 +1194,21 @@ describe('ObservableQuery', () => {
},
);
+ const usedFetchPolicies: WatchQueryFetchPolicy[] = [];
const observable = queryManager.watchQuery({
query: queryWithVars,
variables: variables1,
fetchPolicy: 'cache-and-network',
- nextFetchPolicy: 'cache-first',
+ nextFetchPolicy(currentFetchPolicy, info) {
+ if (info.reason === "variables-changed") {
+ return info.initialFetchPolicy;
+ }
+ usedFetchPolicies.push(currentFetchPolicy);
+ if (info.reason === "after-fetch") {
+ return "cache-first";
+ }
+ return currentFetchPolicy;
+ },
notifyOnNetworkStatusChange: true,
});
@@ -1222,7 +1236,7 @@ describe('ObservableQuery', () => {
}).then(result => {
expect(result.data).toEqual(data);
}).catch(reject);
- expect(observable.options.fetchPolicy).toBe('cache-and-network');
+ expect(observable.options.fetchPolicy).toBe('cache-first');
} else if (handleCount === 4) {
expect(result.loading).toBe(true);
expect(result.networkStatus).toBe(NetworkStatus.setVariables);
@@ -1236,7 +1250,7 @@ describe('ObservableQuery', () => {
}).then(result => {
expect(result.data).toEqual(data2);
}).catch(reject);
- expect(observable.options.fetchPolicy).toBe('cache-and-network');
+ expect(observable.options.fetchPolicy).toBe('cache-first');
} else if (handleCount === 6) {
expect(result.data).toEqual(data2);
expect(result.loading).toBe(true);
@@ -1245,6 +1259,14 @@ describe('ObservableQuery', () => {
expect(result.data).toEqual(data2);
expect(result.loading).toBe(false);
expect(observable.options.fetchPolicy).toBe('cache-first');
+
+ expect(usedFetchPolicies).toEqual([
+ "cache-and-network",
+ "network-only",
+ "cache-and-network",
+ "cache-and-network",
+ ]);
+
setTimeout(resolve, 10);
} else {
reject(`too many renders (${handleCount})`);
diff --git a/src/core/__tests__/fetchPolicies.ts b/src/core/__tests__/fetchPolicies.ts
index 1ea0b1a3f0e..76ae26d41fe 100644
--- a/src/core/__tests__/fetchPolicies.ts
+++ b/src/core/__tests__/fetchPolicies.ts
@@ -896,7 +896,7 @@ describe("nextFetchPolicy", () => {
}).catch(reject);
// Changing variables resets the fetchPolicy to its initial value.
- expect(observable.options.fetchPolicy).toBe("network-only");
+ expect(observable.options.fetchPolicy).toBe("cache-first");
} else if (count === 3) {
expect(result.loading).toBe(false);
diff --git a/src/react/hoc/__tests__/queries/loading.test.tsx b/src/react/hoc/__tests__/queries/loading.test.tsx
index b969541e5f7..916bdc651dd 100644
--- a/src/react/hoc/__tests__/queries/loading.test.tsx
+++ b/src/react/hoc/__tests__/queries/loading.test.tsx
@@ -3,7 +3,7 @@ import { render, waitFor } from '@testing-library/react';
import gql from 'graphql-tag';
import { DocumentNode } from 'graphql';
-import { ApolloClient, NetworkStatus } from '../../../../core';
+import { ApolloClient, NetworkStatus, WatchQueryFetchPolicy } from '../../../../core';
import { ApolloProvider } from '../../../context';
import { InMemoryCache as Cache } from '../../../../cache';
import { itAsync, mockSingleLink } from '../../../../testing';
@@ -360,32 +360,46 @@ describe('[queries] loading', () => {
let count = 0;
+ const usedFetchPolicies: WatchQueryFetchPolicy[] = [];
const Container = graphql<{}, Data>(query, {
options: {
fetchPolicy: 'network-only',
- nextFetchPolicy: 'cache-first',
+ nextFetchPolicy(currentFetchPolicy, info) {
+ if (info.reason === "variables-changed") {
+ return info.initialFetchPolicy;
+ }
+ usedFetchPolicies.push(currentFetchPolicy);
+ if (info.reason === "after-fetch") {
+ return "cache-first";
+ }
+ return currentFetchPolicy;
+ }
},
})(
class extends React.Component> {
render() {
+ ++count;
if (count === 1) {
+ expect(this.props.data!.loading).toBe(true);
+ expect(this.props.data!.allPeople).toBeUndefined();
+ } else if (count === 2) {
+ expect(this.props.data!.loading).toBe(false);
+ expect(this.props.data!.allPeople!.people[0].name).toMatch(
+ /Darth Skywalker - /
+ );
// Has data
- setTimeout(() => {
- render(App);
- }, 0);
- }
- if (count === 2) {
+ setTimeout(() => render(App));
+ } else if (count === 3) {
// Loading after remount
- expect(this.props.data!.loading).toBeTruthy();
- }
- if (count === 3) {
+ expect(this.props.data!.loading).toBe(true);
+ expect(this.props.data!.allPeople).toBeUndefined();
+ } else if (count >= 4) {
// Fetched data loading after remount
- expect(this.props.data!.loading).toBeFalsy();
+ expect(this.props.data!.loading).toBe(false);
expect(this.props.data!.allPeople!.people[0].name).toMatch(
/Darth Skywalker - /
);
}
- count += 1;
return null;
}
}
@@ -399,7 +413,14 @@ describe('[queries] loading', () => {
render(App);
- waitFor(() => expect(count).toBe(5)).then(resolve, reject);
+ return waitFor(() => {
+ expect(usedFetchPolicies).toEqual([
+ "network-only",
+ "network-only",
+ "cache-first",
+ ]);
+ expect(count).toBe(6);
+ }).then(resolve, reject);
});
itAsync('correctly sets loading state on remounted component with changed variables', (resolve, reject) => {
diff --git a/src/react/hoc/__tests__/queries/skip.test.tsx b/src/react/hoc/__tests__/queries/skip.test.tsx
index 8c5988b26c7..09416f054f0 100644
--- a/src/react/hoc/__tests__/queries/skip.test.tsx
+++ b/src/react/hoc/__tests__/queries/skip.test.tsx
@@ -670,7 +670,7 @@ describe('[queries] skip', () => {
break;
case 4:
expect(this.props.skip).toBe(false);
- expect(this.props.data!.loading).toBe(true);
+ expect(this.props.data!.loading).toBe(false);
expect(this.props.data.allPeople).toEqual(data.allPeople);
expect(ranQuery).toBe(2);
break;
diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
index 966246308c9..768b4aa4ade 100644
--- a/src/react/hooks/__tests__/useQuery.test.tsx
+++ b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -1037,7 +1037,7 @@ describe('useQuery Hook', () => {
expect(
result.current.observable.options.fetchPolicy
- ).toBe("network-only");
+ ).toBe("cache-first");
expect(result.current.observable.variables).toEqual({
sourceOfVar: "reobserve",
@@ -1094,7 +1094,7 @@ describe('useQuery Hook', () => {
expect(
result.current.observable.options.fetchPolicy
- ).toBe("network-only");
+ ).toBe("cache-first");
expect(result.current.observable.variables).toEqual({
sourceOfVar: "reobserve without variable merge",
diff --git a/src/utilities/observables/Concast.ts b/src/utilities/observables/Concast.ts
index e04bec7b0ef..2a771079014 100644
--- a/src/utilities/observables/Concast.ts
+++ b/src/utilities/observables/Concast.ts
@@ -12,6 +12,7 @@ function isPromiseLike(value: MaybeAsync): value is PromiseLike {
type Source = MaybeAsync>;
export type ConcastSourcesIterable = Iterable>;
+export type ConcastSourcesArray = Array>;
// A Concast observable concatenates the given sources into a single
// non-overlapping sequence of Ts, automatically unwrapping any promises,