Refactor middlewares to handle optimistic cases

packages/ra-core/src/controller/create/useCreateController.ts

@@ -79,7 +79,7 @@ export const useCreateController = <
         mutationOptions;
     const {
         registerMutationMiddleware,
-        mutateWithMiddlewares,
+        getMutateWithMiddlewares,
         unregisterMutationMiddleware,
     } = useMutationMiddlewares();
 
@@ -134,7 +134,7 @@ export const useCreateController = <
         },
         ...otherMutationOptions,
         returnPromise: true,
-        mutateWithMiddlewares,
+        getMutateWithMiddlewares,
     });
 
     const save = useCallback(

packages/ra-core/src/controller/edit/useEditController.ts

@@ -91,7 +91,7 @@ export const useEditController = <
     } = mutationOptions;
     const {
         registerMutationMiddleware,
-        mutateWithMiddlewares,
+        getMutateWithMiddlewares,
         unregisterMutationMiddleware,
     } = useMutationMiddlewares();
     const {
@@ -192,7 +192,7 @@ export const useEditController = <
             ...otherMutationOptions,
             mutationMode,
             returnPromise: mutationMode === 'pessimistic',
-            mutateWithMiddlewares,
+            getMutateWithMiddlewares,
         }
     );
 

packages/ra-core/src/controller/saveContext/useMutationMiddlewares.ts

@@ -51,12 +51,11 @@ export const useMutationMiddlewares = <
         []
     );
 
-    const mutateWithMiddlewares = useCallback(
-        (
-            fn: MutateFunc,
-            ...args: Parameters<MutateFunc>
-        ): ReturnType<MutateFunc> => {
-            let index = callbacks.current.length - 1;
+    const getMutateWithMiddlewares = useCallback((fn: MutateFunc) => {
+        // Stores the current callbacks in a closure to avoid losing them if the calling component is unmounted
+        const currentCallbacks = [...callbacks.current];
+        return (...args: Parameters<MutateFunc>): ReturnType<MutateFunc> => {
+            let index = currentCallbacks.length - 1;
 
             // Called by middlewares to call the next middleware function
             // Should take the same arguments as the original mutation function
@@ -67,32 +66,31 @@ export const useMutationMiddlewares = <
 
                 // If there are no more middlewares, we call the original mutation function
                 if (index >= 0) {
-                    return callbacks.current[index](...newArgs, next);
+                    return currentCallbacks[index](...newArgs, next);
                 } else {
                     return fn(...newArgs);
                 }
             };
 
-            if (callbacks.current.length > 0) {
+            if (currentCallbacks.length > 0) {
                 // Call the first middleware with the same args as the original mutation function
                 // with an additional next function
-                return callbacks.current[index](...args, next);
+                return currentCallbacks[index](...args, next);
             }
 
             return fn(...args);
-        },
-        []
-    );
+        };
+    }, []);
 
     const functions = useMemo<UseMutationMiddlewaresResult<MutateFunc>>(
         () => ({
             registerMutationMiddleware,
-            mutateWithMiddlewares,
+            getMutateWithMiddlewares,
             unregisterMutationMiddleware,
         }),
         [
             registerMutationMiddleware,
-            mutateWithMiddlewares,
+            getMutateWithMiddlewares,
             unregisterMutationMiddleware,
         ]
     );
@@ -104,10 +102,9 @@ export interface UseMutationMiddlewaresResult<
     MutateFunc extends (...args: any[]) => any = (...args: any[]) => any,
 > {
     registerMutationMiddleware: (callback: Middleware<MutateFunc>) => void;
-    mutateWithMiddlewares: (
-        mutate: MutateFunc,
-        ...args: Parameters<MutateFunc>
-    ) => ReturnType<MutateFunc>;
+    getMutateWithMiddlewares: (
+        mutate: MutateFunc
+    ) => (...args: Parameters<MutateFunc>) => ReturnType<MutateFunc>;
     unregisterMutationMiddleware: (callback: Middleware<MutateFunc>) => void;
 }
 

packages/ra-core/src/dataProvider/useCreate.spec.tsx

@@ -7,6 +7,10 @@ import { testDataProvider } from './testDataProvider';
 import { useCreate } from './useCreate';
 import { useGetList } from './useGetList';
 import { CoreAdminContext } from '../core';
+import {
+    WithMiddlewaresError,
+    WithMiddlewaresSuccess,
+} from './useCreate.stories';
 
 describe('useCreate', () => {
     it('returns a callback that can be used with create arguments', async () => {
@@ -181,4 +185,49 @@ describe('useCreate', () => {
             expect(screen.queryByText('ghi')).not.toBeNull();
         });
     });
+
+    describe('middlewares', () => {
+        it('it accepts middlewares and displays result and success side effects when dataProvider promise resolves', async () => {
+            render(<WithMiddlewaresSuccess timeout={10} />);
+            screen.getByText('Create post').click();
+            await waitFor(() => {
+                expect(screen.queryByText('success')).toBeNull();
+                expect(
+                    screen.queryByText('Hello World from middleware')
+                ).toBeNull();
+                expect(screen.queryByText('mutating')).not.toBeNull();
+            });
+            await waitFor(() => {
+                expect(screen.queryByText('success')).not.toBeNull();
+                expect(
+                    screen.queryByText('Hello World from middleware')
+                ).not.toBeNull();
+                expect(screen.queryByText('mutating')).toBeNull();
+            });
+        });
+
+        it('it accepts middlewares and displays error and error side effects when dataProvider promise rejects', async () => {
+            jest.spyOn(console, 'error').mockImplementation(() => {});
+            render(<WithMiddlewaresError timeout={10} />);
+            screen.getByText('Create post').click();
+            await waitFor(() => {
+                expect(screen.queryByText('success')).toBeNull();
+                expect(screen.queryByText('something went wrong')).toBeNull();
+                expect(
+                    screen.queryByText('Hello World from middleware')
+                ).toBeNull();
+                expect(screen.queryByText('mutating')).not.toBeNull();
+            });
+            await waitFor(() => {
+                expect(screen.queryByText('success')).toBeNull();
+                expect(
+                    screen.queryByText('something went wrong')
+                ).not.toBeNull();
+                expect(
+                    screen.queryByText('Hello World from middleware')
+                ).toBeNull();
+                expect(screen.queryByText('mutating')).toBeNull();
+            });
+        });
+    });
 });