Skip to content

Commit

Permalink
Cancel queryInfo.notifyTimeout when marking final results. (#7347)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjamn authored Nov 20, 2020
1 parent 5184845 commit d1f0b96
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
- Avoid registering `QueryPromise` when `skip` is `true` during server-side rendering. <br/>
[@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. <br/>
[@benjamn](https://github.com/benjamn) in [#7347](https://github.com/apollographql/apollo-client/pull/7347)

## Apollo Client 3.2.7

## Bug Fixes
Expand Down
19 changes: 15 additions & 4 deletions src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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 };

Expand Down Expand Up @@ -399,6 +408,8 @@ export class QueryInfo {
this.networkStatus = NetworkStatus.error;
this.lastWrite = void 0;

cancelNotifyTimeout(this);

if (error.graphQLErrors) {
this.graphQLErrors = error.graphQLErrors;
}
Expand Down
152 changes: 151 additions & 1 deletion src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
<ApolloProvider client={makeClient()}>
<App/>
</ApolloProvider>
);

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",
));
});
});

0 comments on commit d1f0b96

Please sign in to comment.