From 06d98331efec937ff934548a6ef451afa76b7134 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 19 Nov 2020 15:05:30 -0500 Subject: [PATCH 1/3] Cancel notifyTimeout in QueryInfo#mark{Result,Error}. Fixes #7346. --- src/core/QueryInfo.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index e7d9ae74570..69ef5dea972 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -49,6 +49,13 @@ function wrapDestructiveCacheMethod( } } +function cancelNotifyTimeout(info: QueryInfo) { + if (info["notifyTimeout"]) { + clearTimeout(info["notifyTimeout"]); + info["notifyTimeout"] = void 0; + } +} + // A QueryInfo object represents a single query managed by the // QueryManager, which tracks all QueryInfo objects by queryId in its // this.queries Map. QueryInfo objects store the latest results and errors @@ -191,10 +198,7 @@ export class QueryInfo { } notify() { - if (this.notifyTimeout) { - clearTimeout(this.notifyTimeout); - this.notifyTimeout = void 0; - } + cancelNotifyTimeout(this); if (this.shouldNotify()) { this.listeners.forEach(listener => listener(this)); @@ -292,6 +296,11 @@ export class QueryInfo { ) { this.graphQLErrors = isNonEmptyArray(result.errors) ? result.errors : []; + // If there is a pending notify timeout, cancel it because we are + // about to update this.diff to hold the latest data, and we can + // assume the data will be broadcast through some other mechanism. + cancelNotifyTimeout(this); + if (options.fetchPolicy === 'no-cache') { this.diff = { result: result.data, complete: true }; @@ -399,6 +408,8 @@ export class QueryInfo { this.networkStatus = NetworkStatus.error; this.lastWrite = void 0; + cancelNotifyTimeout(this); + if (error.graphQLErrors) { this.graphQLErrors = error.graphQLErrors; } From 46cdf014680b190372397b73838781018c1b3baf Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 19 Nov 2020 15:43:32 -0500 Subject: [PATCH 2/3] Add regression tests for issue #7346. --- src/react/hooks/__tests__/useQuery.test.tsx | 152 +++++++++++++++++++- 1 file changed, 151 insertions(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index ad80a1199de..c1c5faeffd7 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -3,7 +3,7 @@ import { DocumentNode, GraphQLError } from 'graphql'; import gql from 'graphql-tag'; import { render, cleanup, wait } from '@testing-library/react'; -import { ApolloClient, NetworkStatus, TypedDocumentNode } from '../../../core'; +import { ApolloClient, NetworkStatus, TypedDocumentNode, WatchQueryFetchPolicy } from '../../../core'; import { InMemoryCache } from '../../../cache'; import { ApolloProvider } from '../../context'; import { Observable, Reference, concatPagination } from '../../../utilities'; @@ -2439,4 +2439,154 @@ describe('useQuery Hook', () => { }).then(resolve, reject); }); }); + + describe("multiple useQuery calls per component", () => { + type ABFields = { + id: number; + name: string; + }; + + const aQuery: TypedDocumentNode<{ + a: ABFields; + }> = gql`query A { a { id name }}`; + + const bQuery: TypedDocumentNode<{ + b: ABFields; + }> = gql`query B { b { id name }}`; + + const aData = { + a: { + __typename: "A", + id: 65, + name: "ay", + }, + }; + + const bData = { + b: { + __typename: "B", + id: 66, + name: "bee", + }, + }; + + function makeClient() { + return new ApolloClient({ + cache: new InMemoryCache, + link: new ApolloLink(operation => new Observable(observer => { + switch (operation.operationName) { + case "A": + observer.next({ data: aData }); + break; + case "B": + observer.next({ data: bData }); + break; + } + observer.complete(); + })), + }); + } + + function check( + aFetchPolicy: WatchQueryFetchPolicy, + bFetchPolicy: WatchQueryFetchPolicy, + ) { + return ( + resolve: (result: any) => any, + reject: (reason: any) => any, + ) => { + let renderCount = 0; + + function App() { + const a = useQuery(aQuery, { + fetchPolicy: aFetchPolicy, + }); + + const b = useQuery(bQuery, { + fetchPolicy: bFetchPolicy, + }); + + switch (++renderCount) { + case 1: + expect(a.loading).toBe(true); + expect(b.loading).toBe(true); + expect(a.data).toBeUndefined(); + expect(b.data).toBeUndefined(); + break; + case 2: + expect(a.loading).toBe(false); + expect(b.loading).toBe(true); + expect(a.data).toEqual(aData); + expect(b.data).toBeUndefined(); + break; + case 3: + expect(a.loading).toBe(false); + expect(b.loading).toBe(false); + expect(a.data).toEqual(aData); + expect(b.data).toEqual(bData); + break; + default: + reject("too many renders: " + renderCount); + } + + return null; + } + + render( + + + + ); + + return wait(() => { + expect(renderCount).toBe(3); + }).then(resolve, reject); + }; + } + + itAsync("cache-first for both", check( + "cache-first", + "cache-first", + )); + + itAsync("cache-first first, cache-and-network second", check( + "cache-first", + "cache-and-network", + )); + + itAsync("cache-first first, network-only second", check( + "cache-first", + "network-only", + )); + + itAsync("cache-and-network for both", check( + "cache-and-network", + "cache-and-network", + )); + + itAsync("cache-and-network first, cache-first second", check( + "cache-and-network", + "cache-first", + )); + + itAsync("cache-and-network first, network-only second", check( + "cache-and-network", + "network-only", + )); + + itAsync("network-only for both", check( + "network-only", + "network-only", + )); + + itAsync("network-only first, cache-first second", check( + "network-only", + "cache-first", + )); + + itAsync("network-only first, cache-and-network second", check( + "network-only", + "cache-and-network", + )); + }); }); From 7e2e5b407f03d59d0d9fc88303dc373a4939aaf2 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 19 Nov 2020 16:12:52 -0500 Subject: [PATCH 3/3] Mention PR #7347 in CHANGELOG.md. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b05e0936cd..288ab54ac97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,9 @@ - Avoid registering `QueryPromise` when `skip` is `true` during server-side rendering.
[@izumin5210](https://github.com/izumin5210) in [#7310](https://github.com/apollographql/apollo-client/pull/7310) +- Cancel `queryInfo.notifyTimeout` in `QueryInfo#markResult` to prevent unnecessary network requests when using a `FetchPolicy` of `cache-and-network` or `network-only` in a React component with multiple `useQuery` calls.
+ [@benjamn](https://github.com/benjamn) in [#7347](https://github.com/apollographql/apollo-client/pull/7347) + ## Apollo Client 3.2.7 ## Bug Fixes