Merge pull request #9939 from marmelab/fix-form-groups

Fix useFormGroup does not reflect its fields state
marmelab · Jun 20, 2024 · d642511 · d642511
2 parents f9e3ebe + 988f588
commit d642511
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 26 deletions.
diff --git a/packages/ra-core/src/form/useFormGroup.spec.tsx b/packages/ra-core/src/form/useFormGroup.spec.tsx
@@ -22,12 +22,14 @@ describe('useFormGroup', () => {
                     isValid: true,
                     isDirty: false,
                     isTouched: false,
+                    isValidating: false,
                     name: 'title',
                 },
                 {
                     isValid: false,
                     isDirty: true,
                     isTouched: true,
+                    isValidating: false,
                     error: 'Invalid',
                     name: 'description',
                 },
@@ -36,6 +38,7 @@ describe('useFormGroup', () => {
                 isValid: false,
                 isDirty: true,
                 isTouched: true,
+                isValidating: false,
                 errors: {
                     description: 'Invalid',
                 },
@@ -48,19 +51,22 @@ describe('useFormGroup', () => {
                     isValid: true,
                     isDirty: false,
                     isTouched: false,
+                    isValidating: false,
                     name: 'title',
                 },
                 {
                     isValid: true,
                     isDirty: false,
                     isTouched: false,
+                    isValidating: false,
                     name: 'description',
                 },
             ],
             {
                 isValid: true,
                 isDirty: false,
                 isTouched: false,
+                isValidating: false,
                 errors: {},
             },
         ],
@@ -71,19 +77,22 @@ describe('useFormGroup', () => {
                     isValid: true,
                     isDirty: false,
                     isTouched: false,
+                    isValidating: false,
                     name: 'title',
                 },
                 {
                     isValid: true,
                     isDirty: true,
                     isTouched: true,
+                    isValidating: false,
                     name: 'description',
                 },
             ],
             {
                 isValid: true,
                 isDirty: true,
                 isTouched: true,
+                isValidating: false,
                 errors: {},
             },
         ],
@@ -103,7 +112,7 @@ describe('useFormGroup', () => {
 
         render(
             <AdminContext dataProvider={testDataProvider()}>
-                <SimpleForm>
+                <SimpleForm mode="onChange">
                     <FormGroupContextProvider name="simplegroup">
                         <IsDirty />
                         <TextInput source="url" />
@@ -118,20 +127,32 @@ describe('useFormGroup', () => {
                 isDirty: false,
                 isTouched: false,
                 isValid: true,
+                isValidating: false,
             });
         });
 
         const input = screen.getByLabelText('resources.undefined.fields.url');
         fireEvent.change(input, {
             target: { value: 'test' },
         });
+        await waitFor(() => {
+            expect(state).toEqual({
+                errors: {},
+                isDirty: true,
+                isTouched: false,
+                isValid: true,
+                isValidating: false,
+            });
+        });
+        // This is coherent with how react-hook-form works, inputs are only touched when they lose focus
         fireEvent.blur(input);
         await waitFor(() => {
             expect(state).toEqual({
                 errors: {},
                 isDirty: true,
                 isTouched: true,
                 isValid: true,
+                isValidating: false,
             });
         });
     });
@@ -176,6 +197,7 @@ describe('useFormGroup', () => {
                 isDirty: false,
                 isTouched: false,
                 isValid: true,
+                isValidating: false,
             });
         });
 
@@ -190,6 +212,7 @@ describe('useFormGroup', () => {
                 isDirty: true,
                 isTouched: false,
                 isValid: true,
+                isValidating: false,
             });
         });
     });

diff --git a/packages/ra-core/src/form/useFormGroup.ts b/packages/ra-core/src/form/useFormGroup.ts
@@ -1,22 +1,25 @@
-import { useCallback, useEffect, useState } from 'react';
+import { useEffect, useState } from 'react';
 import get from 'lodash/get';
 import isEqual from 'lodash/isEqual';
 import { useFormState } from 'react-hook-form';
 import { useFormGroups } from './useFormGroups';
