From b0c4f3ad8198981a229b46dc430345a76e577e9c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 7 Feb 2024 09:45:20 -0700 Subject: [PATCH] Fix accidental double network call with `useLazyQuery` on some calls of the execute function (#11403) Co-authored-by: Maria Elisabeth Schreiber Co-authored-by: Lenz Weber-Tronic --- .changeset/honest-turtles-move.md | 5 + docs/source/data/queries.mdx | 6 + .../hooks/__tests__/useLazyQuery.test.tsx | 270 ++++++++++++++++++ src/react/hooks/useLazyQuery.ts | 2 +- 4 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 .changeset/honest-turtles-move.md diff --git a/.changeset/honest-turtles-move.md b/.changeset/honest-turtles-move.md new file mode 100644 index 00000000000..abd3f4d01f9 --- /dev/null +++ b/.changeset/honest-turtles-move.md @@ -0,0 +1,5 @@ +--- +"@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. diff --git a/docs/source/data/queries.mdx b/docs/source/data/queries.mdx index d827a478af4..b28e17dab82 100644 --- a/docs/source/data/queries.mdx +++ b/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. + + +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. + + + For a full list of supported options, see the [API reference](../api/react/hooks/#uselazyquery). ## Setting a fetch policy diff --git a/src/react/hooks/__tests__/useLazyQuery.test.tsx b/src/react/hooks/__tests__/useLazyQuery.test.tsx index 150ce125fca..34981cf9b4e 100644 --- a/src/react/hooks/__tests__/useLazyQuery.test.tsx +++ b/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 = 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 }) => ( + {children} + ), + } + ); + + 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 = 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 }) => ( + {children} + ), + } + ); + + 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("maintains stable execute function identity when changing non-callback options", async () => { + interface Data { + user: { id: string; name: string }; + } + + interface Variables { + id: string; + } + + const query: TypedDocumentNode = 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 }) => ( + {children} + ), + } + ); + + const [execute] = result.current; + + rerender({ id: "2" }); + + expect(result.current[0]).toBe(execute); + }); + describe("network errors", () => { async function check(errorPolicy: ErrorPolicy) { const networkError = new Error("from the network"); diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 6c58c86e3ce..909bf8a24da 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -78,7 +78,7 @@ export function useLazyQuery< // Use refs to track options and the used query to ensure the `execute` // function remains referentially stable between renders. - optionsRef.current = merged; + optionsRef.current = options; queryRef.current = document; const internalState = useInternalState(