From 23add8f3b587e83d238118b3eb881e9bfed1d2ea Mon Sep 17 00:00:00 2001 From: hwillson Date: Tue, 3 Sep 2019 08:02:17 -0400 Subject: [PATCH 1/2] Better support for multiple `useLazyQuery` execution function calls Before this commit, calling a `useLazyQuery` exection function multiple times in a row, when using a fetch policy of `network-only`, lead to unexpected results. Only the first network request was submitted as Apollo Client was blocking subsequent requests, since the query, fetch policy and variables remained the same. This commmit adds additional cleanup to the `useLazyQuery` exection function, such that each call will start a new `ObservableQuery` instance. This means each query fired by the `useLazyQuery` execution function is treated as a fully new query. Fixes #3355. --- Changelog.md | 2 + .../hooks/src/__tests__/useLazyQuery.test.tsx | 142 +++++++++++++++--- packages/hooks/src/data/QueryData.ts | 3 + 3 files changed, 127 insertions(+), 20 deletions(-) diff --git a/Changelog.md b/Changelog.md index b90efed345..3681d01928 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,6 +16,8 @@ [@joshalling](https://github.com/joshalling) in [#3417](https://github.com/apollographql/react-apollo/pull/3417) - Prevent inline `onError` and `onCompleted` callbacks from being part of the internal memoization that's used to decide when certain after render units of functionality are run, when using `useQuery`. This fixes issues related to un-necessary component cleanup, like `error` disappearing from results when it should be present.
[@dylanwulf](https://github.com/dylanwulf) in [#3419](https://github.com/apollographql/react-apollo/pull/3419) +- `useLazyQuery`'s execution function can now be called multiple times in a row, and will properly submit network requests each time called, when using a fetch policy of `network-only`.
+ [@hwillson](https://github.com/hwillson) in [#TODO](https://github.com/apollographql/react-apollo/pull/TODO) - Documentation fixes.
[@SeanRoberts](https://github.com/SeanRoberts) in [#3380](https://github.com/apollographql/react-apollo/pull/3380) diff --git a/packages/hooks/src/__tests__/useLazyQuery.test.tsx b/packages/hooks/src/__tests__/useLazyQuery.test.tsx index b8209da69f..4b7062cef8 100644 --- a/packages/hooks/src/__tests__/useLazyQuery.test.tsx +++ b/packages/hooks/src/__tests__/useLazyQuery.test.tsx @@ -1,8 +1,8 @@ -import React, { useState } from 'react'; +import React from 'react'; import { DocumentNode } from 'graphql'; import gql from 'graphql-tag'; import { MockedProvider } from '@apollo/react-testing'; -import { render, cleanup } from '@testing-library/react'; +import { render, wait } from '@testing-library/react'; import { ApolloProvider, useLazyQuery } from '@apollo/react-hooks'; import { ApolloClient } from 'apollo-client'; import { InMemoryCache } from 'apollo-cache-inmemory'; @@ -38,16 +38,16 @@ describe('useLazyQuery Hook', () => { } ]; - afterEach(cleanup); - - it('should hold query execution until manually triggered', done => { + it('should hold query execution until manually triggered', async () => { let renderCount = 0; const Component = () => { const [execute, { loading, data }] = useLazyQuery(CAR_QUERY); switch (renderCount) { case 0: expect(loading).toEqual(false); - execute(); + setTimeout(() => { + execute(); + }); break; case 1: expect(loading).toEqual(true); @@ -55,7 +55,6 @@ describe('useLazyQuery Hook', () => { case 2: expect(loading).toEqual(false); expect(data).toEqual(CAR_RESULT_DATA); - done(); break; default: // Do nothing } @@ -68,11 +67,15 @@ describe('useLazyQuery Hook', () => { ); + + await wait(() => { + expect(renderCount).toBe(3); + }); }); it('should set `called` to false by default', () => { const Component = () => { - const [execute, { loading, called }] = useLazyQuery(CAR_QUERY); + const [, { loading, called }] = useLazyQuery(CAR_QUERY); expect(loading).toBeFalsy(); expect(called).toBeFalsy(); return null; @@ -85,7 +88,7 @@ describe('useLazyQuery Hook', () => { ); }); - it('should set `called` to true after calling the lazy execute function', done => { + it('should set `called` to true after calling the lazy execute function', async () => { let renderCount = 0; const Component = () => { const [execute, { loading, called, data }] = useLazyQuery(CAR_QUERY); @@ -93,7 +96,9 @@ describe('useLazyQuery Hook', () => { case 0: expect(loading).toBeFalsy(); expect(called).toBeFalsy(); - execute(); + setTimeout(() => { + execute(); + }); break; case 1: expect(loading).toBeTruthy(); @@ -103,7 +108,6 @@ describe('useLazyQuery Hook', () => { expect(loading).toEqual(false); expect(called).toBeTruthy(); expect(data).toEqual(CAR_RESULT_DATA); - done(); break; default: // Do nothing } @@ -116,9 +120,13 @@ describe('useLazyQuery Hook', () => { ); + + await wait(() => { + expect(renderCount).toBe(3); + }); }); - it('should override `skip` if lazy mode execution function is called', done => { + it('should override `skip` if lazy mode execution function is called', async () => { let renderCount = 0; const Component = () => { const [execute, { loading, data }] = useLazyQuery(CAR_QUERY, { @@ -127,7 +135,9 @@ describe('useLazyQuery Hook', () => { switch (renderCount) { case 0: expect(loading).toBeFalsy(); - execute(); + setTimeout(() => { + execute(); + }); break; case 1: expect(loading).toBeTruthy(); @@ -135,7 +145,6 @@ describe('useLazyQuery Hook', () => { case 2: expect(loading).toEqual(false); expect(data).toEqual(CAR_RESULT_DATA); - done(); break; default: // Do nothing } @@ -148,12 +157,16 @@ describe('useLazyQuery Hook', () => { ); + + await wait(() => { + expect(renderCount).toBe(3); + }); }); it( 'should use variables defined in hook options (if any), when running ' + 'the lazy execution function', - done => { + async () => { const CAR_QUERY: DocumentNode = gql` query AllCars($year: Int!) { cars(year: $year) @client { @@ -195,7 +208,9 @@ describe('useLazyQuery Hook', () => { switch (renderCount) { case 0: expect(loading).toBeFalsy(); - execute(); + setTimeout(() => { + execute(); + }); break; case 1: expect(loading).toBeTruthy(); @@ -203,7 +218,6 @@ describe('useLazyQuery Hook', () => { case 2: expect(loading).toEqual(false); expect(data.cars).toEqual([CAR_RESULT_DATA[1]]); - done(); break; default: // Do nothing } @@ -216,13 +230,17 @@ describe('useLazyQuery Hook', () => { ); + + await wait(() => { + expect(renderCount).toBe(3); + }); } ); it( 'should use variables passed into lazy execution function, ' + 'overriding similar variables defined in Hook options', - done => { + async () => { const CAR_QUERY: DocumentNode = gql` query AllCars($year: Int!) { cars(year: $year) @client { @@ -264,7 +282,9 @@ describe('useLazyQuery Hook', () => { switch (renderCount) { case 0: expect(loading).toBeFalsy(); - execute({ variables: { year: 2000 } }); + setTimeout(() => { + execute({ variables: { year: 2000 } }); + }); break; case 1: expect(loading).toBeTruthy(); @@ -272,7 +292,6 @@ describe('useLazyQuery Hook', () => { case 2: expect(loading).toEqual(false); expect(data.cars).toEqual([CAR_RESULT_DATA[0]]); - done(); break; default: // Do nothing } @@ -285,6 +304,89 @@ describe('useLazyQuery Hook', () => { ); + + await wait(() => { + expect(renderCount).toBe(3); + }); + } + ); + + it( + 'should fetch data each time the execution function is called, when ' + + 'using a "network-only" fetch policy', + async () => { + const data1 = CAR_RESULT_DATA; + + const data2 = { + cars: [ + { + make: 'Audi', + model: 'SQ5', + vin: 'POWERANDTRUNKSPACE', + __typename: 'Car' + } + ] + }; + + const mocks = [ + { + request: { + query: CAR_QUERY + }, + result: { data: data1 } + }, + { + request: { + query: CAR_QUERY + }, + result: { data: data2 } + } + ]; + + let renderCount = 0; + const Component = () => { + const [execute, { loading, data }] = useLazyQuery(CAR_QUERY, { + fetchPolicy: 'network-only' + }); + switch (renderCount) { + case 0: + expect(loading).toEqual(false); + setTimeout(() => { + execute(); + }); + break; + case 1: + expect(loading).toEqual(true); + break; + case 2: + expect(loading).toEqual(false); + expect(data).toEqual(data1); + setTimeout(() => { + execute(); + }); + break; + case 3: + expect(loading).toEqual(true); + break; + case 4: + expect(loading).toEqual(false); + expect(data).toEqual(data2); + break; + default: // Do nothing + } + renderCount += 1; + return null; + }; + + render( + + + + ); + + await wait(() => { + expect(renderCount).toBe(5); + }); } ); }); diff --git a/packages/hooks/src/data/QueryData.ts b/packages/hooks/src/data/QueryData.ts index bffc33f3cf..5c29955855 100644 --- a/packages/hooks/src/data/QueryData.ts +++ b/packages/hooks/src/data/QueryData.ts @@ -126,6 +126,7 @@ export class QueryData extends OperationData { this.currentObservable.query.resetQueryStoreErrors(); }); } + return this.unmount.bind(this); } @@ -159,6 +160,8 @@ export class QueryData extends OperationData { } private runLazyQuery = (options?: QueryLazyOptions) => { + this.cleanup(); + this.runLazy = true; this.lazyOptions = options; this.forceUpdate(); From 1437a83b0fe409353bac9befdcdff4612062a650 Mon Sep 17 00:00:00 2001 From: hwillson Date: Tue, 3 Sep 2019 08:37:56 -0400 Subject: [PATCH 2/2] Changelog update --- Changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 3681d01928..14a467ae86 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,7 +17,7 @@ - Prevent inline `onError` and `onCompleted` callbacks from being part of the internal memoization that's used to decide when certain after render units of functionality are run, when using `useQuery`. This fixes issues related to un-necessary component cleanup, like `error` disappearing from results when it should be present.
[@dylanwulf](https://github.com/dylanwulf) in [#3419](https://github.com/apollographql/react-apollo/pull/3419) - `useLazyQuery`'s execution function can now be called multiple times in a row, and will properly submit network requests each time called, when using a fetch policy of `network-only`.
- [@hwillson](https://github.com/hwillson) in [#TODO](https://github.com/apollographql/react-apollo/pull/TODO) + [@hwillson](https://github.com/hwillson) in [#3453](https://github.com/apollographql/react-apollo/pull/3453) - Documentation fixes.
[@SeanRoberts](https://github.com/SeanRoberts) in [#3380](https://github.com/apollographql/react-apollo/pull/3380)