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 accidental double network call with useLazyQuery on some calls of the execute function #11403

Merged
merged 47 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
6e44067
Provide failing test for #9448
jerelmiller Dec 1, 2023
8d3d0fe
Merge branch 'main' into jerel/issue-9448-lazy-query-double-call
jerelmiller Feb 1, 2024
eb924aa
Add qualifier to test that is specific to no-cache fetch policy
jerelmiller Feb 1, 2024
eda5450
Move no-cache to hook for explictness
jerelmiller Feb 1, 2024
6a9f81a
Fix assertion against incorrect data
jerelmiller Feb 1, 2024
2b271d0
Maintain existing options in execOptionsRef between calls to execute
jerelmiller Feb 1, 2024
c0687ba
Add changeset
jerelmiller Feb 1, 2024
1160142
Test against all fetch policies that guarantee network requests
jerelmiller Feb 1, 2024
4110dab
Merge branch 'main' into jerel/issue-9448-lazy-query-double-call
jerelmiller Feb 1, 2024
eb53ea2
Update size limits
jerelmiller Feb 1, 2024
a334ec4
Remove accidental call to expect.anything() and replace with real value
jerelmiller Feb 1, 2024
760b92d
Resolve with different data when id is null in test
jerelmiller Feb 2, 2024
660b1d2
Update expectation on data returned from 2nd execute call
jerelmiller Feb 2, 2024
b2b61a1
Use result.current to trigger execution
jerelmiller Feb 2, 2024
8a4acbf
Revert change to store previous result in execOptionsRef
jerelmiller Feb 2, 2024
173d976
Don't merge previous variables in execute function when calling witho…
jerelmiller Feb 2, 2024
48af005
Remove unneeded optionsRef
jerelmiller Feb 2, 2024
1bf404c
Pass query as dep to execute useCallback
jerelmiller Feb 2, 2024
590115d
Add initialFetchPolicy to dep on useCallback
jerelmiller Feb 2, 2024
6234179
Update size-limits
jerelmiller Feb 2, 2024
40d86a5
Merge branch 'main' into jerel/issue-9448-lazy-query-double-call
jerelmiller Feb 2, 2024
0d2be0d
Add note about variables merging
jerelmiller Feb 2, 2024
93ff34a
Add new useStableCallback internal hook
jerelmiller Feb 2, 2024
31911f9
Fix for stale closures and stable execute callback
jerelmiller Feb 2, 2024
bf81a2a
Better test stability of execute function and stale closures
jerelmiller Feb 2, 2024
04215dd
Update size limits
jerelmiller Feb 2, 2024
895af15
Add comment on useStableCallback
jerelmiller Feb 2, 2024
16acef0
Add note about stable callbacks in useLazyQuery
jerelmiller Feb 2, 2024
51d79dc
Add one more check for function identity
jerelmiller Feb 2, 2024
8d4ae0e
Remove unneeded fetchCount in test
jerelmiller Feb 2, 2024
b6652ae
Add clarifying update to test name
jerelmiller Feb 2, 2024
2d42a09
Add note about behavior change to changelog
jerelmiller Feb 2, 2024
031f5c5
Minor tweak to comment
jerelmiller Feb 2, 2024
79c3d8a
Update important info in changeset
jerelmiller Feb 6, 2024
7dced46
Merge remote-tracking branch 'origin/main' into jerel/issue-9448-lazy…
jerelmiller Feb 6, 2024
3a51d12
Rename useStableCallback to useEvent
jerelmiller Feb 6, 2024
8a45baa
Make T the function itself in useEvent
jerelmiller Feb 6, 2024
1689136
Revert change to ternary clause
jerelmiller Feb 6, 2024
11ac51f
Use useEvent for execute function callback
jerelmiller Feb 6, 2024
ce512cf
Update size limits
jerelmiller Feb 6, 2024
0495f52
Remove unneeded note on changeset
jerelmiller Feb 6, 2024
7474100
Remove outdated comment
jerelmiller Feb 6, 2024
263c5ca
Restore previous behavior in useLazyQuery but set optionsRef.current …
jerelmiller Feb 7, 2024
24aa08c
Remove useEvent
jerelmiller Feb 7, 2024
5d00c58
Restore comment
jerelmiller Feb 7, 2024
73298b7
Restore old style to prevent diff noise
jerelmiller Feb 7, 2024
95ab853
Merge branch 'main' into jerel/issue-9448-lazy-query-double-call
jerelmiller Feb 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/honest-turtles-move.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion .size-limits.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"dist/apollo-client.min.cjs": 39061,
"dist/apollo-client.min.cjs": 39067,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32559
}
77 changes: 77 additions & 0 deletions src/react/hooks/__tests__/useLazyQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,83 @@ 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) => {
setTimeout(() => {
observer.next({
data: {
user: { id: operation.variables.id || null, name: "John Doe" },
},
});
observer.complete();
}, 20);
});
});

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

const { result } = renderHook(
() => useLazyQuery(query, { fetchPolicy }),
{
wrapper: ({ children }) => (
<ApolloProvider client={client}>{children}</ApolloProvider>
),
}
);

const [execute] = result.current;

await act(() => execute({ variables: { id: "2" } }));
jerelmiller marked this conversation as resolved.
Show resolved Hide resolved

expect(fetchCount).toBe(1);

await waitFor(() => {
expect(result.current[1].data).toEqual({
user: { id: "2", name: "John Doe" },
});
});

expect(fetchCount).toBe(1);

await act(() => execute());
jerelmiller marked this conversation as resolved.
Show resolved Hide resolved

await waitFor(() => {
expect(result.current[1].data).toEqual({
user: { id: "2", name: "John Doe" },
});
});

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

describe("network errors", () => {
async function check(errorPolicy: ErrorPolicy) {
const networkError = new Error("from the network");
Expand Down
4 changes: 3 additions & 1 deletion src/react/hooks/useLazyQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ export function useLazyQuery<
fetchPolicy: executeOptions.fetchPolicy || initialFetchPolicy,
}
: {
fetchPolicy: initialFetchPolicy,
...execOptionsRef.current,
fetchPolicy:
execOptionsRef.current?.fetchPolicy || initialFetchPolicy,
};

const options = mergeOptions(optionsRef.current, {
Expand Down
Loading