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

Fix issue where a query may not stop polling after unmount when in Strict mode with cache-and-network fetch policy #11837

Merged
merged 11 commits into from
May 13, 2024
5 changes: 5 additions & 0 deletions .changeset/smooth-spoons-cough.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix an issue where a polled query created in React strict mode may not stop polling after the component unmounts while using the `cache-and-network` fetch policy.
4 changes: 2 additions & 2 deletions .size-limits.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"dist/apollo-client.min.cjs": 39540,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32826
"dist/apollo-client.min.cjs": 39544,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32831
}
2 changes: 1 addition & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
options: { pollInterval },
} = this;

if (!pollInterval) {
if (!pollInterval || !this.hasObservers()) {
Copy link
Member

@phryneas phryneas May 13, 2024

Choose a reason for hiding this comment

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

Are we sure that polling is started correctly in all cases now, even if a subscription starts delayed?

Ah, it is called from reobserveAsConcast.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. Thats when this polling would kick in. When the component unmounted, it triggered a broadcast, which ended up calling this updatePolling function and starting a new poll interval from the observable that wasn't subscribed to.

if (pollingInfo) {
clearTimeout(pollingInfo.timeout);
delete this.pollingInfo;
Expand Down
159 changes: 159 additions & 0 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
MockSubscriptionLink,
mockSingleLink,
tick,
wait,
} from "../../../testing";
import { QueryResult } from "../../types/types";
import { useQuery } from "../useQuery";
Expand Down Expand Up @@ -1887,6 +1888,86 @@ describe("useQuery Hook", () => {
requestSpy.mockRestore();
});

// https://github.com/apollographql/apollo-client/issues/9431
// https://github.com/apollographql/apollo-client/issues/11750
it("stops polling when component unmounts with cache-and-network fetch policy", async () => {
const query: TypedDocumentNode<{ hello: string }> = gql`
query {
hello
}
`;

const mocks = [
{
request: { query },
result: { data: { hello: "world 1" } },
},
{
request: { query },
result: { data: { hello: "world 2" } },
},
{
request: { query },
result: { data: { hello: "world 3" } },
},
];

const cache = new InMemoryCache();

const link = new MockLink(mocks);
const requestSpy = jest.spyOn(link, "request");
const onErrorFn = jest.fn();
link.setOnError(onErrorFn);

const ProfiledHook = profileHook(() =>
useQuery(query, { pollInterval: 10, fetchPolicy: "cache-and-network" })
);

const client = new ApolloClient({
queryDeduplication: false,
link,
cache,
});

const { unmount } = render(<ProfiledHook />, {
wrapper: ({ children }: any) => (
<ApolloProvider client={client}>{children}</ApolloProvider>
),
});

{
const snapshot = await ProfiledHook.takeSnapshot();

expect(snapshot.loading).toBe(true);
expect(snapshot.data).toBeUndefined();
}

{
const snapshot = await ProfiledHook.takeSnapshot();

expect(snapshot.loading).toBe(false);
expect(snapshot.data).toEqual({ hello: "world 1" });
expect(requestSpy).toHaveBeenCalledTimes(1);
}

await wait(10);

{
const snapshot = await ProfiledHook.takeSnapshot();

expect(snapshot.loading).toBe(false);
expect(snapshot.data).toEqual({ hello: "world 2" });
expect(requestSpy).toHaveBeenCalledTimes(2);
}

unmount();

await expect(ProfiledHook).not.toRerender({ timeout: 50 });

expect(requestSpy).toHaveBeenCalledTimes(2);
expect(onErrorFn).toHaveBeenCalledTimes(0);
});

it("should stop polling when component is unmounted in Strict Mode", async () => {
const query = gql`
{
Expand Down Expand Up @@ -1960,6 +2041,84 @@ describe("useQuery Hook", () => {
requestSpy.mockRestore();
});

// https://github.com/apollographql/apollo-client/issues/9431
// https://github.com/apollographql/apollo-client/issues/11750
it("stops polling when component unmounts in strict mode with cache-and-network fetch policy", async () => {
const query: TypedDocumentNode<{ hello: string }> = gql`
query {
hello
}
`;

const mocks = [
{
request: { query },
result: { data: { hello: "world 1" } },
},
{
request: { query },
result: { data: { hello: "world 2" } },
},
{
request: { query },
result: { data: { hello: "world 3" } },
},
];

const cache = new InMemoryCache();

const link = new MockLink(mocks);
const requestSpy = jest.spyOn(link, "request");
const onErrorFn = jest.fn();
link.setOnError(onErrorFn);

const ProfiledHook = profileHook(() =>
useQuery(query, { pollInterval: 10, fetchPolicy: "cache-and-network" })
);

const client = new ApolloClient({ link, cache });

const { unmount } = render(<ProfiledHook />, {
wrapper: ({ children }: any) => (
<React.StrictMode>
<ApolloProvider client={client}>{children}</ApolloProvider>
</React.StrictMode>
),
});

{
const snapshot = await ProfiledHook.takeSnapshot();

expect(snapshot.loading).toBe(true);
expect(snapshot.data).toBeUndefined();
}

{
const snapshot = await ProfiledHook.takeSnapshot();

expect(snapshot.loading).toBe(false);
expect(snapshot.data).toEqual({ hello: "world 1" });
expect(requestSpy).toHaveBeenCalledTimes(1);
}

await wait(10);

{
const snapshot = await ProfiledHook.takeSnapshot();

expect(snapshot.loading).toBe(false);
expect(snapshot.data).toEqual({ hello: "world 2" });
expect(requestSpy).toHaveBeenCalledTimes(2);
}

unmount();

await expect(ProfiledHook).not.toRerender({ timeout: 50 });

expect(requestSpy).toHaveBeenCalledTimes(2);
expect(onErrorFn).toHaveBeenCalledTimes(0);
});

it("should start and stop polling in Strict Mode", async () => {
const query = gql`
{
Expand Down
Loading