Fix accidental double network call with useLazyQuery on some calls of the execute function

.changeset/honest-turtles-move.md

@@ -0,0 +1,11 @@
+---
+"@apollo/client": patch
+---
+
+Fix issue in `useLazyQuery` that results in a double network call when calling the execute function with no arguments after having called it previously with another set of arguments.
+
+### IMPORTANT CHANGE
+
+The query function returned from `useLazyQuery` is no longer an unconditionally stable reference across renders. The function identity changes when options passed to `useLazyQuery` change, excluding callback function options. This change prevents stale closure issues in the query function which may result in subtle bugs.
+
+This change only affects you if the query function in used in a `useEffect` callback. `useEffect` may now fire more often when there are changes to the options passed to `useLazyQuery`.

.size-limits.json

@@ -1,4 +1,4 @@
 {
-  "dist/apollo-client.min.cjs": 39061,
+  "dist/apollo-client.min.cjs": 39264,
   "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32559
 }

docs/source/data/queries.mdx

@@ -269,6 +269,12 @@ The first item in `useLazyQuery`'s return tuple is the query function, and the s
 
 As shown above, you can pass options to the query function just like you pass them to `useLazyQuery` itself. If you pass a particular option to _both_, the value you pass to the query function takes precedence. This is a handy way to pass _default_ options to `useLazyQuery` and then customize those options in the query function.
 
