Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
5 changes: 5 additions & 0 deletions .changeset/chilly-games-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix an issue where `useQuery` would poll with `pollInterval` when `skip` was initialized to `true`.
7 changes: 6 additions & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,12 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
options: { fetchPolicy, pollInterval },
} = this;

if (!pollInterval || !this.hasObservers() || fetchPolicy === "cache-only") {
if (
!pollInterval ||
!this.hasObservers() ||
fetchPolicy === "cache-only" ||
fetchPolicy === "standby"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to add a similar check inside of maybeFetch, too?
Currently all code paths changing any of those seem to be covered, but it seems like it could easily happen that some code path might run out of sync again.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

) {
if (__DEV__) {
if (
!this.didWarnCacheOnlyPolling &&
Expand Down
47 changes: 47 additions & 0 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2357,6 +2357,53 @@ describe("useQuery Hook", () => {
}
});

// https://github.com/apollographql/apollo-client/issues/13154
it("doesn't poll when initially skipped", async () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it("does not poll when initially skipped")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hardanish-Singh I'm fine with it the way it is. The contraction is perfectly fine.

In the future, I'd appreciate if you didn't add these kinds of review comments. They are just noise.

I'm happy to answer specific questions or concerns you may have if you're looking at a PR, but please avoid +1 or nit comments like this. Thanks.

const query = gql`
query {
hello
}
`;

const requestHandler = jest.fn<
ReturnType<ApolloLink.RequestHandler>,
Parameters<ApolloLink.RequestHandler>
>(() => of({ data: { hello: "world" } }));

const cache = new InMemoryCache();
const client = new ApolloClient({
cache,
link: new ApolloLink(requestHandler),
});

using _disabledAct = disableActEnvironment();
const renderStream = await renderHookToSnapshotStream(
() => useQuery(query, { pollInterval: 80, skip: true }),
{
wrapper: ({ children }) => (
<ApolloProvider client={client}>{children}</ApolloProvider>
),
}
);
const { takeSnapshot } = renderStream;

{
const result = await takeSnapshot();

expect(result).toStrictEqualTyped({
data: undefined,
dataState: "empty",
loading: false,
networkStatus: NetworkStatus.ready,
previousData: undefined,
variables: {},
});
}

await expect(takeSnapshot).not.toRerender({ timeout: 100 });
expect(requestHandler).not.toHaveBeenCalled();
});

it("should start polling when changing skipToken to options", async () => {
const query = gql`
{
Expand Down