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..6b3dbfa8716 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(); @@ -323,19 +325,7 @@ export class QueryData extends OperationData { let result: any = this.observableQueryFields(); const options = this.getOptions(); - // 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). - if (options.skip) { - result = { - ...result, - data: undefined, - error: undefined, - loading: false, - called: true - }; - } else if (this.currentObservable) { - // Fetch the current result (if any) from the store. + if (this.currentObservable) { const currentResult = this.currentObservable.getCurrentResult(); const { data, loading, partial, networkStatus, errors } = currentResult; let { error } = currentResult; @@ -355,7 +345,7 @@ export class QueryData extends OperationData { called: true }; - if (loading) { + if (loading || options.skip) { // Fall through without modifying result... } else if (error) { Object.assign(result, { diff --git a/src/react/hoc/__tests__/queries/skip.test.tsx b/src/react/hoc/__tests__/queries/skip.test.tsx index 9f254236e74..58e3fac279f 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({ @@ -620,19 +625,26 @@ describe('[queries] skip', () => { })( class extends React.Component { render() { + console.log(this.props); switch (++count) { case 1: expect(this.props.data.loading).toBe(true); 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 +652,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 +699,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..cd4877648d1 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -526,7 +526,7 @@ describe('useQuery Hook', () => { break; case 3: expect(loading).toBeFalsy(); - expect(data).toBeUndefined(); + expect(data).toEqual(CAR_RESULT_DATA); break; case 4: throw new Error('Uh oh - we should have stopped polling!'); @@ -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).toEqual(CAR_RESULT_DATA); + expect(networkRequestCount).toBe(1); + break; + default: + reject('too many renders'); + } + + return null; + }; + + render( + + + + ); + + return wait(() => { + expect(renderCount).toBe(3); + }).then(resolve, reject); + }); }); });