From 090524ce23809e5435cc9d172c29c3ca776a9aae Mon Sep 17 00:00:00 2001 From: Michel Paiva Date: Tue, 27 Aug 2024 09:24:56 +0200 Subject: [PATCH 1/2] Fix input validation and dirty state after re-enabling disabled fields, by updating react-hook-form and removing workaround --- examples/simple/package.json | 2 +- packages/ra-core/package.json | 4 +- packages/ra-core/src/form/useInput.spec.tsx | 90 ++++++++++++++++++++- packages/ra-core/src/form/useInput.ts | 4 - packages/ra-input-rich-text/package.json | 2 +- packages/ra-ui-materialui/package.json | 2 +- packages/react-admin/package.json | 2 +- yarn.lock | 20 ++--- 8 files changed, 105 insertions(+), 21 deletions(-) diff --git a/examples/simple/package.json b/examples/simple/package.json index 73add2fa5f7..d576b6903cc 100644 --- a/examples/simple/package.json +++ b/examples/simple/package.json @@ -26,7 +26,7 @@ "react": "^18.3.1", "react-admin": "^5.1.2", "react-dom": "^18.3.1", - "react-hook-form": "^7.52.0", + "react-hook-form": "^7.53.0", "react-router": "^6.22.0", "react-router-dom": "^6.22.0" }, diff --git a/packages/ra-core/package.json b/packages/ra-core/package.json index 72556c3dfbc..1dc2b7caa56 100644 --- a/packages/ra-core/package.json +++ b/packages/ra-core/package.json @@ -40,7 +40,7 @@ "jscodeshift": "^0.15.2", "react": "^18.3.1", "react-dom": "^18.3.1", - "react-hook-form": "^7.52.0", + "react-hook-form": "^7.53.0", "react-router": "^6.25.1", "react-router-dom": "^6.25.1", "react-test-renderer": "^18.2.0", @@ -53,7 +53,7 @@ "peerDependencies": { "react": "^18.0.0 || ^19.0.0", "react-dom": "^18.0.0 || ^19.0.0", - "react-hook-form": "^7.52.0", + "react-hook-form": "^7.53.0", "react-router": "^6.22.0", "react-router-dom": "^6.22.0" }, diff --git a/packages/ra-core/src/form/useInput.spec.tsx b/packages/ra-core/src/form/useInput.spec.tsx index 8fef8d4a666..78ea8262969 100644 --- a/packages/ra-core/src/form/useInput.spec.tsx +++ b/packages/ra-core/src/form/useInput.spec.tsx @@ -5,7 +5,7 @@ import { useFormContext, useWatch } from 'react-hook-form'; import { CoreAdminContext, SourceContextProvider } from '../core'; import { testDataProvider } from '../dataProvider'; import { Form } from './Form'; -import { useInput, InputProps } from './useInput'; +import { useInput, InputProps, UseInputValue } from './useInput'; import { required } from './validate'; const Input: FunctionComponent< @@ -566,5 +566,93 @@ describe('useInput', () => { ); }); }); + + it('should validate and be dirty for inputs that were disabled and re-enabled', async () => { + let inputProps: UseInputValue | undefined; + + const DisabledEnableInput = () => { + const [disabled, setDisabled] = React.useState(false); + + return ( + <> + + + {props => { + inputProps = props; // Capture the latest props + return ( + + ); + }} + + + ); + }; + + render( + +
+ + +
+ ); + + // Initial state assertions + expect(inputProps?.fieldState.isDirty).toBe(false); + expect(inputProps?.field.disabled).toBe(false); + + // Disable the input + fireEvent.click(screen.getByText('Toggle')); + + await waitFor(() => { + expect(inputProps?.fieldState.isDirty).toBe(false); + expect(inputProps?.field.disabled).toBe(true); + }); + + // Re-enable the input + fireEvent.click(screen.getByText('Toggle')); + + await waitFor(() => { + expect(inputProps?.fieldState.isDirty).toBe(false); + expect(inputProps?.field.disabled).toBe(false); + }); + + // Type in the input + fireEvent.change(screen.getByLabelText('Title'), { + target: { value: 'A title' }, + }); + + // Assert that the field is now dirty + await waitFor(() => { + expect(inputProps?.fieldState.isDirty).toBe(true); // Now the input should be dirty + expect(inputProps?.field.value).toBe('A title'); + }); + + // Clear the input + fireEvent.change(screen.getByLabelText('Title'), { + target: { value: '' }, + }); + + // Assert that the field is now dirty and invalid because it is required + await waitFor(() => { + expect(inputProps?.fieldState.isDirty).toBe(true); // Now the input should be dirty + expect(inputProps?.field.value).toBe(''); + expect(inputProps?.fieldState.invalid).toBe(true); + }); + }); }); }); diff --git a/packages/ra-core/src/form/useInput.ts b/packages/ra-core/src/form/useInput.ts index 3f1de19fdb3..30cef57a6c0 100644 --- a/packages/ra-core/src/form/useInput.ts +++ b/packages/ra-core/src/form/useInput.ts @@ -104,10 +104,6 @@ export const useInput = ( }, }, ...options, - // Workaround for https://github.com/react-hook-form/react-hook-form/issues/10907 - // FIXME - remove when fixed - // @ts-ignore - only exists since react-hook-form 7.46.0 - disabled: options.disabled || undefined, }); // Because our forms may receive an asynchronously loaded record for instance, diff --git a/packages/ra-input-rich-text/package.json b/packages/ra-input-rich-text/package.json index 48952004903..d14ff5c27d8 100644 --- a/packages/ra-input-rich-text/package.json +++ b/packages/ra-input-rich-text/package.json @@ -57,7 +57,7 @@ "ra-ui-materialui": "^5.1.2", "react": "^18.3.1", "react-dom": "^18.3.1", - "react-hook-form": "^7.52.0", + "react-hook-form": "^7.53.0", "rimraf": "^3.0.2", "tippy.js": "^6.3.7", "typescript": "^5.1.3" diff --git a/packages/ra-ui-materialui/package.json b/packages/ra-ui-materialui/package.json index 197e3895767..30bf06d5755 100644 --- a/packages/ra-ui-materialui/package.json +++ b/packages/ra-ui-materialui/package.json @@ -43,7 +43,7 @@ "ra-language-english": "^5.1.2", "react": "^18.3.1", "react-dom": "^18.3.1", - "react-hook-form": "^7.52.0", + "react-hook-form": "^7.53.0", "react-is": "^18.2.0", "react-router": "^6.25.1", "react-router-dom": "^6.25.1", diff --git a/packages/react-admin/package.json b/packages/react-admin/package.json index 378158c82e7..c8240da1a5f 100644 --- a/packages/react-admin/package.json +++ b/packages/react-admin/package.json @@ -44,7 +44,7 @@ "ra-i18n-polyglot": "^5.1.2", "ra-language-english": "^5.1.2", "ra-ui-materialui": "^5.1.2", - "react-hook-form": "^7.52.0", + "react-hook-form": "^7.53.0", "react-router": "^6.22.0", "react-router-dom": "^6.22.0" }, diff --git a/yarn.lock b/yarn.lock index 9237d82883a..506ceb00f45 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17716,7 +17716,7 @@ __metadata: react: "npm:^18.3.1" react-dom: "npm:^18.3.1" react-error-boundary: "npm:^4.0.13" - react-hook-form: "npm:^7.52.0" + react-hook-form: "npm:^7.53.0" react-is: "npm:^18.2.0" react-router: "npm:^6.25.1" react-router-dom: "npm:^6.25.1" @@ -17729,7 +17729,7 @@ __metadata: peerDependencies: react: ^18.0.0 || ^19.0.0 react-dom: ^18.0.0 || ^19.0.0 - react-hook-form: ^7.52.0 + react-hook-form: ^7.53.0 react-router: ^6.22.0 react-router-dom: ^6.22.0 languageName: unknown @@ -17896,7 +17896,7 @@ __metadata: ra-ui-materialui: "npm:^5.1.2" react: "npm:^18.3.1" react-dom: "npm:^18.3.1" - react-hook-form: "npm:^7.52.0" + react-hook-form: "npm:^7.53.0" rimraf: "npm:^3.0.2" tippy.js: "npm:^6.3.7" typescript: "npm:^5.1.3" @@ -17993,7 +17993,7 @@ __metadata: react-dom: "npm:^18.3.1" react-dropzone: "npm:^14.2.3" react-error-boundary: "npm:^4.0.13" - react-hook-form: "npm:^7.52.0" + react-hook-form: "npm:^7.53.0" react-is: "npm:^18.2.0" react-router: "npm:^6.25.1" react-router-dom: "npm:^6.25.1" @@ -18182,7 +18182,7 @@ __metadata: ra-i18n-polyglot: "npm:^5.1.2" ra-language-english: "npm:^5.1.2" ra-ui-materialui: "npm:^5.1.2" - react-hook-form: "npm:^7.52.0" + react-hook-form: "npm:^7.53.0" react-router: "npm:^6.22.0" react-router-dom: "npm:^6.22.0" rimraf: "npm:^3.0.2" @@ -18291,12 +18291,12 @@ __metadata: languageName: node linkType: hard -"react-hook-form@npm:^7.52.0": - version: 7.52.0 - resolution: "react-hook-form@npm:7.52.0" +"react-hook-form@npm:^7.53.0": + version: 7.53.0 + resolution: "react-hook-form@npm:7.53.0" peerDependencies: react: ^16.8.0 || ^17 || ^18 || ^19 - checksum: 058bf5596f314c071863bb133979deb56d0a7817d5bf1908a569c003fe03a15736402b040d3e18aeb259723c6e15c243fe75d2d887ea47ff4be87fc472f31ad5 + checksum: 6d62b150618a833c17d59e669b707661499e2bb516a8d340ca37699f99eb448bbba7b5b78324938c8948014e21efaa32e3510c2ba246fd5e97a96fca0cfa7c98 languageName: node linkType: hard @@ -19672,7 +19672,7 @@ __metadata: react: "npm:^18.3.1" react-admin: "npm:^5.1.2" react-dom: "npm:^18.3.1" - react-hook-form: "npm:^7.52.0" + react-hook-form: "npm:^7.53.0" react-router: "npm:^6.22.0" react-router-dom: "npm:^6.22.0" typescript: "npm:^5.1.3" From 7745d5201a71ebe423f259bea0db46760f6cdfe8 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Wed, 28 Aug 2024 11:26:52 +0200 Subject: [PATCH 2/2] Fix `useFormGroup` doesn't return validation errors with react-hook-form 7.53.0 (cherry picked from commit 303fdc3334de4616d8bdcbc69e37e525495b6d5b) --- packages/ra-core/src/form/useFormGroup.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/ra-core/src/form/useFormGroup.ts b/packages/ra-core/src/form/useFormGroup.ts index 775f0f99fc2..5aaa915f33d 100644 --- a/packages/ra-core/src/form/useFormGroup.ts +++ b/packages/ra-core/src/form/useFormGroup.ts @@ -69,13 +69,14 @@ export const useFormGroup = (name: string): FormGroupState => { const { dirtyFields, touchedFields, validatingFields, errors } = useFormState(); - // dirtyFields, touchedFields and validatingFields are objects with keys being the field names + // dirtyFields, touchedFields, validatingFields and errors 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 errorsNames = Object.keys(errors); const formGroups = useFormGroups(); const [state, setState] = useState({ @@ -121,7 +122,8 @@ export const useFormGroup = (name: string): FormGroupState => { [ // eslint-disable-next-line react-hooks/exhaustive-deps JSON.stringify(dirtyFieldsNames), - errors, + // eslint-disable-next-line react-hooks/exhaustive-deps + JSON.stringify(errorsNames), // eslint-disable-next-line react-hooks/exhaustive-deps JSON.stringify(touchedFieldsNames), // eslint-disable-next-line react-hooks/exhaustive-deps