Skip to content

Commit

Permalink
useMutation: fix rules of React violations (#11852)
Browse files Browse the repository at this point in the history
* useMutation: fix rules of React violations

* size-limits

* Clean up Prettier, Size-limit, and Api-Extractor

---------

Co-authored-by: phryneas <[email protected]>
  • Loading branch information
phryneas and phryneas authored May 21, 2024
1 parent 45c47be commit d502a69
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/loud-hairs-think.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix a bug where calling the `useMutation` `reset` function would point the hook to an outdated `client` reference.
6 changes: 6 additions & 0 deletions .changeset/mighty-monkeys-explain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@apollo/client": patch
---

Prevent writing to a ref in render in `useMutation`.
As a result, you might encounter problems in the future if you call the mutation's `execute` function during render. Please note that this was never supported behavior, and we strongly recommend against it.
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": 39607,
"dist/apollo-client.min.cjs": 39620,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32821
}
4 changes: 2 additions & 2 deletions src/react/components/__tests__/client/Mutation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,7 @@ describe("General Mutation testing", () => {
if (count === 0) {
expect(result.called).toEqual(false);
expect(result.loading).toEqual(false);
createTodo();
setTimeout(createTodo, 10);
} else if (count === 2 && result) {
expect(result.data).toEqual(data);
setTimeout(() => {
Expand All @@ -1358,7 +1358,7 @@ describe("General Mutation testing", () => {
});
} else if (count === 3) {
expect(result.loading).toEqual(false);
createTodo();
setTimeout(createTodo, 10);
} else if (count === 5) {
expect(result.data).toEqual(data3);
}
Expand Down
4 changes: 3 additions & 1 deletion src/react/hoc/__tests__/mutations/lifecycle.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ describe("graphql(mutation) lifecycle", () => {
class Container extends React.Component<ChildProps<Props>> {
render() {
if (this.props.listId !== 2) return null;
this.props.mutate!().then(() => resolve());
setTimeout(() => {
this.props.mutate!().then(() => resolve());
});
return null;
}
}
Expand Down
17 changes: 10 additions & 7 deletions src/react/hooks/useMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,9 @@ export function useMutation<
options,
});

// TODO: Trying to assign these in a useEffect or useLayoutEffect breaks
// higher-order components.
{
React.useLayoutEffect(() => {
Object.assign(ref.current, { client, options, mutation });
}
});

const execute = React.useCallback(
(
Expand Down Expand Up @@ -221,17 +219,22 @@ export function useMutation<

const reset = React.useCallback(() => {
if (ref.current.isMounted) {
const result = { called: false, loading: false, client };
const result = {
called: false,
loading: false,
client: ref.current.client,
};
Object.assign(ref.current, { mutationId: 0, result });
setResult(result);
}
}, []);

React.useEffect(() => {
ref.current.isMounted = true;
const current = ref.current;
current.isMounted = true;

return () => {
ref.current.isMounted = false;
current.isMounted = false;
};
}, []);

Expand Down

0 comments on commit d502a69

Please sign in to comment.