Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import * as React from 'react';
import { render, waitFor } from '@testing-library/react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import expect from 'expect';
import { useMutationWithMutationMode } from './useMutationWithMutationMode';
import {
useMutationWithMutationMode,
UseMutationWithMutationModeOptions,
} from './useMutationWithMutationMode';
import { CoreAdminContext } from '../core/CoreAdminContext';
import { useDataProvider } from './useDataProvider';
import { DataProvider } from '../types';
Expand All @@ -12,7 +15,17 @@ describe('useMutationWithMutationMode', () => {
updateUserProfile: ({ data }: { data: any }) => Promise<{ data: any }>;
};

const useUpdateUserProfile = (args?: { data?: any }) => {
const useUpdateUserProfile = (
args?: { data?: any },
options?: Pick<
UseMutationWithMutationModeOptions<
Error,
{ data: any },
{ data?: any }
>,
'mutationMode'
>
) => {
const dataProvider = useDataProvider<MyDataProvider>();
return useMutationWithMutationMode<
Error,
Expand All @@ -33,6 +46,7 @@ describe('useMutationWithMutationMode', () => {
getSnapshot: () => {
return [];
},
...options,
});
};

Expand Down Expand Up @@ -91,4 +105,53 @@ describe('useMutationWithMutationMode', () => {
});
});
});

it('uses the latest params at execution time in optimistic mode', async () => {
const dataProvider = testDataProvider({
updateUserProfile: jest.fn(({ data }) =>
Promise.resolve({ data: { id: 1, ...data } } as any)
),
}) as MyDataProvider;
const Dummy = () => {
const [data, setData] = React.useState({ value: 'value1' });
const [update] = useUpdateUserProfile(
{
data,
},
{ mutationMode: 'optimistic' }
);
return (
<>
<p>{data.value}</p>
<button onClick={() => setData({ value: 'value2' })}>
Update data
</button>
<button onClick={() => update()}>Update</button>
</>
);
};

render(
<CoreAdminContext dataProvider={dataProvider}>
<Dummy />
</CoreAdminContext>
);
fireEvent.click(screen.getByText('Update'));
// In case of undoable, clicking the _Update data_ button would trigger the mutation
fireEvent.click(screen.getByText('Update data'));
await screen.findByText('value2');
fireEvent.click(screen.getByText('Update'));
await waitFor(() => {
expect(dataProvider.updateUserProfile).toHaveBeenCalledWith({
data: { value: 'value1' },
});
});

// Ensure the next call uses the latest data
await waitFor(() => {
expect(dataProvider.updateUserProfile).toHaveBeenCalledWith({
data: { value: 'value2' },
});
});
});
});
47 changes: 28 additions & 19 deletions packages/ra-core/src/dataProvider/useMutationWithMutationMode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ export const useMutationWithMutationMode = <
mode.current = mutationMode;
}, [mutationMode]);

// This ref won't be updated when params change in an effect, only when the mutate callback is called (See L247)
// This ensures that for undoable and optimistic mutations, the params are not changed by side effects (unselectAll for instance)
// _after_ the mutate function has been called, while keeping the ability to change declaration time params _until_ the mutation is called.
const paramsRef = useRef<Partial<TVariables>>(params);
useEffect(() => {
paramsRef.current = params;
}, [params]);

// Ref that stores the snapshot of the state before the mutation to allow reverting it
const snapshot = useRef<Snapshot>([]);
Expand Down Expand Up @@ -96,7 +96,6 @@ export const useMutationWithMutationMode = <
{
mutationKey,
mutationFn: async params => {
const callTimeParams = { ...paramsRef.current, ...params };
if (params == null) {
throw new Error(
'useMutationWithMutationMode mutation requires parameters'
Expand All @@ -105,7 +104,7 @@ export const useMutationWithMutationMode = <

return (
mutateWithMiddlewares
.current(callTimeParams as TVariables)
.current(params as TVariables)
// Middlewares expect the data property of the dataProvider response
.then(({ data }) => data)
);
Expand Down Expand Up @@ -210,12 +209,15 @@ export const useMutationWithMutationMode = <
...otherCallTimeOptions
} = callTimeOptions;

// store the hook time params *at the moment of the call*
// because they may change afterwards, which would break the undoable mode
// as the previousData would be overwritten by the optimistic update
paramsRef.current = params;

// Store the mutation with middlewares to avoid losing them if the calling component is unmounted
if (getMutateWithMiddlewares) {
mutateWithMiddlewares.current = getMutateWithMiddlewaresEvent(
(params: TVariables) => {
// Store the final parameters which might have been changed by middlewares
paramsRef.current = params;
return mutationFnEvent(params);
}
);
Expand All @@ -230,11 +232,6 @@ export const useMutationWithMutationMode = <
callTimeOnError.current = onError;
callTimeOnSettled.current = onSettled;

// store the hook time params *at the moment of the call*
// because they may change afterwards, which would break the undoable mode
// as the previousData would be overwritten by the optimistic update
paramsRef.current = params;

if (mutationMode) {
mode.current = mutationMode;
}
Expand Down Expand Up @@ -288,7 +285,7 @@ export const useMutationWithMutationMode = <
if (onSuccess) {
onSuccess(
optimisticResult,
callTimeParams,
{ ...paramsRef.current, ...callTimeParams },
{
snapshot: snapshot.current,
},
Expand All @@ -304,7 +301,7 @@ export const useMutationWithMutationMode = <
) {
mutationOptions.onSuccess(
optimisticResult,
callTimeParams,
{ ...paramsRef.current, ...callTimeParams },
{
snapshot: snapshot.current,
},
Expand All @@ -319,24 +316,36 @@ export const useMutationWithMutationMode = <

if (mode.current === 'optimistic') {
// call the mutate method without success side effects
return mutation.mutate(callTimeParams);
return mutation.mutate({
...paramsRef.current,
...callTimeParams,
});
} else {
// Undoable mutation: add the mutation to the undoable queue.
// The Notification component will dequeue it when the user confirms or cancels the message.
addUndoableMutation(({ isUndo }) => {
if (isUndo) {
if (onUndo) {
onUndoEvent(callTimeParams, {
mutationMode: mode.current,
});
onUndoEvent(
{
...paramsRef.current,
...callTimeParams,
},
{
mutationMode: mode.current,
}
);
}
// rollback
snapshot.current.forEach(([key, value]) => {
queryClient.setQueryData(key, value);
});
} else {
// call the mutate method without success side effects
mutation.mutate(callTimeParams);
mutation.mutate({
...paramsRef.current,
...callTimeParams,
});
}
});
}
Expand Down
Loading