Skip to content

Commit

Permalink
Fix network request when using skip/skipToken with useSuspenseQuery i…
Browse files Browse the repository at this point in the history
…n strict mode (#11769)
  • Loading branch information
jerelmiller authored Apr 10, 2024
1 parent 286ff15 commit 04132af
Show file tree
Hide file tree
Showing 5 changed files with 293 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/thick-buttons-juggle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix an issue where using `skipToken` or the `skip` option with `useSuspenseQuery` in React's strict mode would perform a network request.
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": 39368,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32634
"dist/apollo-client.min.cjs": 39373,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32636
}
186 changes: 186 additions & 0 deletions src/react/hooks/__tests__/useBackgroundQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2100,6 +2100,192 @@ it("does not make network requests when `skipToken` is used", async () => {
}
});

it("does not make network requests when `skipToken` is used in strict mode", async () => {
const { query, mocks } = setupSimpleCase();
const Profiler = createDefaultProfiler<SimpleCaseData>();
const { SuspenseFallback, ReadQueryHook } =
createDefaultTrackedComponents(Profiler);
const user = userEvent.setup();

let fetchCount = 0;

const link = new ApolloLink((operation) => {
return new Observable((observer) => {
fetchCount++;

const mock = mocks.find(({ request }) =>
equal(request.query, operation.query)
);

if (!mock) {
throw new Error("Could not find mock for operation");
}

observer.next((mock as any).result);
observer.complete();
});
});

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

function App() {
useTrackRenders();
const [skip, setSkip] = React.useState(true);
const [queryRef] = useBackgroundQuery(query, skip ? skipToken : undefined);

return (
<>
<button onClick={() => setSkip((skip) => !skip)}>Toggle skip</button>
<Suspense fallback={<SuspenseFallback />}>
{queryRef && <ReadQueryHook queryRef={queryRef} />}
</Suspense>
</>
);
}

renderWithClient(<App />, {
client,
wrapper: ({ children }) => (
<React.StrictMode>
<Profiler>{children}</Profiler>
</React.StrictMode>
),
});

// initial skipped result
await Profiler.takeRender();
expect(fetchCount).toBe(0);

// Toggle skip to `false`
await act(() => user.click(screen.getByText("Toggle skip")));
await Profiler.takeRender();

{
const { snapshot } = await Profiler.takeRender();

expect(snapshot.result).toEqual({
data: { greeting: "Hello" },
error: undefined,
networkStatus: NetworkStatus.ready,
});
}

expect(fetchCount).toBe(1);

// Toggle skip to `true`
await act(() => user.click(screen.getByText("Toggle skip")));

{
const { snapshot } = await Profiler.takeRender();

expect(snapshot.result).toEqual({
data: { greeting: "Hello" },
error: undefined,
networkStatus: NetworkStatus.ready,
});
}

expect(fetchCount).toBe(1);

await expect(Profiler).not.toRerender();
});

it("does not make network requests when using `skip` option in strict mode", async () => {
const { query, mocks } = setupSimpleCase();
const Profiler = createDefaultProfiler<SimpleCaseData>();
const { SuspenseFallback, ReadQueryHook } =
createDefaultTrackedComponents(Profiler);
const user = userEvent.setup();

let fetchCount = 0;

const link = new ApolloLink((operation) => {
return new Observable((observer) => {
fetchCount++;

const mock = mocks.find(({ request }) =>
equal(request.query, operation.query)
);

if (!mock) {
throw new Error("Could not find mock for operation");
}

observer.next((mock as any).result);
observer.complete();
});
});

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

function App() {
useTrackRenders();
const [skip, setSkip] = React.useState(true);
const [queryRef] = useBackgroundQuery(query, { skip });

return (
<>
<button onClick={() => setSkip((skip) => !skip)}>Toggle skip</button>
<Suspense fallback={<SuspenseFallback />}>
{queryRef && <ReadQueryHook queryRef={queryRef} />}
</Suspense>
</>
);
}

renderWithClient(<App />, {
client,
wrapper: ({ children }) => (
<React.StrictMode>
<Profiler>{children}</Profiler>
</React.StrictMode>
),
});

// initial skipped result
await Profiler.takeRender();
expect(fetchCount).toBe(0);

// Toggle skip to `false`
await act(() => user.click(screen.getByText("Toggle skip")));
await Profiler.takeRender();

{
const { snapshot } = await Profiler.takeRender();

expect(snapshot.result).toEqual({
data: { greeting: "Hello" },
error: undefined,
networkStatus: NetworkStatus.ready,
});
}

expect(fetchCount).toBe(1);

// Toggle skip to `true`
await act(() => user.click(screen.getByText("Toggle skip")));

{
const { snapshot } = await Profiler.takeRender();

expect(snapshot.result).toEqual({
data: { greeting: "Hello" },
error: undefined,
networkStatus: NetworkStatus.ready,
});
}

expect(fetchCount).toBe(1);

await expect(Profiler).not.toRerender();
});