+import { useEvent } from '../util';
 
 type FieldState = {
     name: string;
     error?: any;
     isDirty: boolean;
     isTouched: boolean;
     isValid: boolean;
+    isValidating: boolean;
 };
 
 type FormGroupState = {
     errors?: object;
     isDirty: boolean;
     isTouched: boolean;
     isValid: boolean;
+    isValidating: boolean;
 };
 
 /**
@@ -63,16 +66,27 @@ type FormGroupState = {
  * @returns {FormGroupState} The form group state
  */
 export const useFormGroup = (name: string): FormGroupState => {
-    const { dirtyFields, touchedFields, errors } = useFormState();
+    const { dirtyFields, touchedFields, validatingFields, errors } =
+        useFormState();
+
+    // dirtyFields, touchedFields and validatingFields are objects with keys being the field names
+    // Ex: { title: true }
+    // However, they are not correctly serialized when using JSON.stringify
+    // To avoid our effects to not be triggered when they should, we extract the keys and use that as a dependency
+    const dirtyFieldsNames = Object.keys(dirtyFields);
+    const touchedFieldsNames = Object.keys(touchedFields);
+    const validatingFieldsNames = Object.keys(validatingFields);
+
     const formGroups = useFormGroups();
     const [state, setState] = useState<FormGroupState>({
         errors: undefined,
         isDirty: false,
         isTouched: false,
         isValid: true,
+        isValidating: true,
     });
 
-    const updateGroupState = useCallback(() => {
+    const updateGroupState = useEvent(() => {
         if (!formGroups) return;
         const fields = formGroups.getGroupFields(name);
         const fieldStates = fields
@@ -81,7 +95,9 @@ export const useFormGroup = (name: string): FormGroupState => {
                     name: field,
                     error: get(errors, field, undefined),
                     isDirty: get(dirtyFields, field, false) !== false,
-                    isValid: get(errors, field, undefined) == undefined, // eslint-disable-line
+                    isValid: get(errors, field, undefined) == null,
+                    isValidating:
+                        get(validatingFields, field, undefined) == null,
                     isTouched: get(touchedFields, field, false) !== false,
                 };
             })
@@ -95,16 +111,21 @@ export const useFormGroup = (name: string): FormGroupState => {
 
             return oldState;
         });
-    }, [dirtyFields, errors, touchedFields, formGroups, name]);
+    });
 
     useEffect(
         () => {
             updateGroupState();
         },
-        // eslint-disable-next-line
+        // eslint-disable-next-line react-hooks/exhaustive-deps
         [
-            // eslint-disable-next-line
-            JSON.stringify({ dirtyFields, errors, touchedFields }),
+            // eslint-disable-next-line react-hooks/exhaustive-deps
+            JSON.stringify(dirtyFieldsNames),
+            errors,
+            // eslint-disable-next-line react-hooks/exhaustive-deps
+            JSON.stringify(touchedFieldsNames),
+            // eslint-disable-next-line react-hooks/exhaustive-deps
+            JSON.stringify(validatingFieldsNames),
             updateGroupState,
         ]
     );
@@ -144,6 +165,7 @@ export const getFormGroupState = (
                 errors,
                 isTouched: acc.isTouched || fieldState.isTouched,
                 isValid: acc.isValid && fieldState.isValid,
+                isValidating: acc.isValidating && fieldState.isValidating,
             };
 
             return newState;
@@ -153,6 +175,7 @@ export const getFormGroupState = (
             errors: undefined,
             isValid: true,
             isTouched: false,
+            isValidating: false,
         }
     );
 };
diff --git a/packages/ra-ui-materialui/src/input/ResettableTextField.tsx b/packages/ra-ui-materialui/src/input/ResettableTextField.tsx
@@ -30,7 +30,7 @@ export const ResettableTextField = forwardRef(
 
         const translate = useTranslate();
 
-        const { onChange, onFocus, onBlur } = props;
+        const { onChange } = props;
         const handleClickClearButton = useCallback(
             event => {
                 event.preventDefault();
@@ -39,20 +39,6 @@ export const ResettableTextField = forwardRef(
             [onChange]
         );
 
-        const handleFocus = useCallback(
-            event => {
-                onFocus && onFocus(event);
-            },
-            [onFocus]
-        );
-
-        const handleBlur = useCallback(
-            event => {
-                onBlur && onBlur(event);
-            },
-            [onBlur]
-        );
-
         const {
             clearButton,
             clearIcon,
@@ -164,8 +150,6 @@ export const ResettableTextField = forwardRef(
                 margin={margin}
                 className={className}
                 {...rest}
-                onFocus={handleFocus}
-                onBlur={handleBlur}
                 inputRef={ref}
             />
         );