diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1e050508cc8..9a1636da2b7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## Apollo Client 3.1.2 (not yet released)
+
+## Bug Fixes
+
+- Avoid making network requests when `skip` is `true`.
+ [@hwillson](https://github.com/hwillson) in [#6752](https://github.com/apollographql/apollo-client/pull/6752)
+
## Apollo Client 3.1.1
## Bug Fixes
diff --git a/src/react/data/QueryData.ts b/src/react/data/QueryData.ts
index ce3e45f20a2..90a24363f1c 100644
--- a/src/react/data/QueryData.ts
+++ b/src/react/data/QueryData.ts
@@ -227,6 +227,8 @@ export class QueryData extends OperationData {
}
private updateObservableQuery() {
+ if (this.getOptions().skip) return;
+
// If we skipped initially, we may not have yet created the observable
if (!this.currentObservable) {
this.initializeObservableQuery();
@@ -326,6 +328,13 @@ export class QueryData extends OperationData {
// When skipping a query (ie. we're not querying for data but still want
// to render children), make sure the `data` is cleared out and
// `loading` is set to `false` (since we aren't loading anything).
+ //
+ // NOTE: We no longer think this is the correct behavior. Skipping should
+ // not automatically set `data` to `undefined`, but instead leave the
+ // previous data in place. In other words, skipping should not mandate
+ // that previously received data is all of a sudden removed. Unfortunately,
+ // changing this is breaking, so we'll have to wait until Apollo Client
+ // 4.0 to address this.
if (options.skip) {
result = {
...result,
diff --git a/src/react/hoc/__tests__/queries/skip.test.tsx b/src/react/hoc/__tests__/queries/skip.test.tsx
index 9f254236e74..24285bbaf54 100644
--- a/src/react/hoc/__tests__/queries/skip.test.tsx
+++ b/src/react/hoc/__tests__/queries/skip.test.tsx
@@ -596,11 +596,16 @@ describe('[queries] skip', () => {
ranQuery++;
return f ? f(o) : null;
}).concat(
- mockSingleLink({
- request: { query },
- result: { data },
- newData: () => ({ data: nextData })
- })
+ mockSingleLink(
+ {
+ request: { query },
+ result: { data },
+ },
+ {
+ request: { query },
+ result: { data: nextData },
+ },
+ )
);
const client = new ApolloClient({
@@ -626,13 +631,19 @@ describe('[queries] skip', () => {
expect(ranQuery).toBe(1);
break;
case 2:
+ // The first batch of data is fetched over the network, and
+ // verified here, followed by telling the component we want to
+ // skip running subsequent queries.
expect(this.props.data.loading).toBe(false);
+ expect(this.props.data.allPeople).toEqual(data.allPeople);
expect(ranQuery).toBe(1);
setTimeout(() => {
this.props.setSkip(true);
});
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.data).toBeUndefined();
expect(ranQuery).toBe(1);
setTimeout(() => {
@@ -640,12 +651,26 @@ describe('[queries] skip', () => {
});
break;
case 4:
- expect(this.props.data!.loading).toBe(true);
- 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();
+ });
break;
case 5:
+ expect(this.props.data!.loading).toBe(true);
+ expect(ranQuery).toBe(2);
+ break;
+ case 6:
+ // The next batch of data has loaded.
expect(this.props.data!.loading).toBe(false);
- expect(ranQuery).toBe(3);
+ expect(this.props.data.allPeople).toEqual(nextData.allPeople);
+ expect(ranQuery).toBe(2);
break;
default:
}
@@ -673,7 +698,7 @@ describe('[queries] skip', () => {
);
await wait(() => {
- expect(count).toEqual(5);
+ expect(count).toEqual(6);
});
});
diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
index ce71b578232..462bd4a6e93 100644
--- a/src/react/hooks/__tests__/useQuery.test.tsx
+++ b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -2162,5 +2162,68 @@ describe('useQuery Hook', () => {
expect(renderCount).toBe(3);
}).then(resolve, reject);
});
+
+ itAsync('should not make network requests when `skip` is `true`', (resolve, reject) => {
+ let networkRequestCount = 0;
+ const link = new ApolloLink((o, f) => {
+ networkRequestCount += 1;
+ return f ? f(o) : null;
+ }).concat(mockSingleLink(
+ {
+ request: {
+ query: CAR_QUERY,
+ variables: { someVar: true }
+ },
+ result: { data: CAR_RESULT_DATA }
+ }
+ ));
+
+ const client = new ApolloClient({
+ link,
+ cache: new InMemoryCache()
+ });
+
+ let renderCount = 0;
+ const Component = () => {
+ const [skip, setSkip] = useState(false);
+ const { loading, data } = useQuery(CAR_QUERY, {
+ fetchPolicy: 'no-cache',
+ skip,
+ variables: { someVar: !skip }
+ });
+
+ switch (++renderCount) {
+ case 1:
+ expect(loading).toBeTruthy();
+ expect(data).toBeUndefined();
+ break;
+ case 2:
+ expect(loading).toBeFalsy();
+ expect(data).toEqual(CAR_RESULT_DATA);
+ expect(networkRequestCount).toBe(1);
+ setTimeout(() => setSkip(true));
+ break;
+ case 3:
+ expect(loading).toBeFalsy();
+ expect(data).toBeUndefined();
+ expect(networkRequestCount).toBe(1);
+ break;
+ default:
+ reject('too many renders');
+ }
+
+ return null;
+ };
+
+ render(
+
+
+
+ );
+
+ return wait(() => {
+ expect(renderCount).toBe(3);
+ }).then(resolve, reject);
+ });
});
});