it("result is referentially stable", async () => {
const { query, mocks } = setupSimpleCase();

Expand Down
94 changes: 94 additions & 0 deletions src/react/hooks/__tests__/useSuspenseQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5560,6 +5560,100 @@ describe("useSuspenseQuery", () => {
expect(fetchCount).toBe(1);
});

// https://github.com/apollographql/apollo-client/issues/11768
it("does not make network requests when using `skipToken` with strict mode", async () => {
const { query, mocks } = useVariablesQueryCase();

let fetchCount = 0;

const link = new ApolloLink((operation) => {
return new Observable((observer) => {
fetchCount++;

const mock = mocks.find(({ request }) =>
equal(request.variables, operation.variables)
);

if (!mock) {
throw new Error("Could not find mock for operation");
}

observer.next(mock.result);
observer.complete();
});
});

const { result, rerender } = renderSuspenseHook(
({ skip, id }) =>
useSuspenseQuery(query, skip ? skipToken : { variables: { id } }),
{ mocks, link, strictMode: true, initialProps: { skip: true, id: "1" } }
);

expect(fetchCount).toBe(0);

rerender({ skip: false, id: "1" });

expect(fetchCount).toBe(1);

await waitFor(() => {
expect(result.current).toMatchObject({
...mocks[0].result,
networkStatus: NetworkStatus.ready,
error: undefined,
});
});

rerender({ skip: true, id: "2" });

expect(fetchCount).toBe(1);
});

it("does not make network requests when using `skip` with strict mode", async () => {
const { query, mocks } = useVariablesQueryCase();

let fetchCount = 0;

const link = new ApolloLink((operation) => {
return new Observable((observer) => {
fetchCount++;

const mock = mocks.find(({ request }) =>
equal(request.variables, operation.variables)
);

if (!mock) {
throw new Error("Could not find mock for operation");
}

observer.next(mock.result);
observer.complete();
});
});

const { result, rerender } = renderSuspenseHook(
({ skip, id }) => useSuspenseQuery(query, { skip, variables: { id } }),
{ mocks, link, strictMode: true, initialProps: { skip: true, id: "1" } }
);

expect(fetchCount).toBe(0);

rerender({ skip: false, id: "1" });

expect(fetchCount).toBe(1);

await waitFor(() => {
expect(result.current).toMatchObject({
...mocks[0].result,
networkStatus: NetworkStatus.ready,
error: undefined,
});
});

rerender({ skip: true, id: "2" });

expect(fetchCount).toBe(1);
});

it("`skip` result is referentially stable", async () => {
const { query, mocks } = useSimpleQueryCase();

Expand Down
10 changes: 6 additions & 4 deletions src/react/internal/cache/QueryReference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,18 +207,20 @@ export class InternalQueryReference<TData = unknown> {
const { observable } = this;

const originalFetchPolicy = this.watchQueryOptions.fetchPolicy;
const avoidNetworkRequests =
originalFetchPolicy === "no-cache" || originalFetchPolicy === "standby";

try {
if (originalFetchPolicy !== "no-cache") {
if (avoidNetworkRequests) {
observable.silentSetOptions({ fetchPolicy: "standby" });
} else {
observable.resetLastResults();
observable.silentSetOptions({ fetchPolicy: "cache-first" });
} else {
observable.silentSetOptions({ fetchPolicy: "standby" });
}

this.subscribeToQuery();

if (originalFetchPolicy === "no-cache") {
if (avoidNetworkRequests) {
return;
}

Expand Down

0 comments on commit 04132af

Please sign in to comment.