diff --git a/packages/ra-core/src/dataProvider/useMutationWithMutationMode.spec.tsx b/packages/ra-core/src/dataProvider/useMutationWithMutationMode.spec.tsx index aaf07148713..f4a4b20b08c 100644 --- a/packages/ra-core/src/dataProvider/useMutationWithMutationMode.spec.tsx +++ b/packages/ra-core/src/dataProvider/useMutationWithMutationMode.spec.tsx @@ -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'; @@ -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(); return useMutationWithMutationMode< Error, @@ -33,6 +46,7 @@ describe('useMutationWithMutationMode', () => { getSnapshot: () => { return []; }, + ...options, }); }; @@ -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 ( + <> +

{data.value}

+ + + + ); + }; + + render( + + + + ); + 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' }, + }); + }); + }); }); diff --git a/packages/ra-core/src/dataProvider/useMutationWithMutationMode.ts b/packages/ra-core/src/dataProvider/useMutationWithMutationMode.ts index 663dced03f5..416c58d0ccd 100644 --- a/packages/ra-core/src/dataProvider/useMutationWithMutationMode.ts +++ b/packages/ra-core/src/dataProvider/useMutationWithMutationMode.ts @@ -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>(params); - useEffect(() => { - paramsRef.current = params; - }, [params]); // Ref that stores the snapshot of the state before the mutation to allow reverting it const snapshot = useRef([]); @@ -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' @@ -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) ); @@ -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); } ); @@ -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; } @@ -288,7 +285,7 @@ export const useMutationWithMutationMode = < if (onSuccess) { onSuccess( optimisticResult, - callTimeParams, + { ...paramsRef.current, ...callTimeParams }, { snapshot: snapshot.current, }, @@ -304,7 +301,7 @@ export const useMutationWithMutationMode = < ) { mutationOptions.onSuccess( optimisticResult, - callTimeParams, + { ...paramsRef.current, ...callTimeParams }, { snapshot: snapshot.current, }, @@ -319,16 +316,25 @@ 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]) => { @@ -336,7 +342,10 @@ export const useMutationWithMutationMode = < }); } else { // call the mutate method without success side effects - mutation.mutate(callTimeParams); + mutation.mutate({ + ...paramsRef.current, + ...callTimeParams, + }); } }); }