+<Note>
+
+Variables are merged by taking the `variables` passed as options to the hook and merging them with the `variables` passed to the query function. If you do not pass `variables` to the query function, only the `variables` passed to the hook are used in the query execution.
+
+</Note>
+
 For a full list of supported options, see the [API reference](../api/react/hooks/#uselazyquery).
 
 ## Setting a fetch policy

src/react/hooks/__tests__/useLazyQuery.test.tsx

@@ -5,6 +5,7 @@ import { act, render, renderHook, waitFor } from "@testing-library/react";
 
 import {
   ApolloClient,
+  ApolloError,
   ApolloLink,
   ErrorPolicy,
   InMemoryCache,
@@ -19,6 +20,7 @@ import {
   wait,
   tick,
   MockSubscriptionLink,
+  MockLink,
 } from "../../../testing";
 import { useLazyQuery } from "../useLazyQuery";
 import { QueryResult } from "../../types/types";
@@ -1483,6 +1485,274 @@ describe("useLazyQuery Hook", () => {
     expect(fetchCount).toBe(1);
   });
 
+  // https://github.com/apollographql/apollo-client/issues/9448
+  it.each(["network-only", "no-cache", "cache-and-network"] as const)(
+    "does not issue multiple network calls when calling execute again without variables with a %s fetch policy",
+    async (fetchPolicy) => {
+      interface Data {
+        user: { id: string; name: string };
+      }
+
+      interface Variables {
+        id?: string;
+      }
+
+      const query: TypedDocumentNode<Data, Variables> = gql`
+        query UserQuery($id: ID) {
+          user(id: $id) {
+            id
+            name
+          }
+        }
+      `;
+
+      let fetchCount = 0;
+
+      const link = new ApolloLink((operation) => {
+        fetchCount++;
+        return new Observable((observer) => {
+          const { id } = operation.variables;
+
+          setTimeout(() => {
+            observer.next({
+              data: {
+                user:
+                  id ?
+                    { id, name: "John Doe" }
+                  : { id: null, name: "John Default" },
+              },
+            });
+            observer.complete();
+          }, 20);
+        });
+      });
+
+      const client = new ApolloClient({
+        link,
+        cache: new InMemoryCache(),
+      });
+
+      const { result } = renderHook(
+        () => useLazyQuery(query, { fetchPolicy }),
+        {
+          wrapper: ({ children }) => (
+            <ApolloProvider client={client}>{children}</ApolloProvider>
+          ),
+        }
+      );
+
+      await act(() => result.current[0]({ variables: { id: "2" } }));
+
+      expect(fetchCount).toBe(1);
+
+      await waitFor(() => {
+        expect(result.current[1].data).toEqual({
+          user: { id: "2", name: "John Doe" },
+        });
+      });
+
+      expect(fetchCount).toBe(1);
+
+      await act(() => result.current[0]());
+
+      await waitFor(() => {
+        expect(result.current[1].data).toEqual({
+          user: { id: null, name: "John Default" },
+        });
+      });
+
+      expect(fetchCount).toBe(2);
+    }
+  );
+
+  it("maintains stable execute function when passing in dynamic function options", async () => {
+    interface Data {
+      user: { id: string; name: string };
+    }
+
+    interface Variables {
+      id: string;
+    }
+
+    const query: TypedDocumentNode<Data, Variables> = gql`
+      query UserQuery($id: ID!) {
+        user(id: $id) {
+          id
+          name
+        }
+      }
+    `;
+
+    const link = new MockLink([
+      {
+        request: { query, variables: { id: "1" } },
+        result: { data: { user: { id: "1", name: "John Doe" } } },
+        delay: 20,
+      },
+      {
+        request: { query, variables: { id: "2" } },
+        result: { errors: [new GraphQLError("Oops")] },
+        delay: 20,
+      },
+      {
+        request: { query, variables: { id: "3" } },
+        result: { data: { user: { id: "3", name: "Johnny Three" } } },
+        delay: 20,
+        maxUsageCount: Number.POSITIVE_INFINITY,
+      },
+    ]);
+
+    const client = new ApolloClient({ link, cache: new InMemoryCache() });
+
+    let countRef = { current: 0 };
+
+    const trackClosureValue = jest.fn();
+
+    const { result, rerender } = renderHook(
+      () => {
+        let count = countRef.current;
+
+        return useLazyQuery(query, {
+          fetchPolicy: "cache-first",
+          variables: { id: "1" },
+          onCompleted: () => {
+            trackClosureValue("onCompleted", count);
+          },
+          onError: () => {
+            trackClosureValue("onError", count);
+          },
+          skipPollAttempt: () => {
+            trackClosureValue("skipPollAttempt", count);
+            return false;
+          },
+          nextFetchPolicy: (currentFetchPolicy) => {
+            trackClosureValue("nextFetchPolicy", count);
+            return currentFetchPolicy;
+          },
+        });
+      },
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>{children}</ApolloProvider>
+        ),
+      }
+    );
+
+    const [originalExecute] = result.current;
+
+    countRef.current++;
+    rerender();
+
+    expect(result.current[0]).toBe(originalExecute);
+
+    // Check for stale closures with onCompleted
+    await act(() => result.current[0]());
+    await waitFor(() => {
+      expect(result.current[1].data).toEqual({
+        user: { id: "1", name: "John Doe" },
+      });
+    });
+
+    // after fetch
+    expect(trackClosureValue).toHaveBeenNthCalledWith(1, "nextFetchPolicy", 1);
+    expect(trackClosureValue).toHaveBeenNthCalledWith(2, "onCompleted", 1);
+    trackClosureValue.mockClear();
+
+    countRef.current++;
+    rerender();
+
+    expect(result.current[0]).toBe(originalExecute);
+
+    // Check for stale closures with onError
+    await act(() => result.current[0]({ variables: { id: "2" } }));
+    await waitFor(() => {
+      expect(result.current[1].error).toEqual(
+        new ApolloError({ graphQLErrors: [new GraphQLError("Oops")] })
+      );
+    });
+
+    // variables changed
+    expect(trackClosureValue).toHaveBeenNthCalledWith(1, "nextFetchPolicy", 2);
+    // after fetch
+    expect(trackClosureValue).toHaveBeenNthCalledWith(2, "nextFetchPolicy", 2);
+    expect(trackClosureValue).toHaveBeenNthCalledWith(3, "onError", 2);
+    trackClosureValue.mockClear();
+
+    countRef.current++;
+    rerender();
+
+    expect(result.current[0]).toBe(originalExecute);
+
+    await act(() => result.current[0]({ variables: { id: "3" } }));
+    await waitFor(() => {
+      expect(result.current[1].data).toEqual({
+        user: { id: "3", name: "Johnny Three" },
+      });
+    });
+
+    // variables changed
+    expect(trackClosureValue).toHaveBeenNthCalledWith(1, "nextFetchPolicy", 3);
+    // after fetch
+    expect(trackClosureValue).toHaveBeenNthCalledWith(2, "nextFetchPolicy", 3);
+    expect(trackClosureValue).toHaveBeenNthCalledWith(3, "onCompleted", 3);
+    trackClosureValue.mockClear();
+
+    // Test for stale closures for skipPollAttempt
+    result.current[1].startPolling(20);
+    await wait(50);
+    result.current[1].stopPolling();
+
+    expect(trackClosureValue).toHaveBeenCalledWith("skipPollAttempt", 3);
+  });
+
+  it("changes execute function identity when changing non-callback options", async () => {
+    interface Data {
+      user: { id: string; name: string };
+    }
+
+    interface Variables {
+      id: string;
+    }
+
+    const query: TypedDocumentNode<Data, Variables> = gql`
+      query UserQuery($id: ID!) {
+        user(id: $id) {
+          id
+          name
+        }
+      }
+    `;
+
+    const link = new ApolloLink((operation) => {
+      return new Observable((observer) => {
+        setTimeout(() => {
+          observer.next({
+            data: { user: { id: operation.variables.id, name: "John Doe" } },
+          });
+          observer.complete();
+        }, 20);
+      });
+    });
+
+    const client = new ApolloClient({ link, cache: new InMemoryCache() });
+
+    const { result, rerender } = renderHook(
+      ({ id }) => useLazyQuery(query, { variables: { id } }),
+      {
+        initialProps: { id: "1" },
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>{children}</ApolloProvider>
+        ),
+      }
+    );
+
+    const [execute] = result.current;
+
+    rerender({ id: "2" });
+
+    expect(result.current[0]).not.toBe(execute);
+  });
+
   describe("network errors", () => {
     async function check(errorPolicy: ErrorPolicy) {
       const networkError = new Error("from the network");

src/react/hooks/internal/index.ts

@@ -3,4 +3,5 @@ export { useDeepMemo } from "./useDeepMemo.js";
 export { useIsomorphicLayoutEffect } from "./useIsomorphicLayoutEffect.js";
 export { useRenderGuard } from "./useRenderGuard.js";
 export { useLazyRef } from "./useLazyRef.js";
+export { useStableCallback } from "./useStableCallback.js";
 export { __use } from "./__use.js";

src/react/hooks/internal/useStableCallback.ts

@@ -0,0 +1,24 @@
+import React from "rehackt";
+
+/**
+ * A hook that provides a stable reference to a function without suffering from
+ * stale closures. Useful to get a stable function reference for callback
+ * function options in other hooks. This avoids the need for the user to wrap
+ * each callback in a `useCallback`.
+ *
+ * @param unstableCallback - A callback function
+ */
+export function useStableCallback<TArgs extends unknown[], TReturn>(
+  unstableCallback: (...args: TArgs) => TReturn
+) {
+  const callbackRef = React.useRef(unstableCallback);
+
+  React.useLayoutEffect(() => {
+    callbackRef.current = unstableCallback;
+  });
+
+  return React.useCallback((...args: TArgs): TReturn => {
+    const fn = callbackRef.current;
+    return fn(...args);
+  }, []);
+}