Fix network request when using skip/skipToken with useSuspenseQuery i…

…n strict mode (#11769)
apollographql · Apr 10, 2024 · 04132af · 04132af
1 parent 286ff15
commit 04132af
Show file tree

Hide file tree

Showing 5 changed files with 293 additions and 6 deletions.
diff --git a/.changeset/thick-buttons-juggle.md b/.changeset/thick-buttons-juggle.md
@@ -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.
diff --git a/.size-limits.json b/.size-limits.json
@@ -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
 }
diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx
@@ -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();
 

diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx
@@ -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();
 

diff --git a/src/react/internal/cache/QueryReference.ts b/src/react/internal/cache/QueryReference.ts
@@ -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;
       }