Skip to content

Commit

Permalink
delay state updates until after the mutation execution promise resolves
Browse files Browse the repository at this point in the history
Fixes #8137
  • Loading branch information
brainkim committed Aug 18, 2021
1 parent 73569ef commit cf90a3d
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 48 deletions.
38 changes: 38 additions & 0 deletions src/react/hooks/__tests__/useMutation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,44 @@ describe('useMutation Hook', () => {
});
});

it('the useMutation promise should resolve before the hook updates', async () => {
const variables = {
description: 'Get milk!'
};

const mocks = [
{
request: {
query: CREATE_TODO_MUTATION,
variables
},
result: { data: CREATE_TODO_RESULT }
}
];

const { result, waitForNextUpdate } = renderHook(
() => useMutation(CREATE_TODO_MUTATION),
{ wrapper: ({ children }) => (
<MockedProvider mocks={mocks}>
{children}
</MockedProvider>
)},
);

const mutate = result.current[0];

await act(async () => {
await mutate({ variables });
});

expect(result.current[1].loading).toBe(true);
expect(result.current[1].data).toBe(undefined);
await waitForNextUpdate();

expect(result.current[1].loading).toBe(false);
expect(result.current[1].data).toEqual(CREATE_TODO_RESULT);
});

describe('mutate function upon error', () => {
it('resolves with the resulting data and errors', async () => {
const variables = {
Expand Down
106 changes: 58 additions & 48 deletions src/react/hooks/useMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export function useMutation<
const ref = useRef({
result,
mutationId: 0,
baseOptions: { ...options, mutation },
isMounted: true,
});

Expand All @@ -50,7 +51,7 @@ export function useMutation<
TCache
> = {},
) => {
if (!ref.current.result.loading && !options?.ignoreResults) {
if (!ref.current.result.loading && !ref.current.baseOptions?.ignoreResults) {
setResult(ref.current.result = {
loading: true,
error: void 0,
Expand All @@ -62,64 +63,73 @@ export function useMutation<

const mutationId = ++ref.current.mutationId;
const clientOptions = mergeOptions(
{ ...options, mutation },
ref.current.baseOptions,
executeOptions as any,
);
return client.mutate(clientOptions).then((response) => {
const { data, errors } = response;
const error =
errors && errors.length > 0
? new ApolloError({ graphQLErrors: errors })
: void 0;

if (mutationId === ref.current.mutationId && !options?.ignoreResults) {
const result = {
called: true,
loading: false,
data,
error,
client,
};

if (
ref.current.isMounted &&
!equal(ref.current.result, result)
) {
ref.current.result = result;
setResult(result);
}
}

options?.onCompleted?.(data!);
const execution = client.mutate(clientOptions).then((response) =>{
ref.current.baseOptions?.onCompleted?.(response.data!);
return response;
}).catch((error) => {
if (mutationId === ref.current.mutationId) {
const result = {
loading: false,
error,
data: void 0,
called: true,
client,
};

if (
ref.current.isMounted &&
!equal(ref.current.result, result)
) {
ref.current.result = result;
setResult(result);
}
}

if (options?.onError) {
options.onError(error);
if (ref.current.baseOptions?.onError) {
ref.current.baseOptions?.onError(error);
// TODO(brian): why are we returning this here???
return { data: void 0, errors: error };
}

throw error;
});
}, [client, options, mutation]);

execution
.finally(() => new Promise((resolve) => setTimeout(resolve)))
.then(
(response) => {
const { data, errors } = response;
const error =
errors && errors.length > 0
? new ApolloError({ graphQLErrors: errors })
: void 0;

if (
mutationId === ref.current.mutationId &&
!ref.current.baseOptions?.ignoreResults
) {
const result = {
called: true,
loading: false,
data,
error,
client,
};

if (ref.current.isMounted && !equal(ref.current.result, result)) {
setResult(ref.current.result = result);
}
}
},
(error) => {
if (mutationId === ref.current.mutationId) {
const result = {
loading: false,
error,
data: void 0,
called: true,
client,
};

if (ref.current.isMounted && !equal(ref.current.result, result)) {
setResult(ref.current.result = result);
}
}
},
);

return execution;
}, [client]);

useEffect(() => {
ref.current.baseOptions = { ...options, mutation };
}, [options, mutation]);

useEffect(() => () => {
ref.current.isMounted = false;
Expand Down

0 comments on commit cf90a3d

Please sign in to comment.