Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel queryInfo.notifyTimeout when marking final results. #7347

Merged
merged 3 commits into from
Nov 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",
));
});
});