From cb11f606f6a9530d146b1eda2823434a4c009caf Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Tue, 11 Apr 2023 18:33:54 +0200 Subject: [PATCH 1/3] Fix Form should only trigger field validation on submit if not told otherwise --- packages/ra-core/src/form/Form.spec.tsx | 7 ++--- .../ra-core/src/form/FormDataConsumer.tsx | 21 ++++++++------- packages/ra-core/src/form/useAugmentedForm.ts | 27 ------------------- packages/ra-core/src/form/useFormValues.ts | 13 +++++++++ 4 files changed, 28 insertions(+), 40 deletions(-) create mode 100644 packages/ra-core/src/form/useFormValues.ts diff --git a/packages/ra-core/src/form/Form.spec.tsx b/packages/ra-core/src/form/Form.spec.tsx index 9c63b55dafc..acd31ad939f 100644 --- a/packages/ra-core/src/form/Form.spec.tsx +++ b/packages/ra-core/src/form/Form.spec.tsx @@ -109,10 +109,11 @@ describe('Form', () => { }); it('should update Form state on submit', async () => { - let globalFormState; + let isSubmitting; const CustomInput = props => { - globalFormState = useFormContext(); + const formContext = useFormContext(); + isSubmitting = formContext.formState.isSubmitting; return ; }; @@ -128,7 +129,7 @@ describe('Form', () => { fireEvent.click(screen.getByText('Submit')); await waitFor(() => { - assert.equal(globalFormState.formState.isSubmitting, true); + assert.equal(isSubmitting, true); }); }); diff --git a/packages/ra-core/src/form/FormDataConsumer.tsx b/packages/ra-core/src/form/FormDataConsumer.tsx index 1c1587a22e1..5f84d0ca237 100644 --- a/packages/ra-core/src/form/FormDataConsumer.tsx +++ b/packages/ra-core/src/form/FormDataConsumer.tsx @@ -1,7 +1,8 @@ import * as React from 'react'; import { ReactNode } from 'react'; -import { useWatch, useFormContext, FieldValues } from 'react-hook-form'; +import { useFormContext, FieldValues } from 'react-hook-form'; import get from 'lodash/get'; +import { useFormValues } from './useFormValues'; /** * Get the current (edited) value of the record from the form and pass it @@ -44,15 +45,15 @@ import get from 'lodash/get'; const FormDataConsumer = ( props: ConnectedProps ) => { - const { getValues } = useFormContext(); - let formData = (useWatch() as unknown) as TFieldValues; - - //useWatch will initially return the provided defaultValues of the form. - //We must get the initial formData from getValues - if (Object.keys(formData).length === 0) { - formData = getValues(); - } - + const form = useFormContext(); + const { + formState: { + // Don't know exactly why, but this is needed for the form values to be updated + // eslint-disable-next-line @typescript-eslint/no-unused-vars + isDirty, + }, + } = form; + const formData = useFormValues(); return ( formData={formData} {...props} /> ); diff --git a/packages/ra-core/src/form/useAugmentedForm.ts b/packages/ra-core/src/form/useAugmentedForm.ts index 1f2b6c06112..065cccb3ac2 100644 --- a/packages/ra-core/src/form/useAugmentedForm.ts +++ b/packages/ra-core/src/form/useAugmentedForm.ts @@ -93,33 +93,6 @@ export const useAugmentedForm = (props: UseAugmentedFormProps) => { const formRef = useRef(form); - // According to react-hook-form docs: https://react-hook-form.com/api/useform/formstate - // `formState` must be read before a render in order to enable the state update. - const { - formState: { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - isSubmitting, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - isDirty, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - isValid, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - isValidating, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - dirtyFields, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - errors, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - submitCount, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - touchedFields, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - isSubmitted, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - isSubmitSuccessful, - }, - } = form; - // initialize form with record /* eslint-disable react-hooks/exhaustive-deps */ useEffect(() => { diff --git a/packages/ra-core/src/form/useFormValues.ts b/packages/ra-core/src/form/useFormValues.ts new file mode 100644 index 00000000000..03108f3314d --- /dev/null +++ b/packages/ra-core/src/form/useFormValues.ts @@ -0,0 +1,13 @@ +import { FieldValues, useFormContext, useWatch } from 'react-hook-form'; + +// hook taken from https://react-hook-form.com/api/usewatch/#rules +export const useFormValues = < + TFieldValues extends FieldValues = FieldValues +>() => { + const { getValues } = useFormContext(); + + return { + ...useWatch(), // subscribe to form value updates + ...getValues(), // always merge with latest form values + }; +}; From 347b4d525e099d65f62c54598efaf27c18160d2d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Tue, 11 Apr 2023 18:38:01 +0200 Subject: [PATCH 2/3] add a test --- packages/ra-core/src/form/Form.spec.tsx | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/ra-core/src/form/Form.spec.tsx b/packages/ra-core/src/form/Form.spec.tsx index acd31ad939f..c7eb4798a85 100644 --- a/packages/ra-core/src/form/Form.spec.tsx +++ b/packages/ra-core/src/form/Form.spec.tsx @@ -640,4 +640,25 @@ describe('Form', () => { render(); // no assertion needed: if there is a console error, the test fails }); + + it('should only validate inputs on submit', async () => { + let validate = jest.fn(); + render( + +
+ + +
+ + ); + + fireEvent.change(screen.getByLabelText('name'), { + target: { value: 'hello' }, + }); + expect(validate).not.toHaveBeenCalled(); + fireEvent.click(screen.getByText('Submit')); + await waitFor(() => { + expect(validate).toHaveBeenCalled(); + }); + }); }); From 09707bcb26702f0028033734b53b4282c08152bd Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Wed, 12 Apr 2023 16:14:43 +0200 Subject: [PATCH 3/3] mention the rhf rule in the docs --- docs/Form.md | 7 +++++++ docs/Inputs.md | 7 +++++++ docs/SimpleForm.md | 7 +++++++ docs/TabbedForm.md | 7 +++++++ docs/Upgrade.md | 21 +++++++++++++++++++++ docs/useInput.md | 14 ++++++++++++++ 6 files changed, 63 insertions(+) diff --git a/docs/Form.md b/docs/Form.md index 00662746186..cc028a95da5 100644 --- a/docs/Form.md +++ b/docs/Form.md @@ -53,6 +53,13 @@ Here are all the props you can set on the `
` component: Additional props are passed to [the `useForm` hook](https://react-hook-form.com/api/useform). +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ## `defaultValues` The value of the form `defaultValues` prop is an object, or a function returning an object, specifying default values for the created record. For instance: diff --git a/docs/Inputs.md b/docs/Inputs.md index 20898df62f3..38f36ccf9ae 100644 --- a/docs/Inputs.md +++ b/docs/Inputs.md @@ -891,6 +891,13 @@ const PersonEdit = () => ( ); ``` +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ## Third-Party Components You can find components for react-admin in third-party repositories. diff --git a/docs/SimpleForm.md b/docs/SimpleForm.md index 8bf0b8cb948..059bf264c05 100644 --- a/docs/SimpleForm.md +++ b/docs/SimpleForm.md @@ -46,6 +46,13 @@ Here are all the props you can set on the `` component: Additional props are passed to [the `useForm` hook](https://react-hook-form.com/api/useform). +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ## `component` `` renders a MUI `` by default. You replace it by any component you want as wrapper, just pass it as the `component` prop. diff --git a/docs/TabbedForm.md b/docs/TabbedForm.md index fc0ab6947eb..7a4431feb13 100644 --- a/docs/TabbedForm.md +++ b/docs/TabbedForm.md @@ -85,6 +85,13 @@ Here are all the props you can set on the `` component: Additional props are passed to [the `useForm` hook](https://react-hook-form.com/api/useform). +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ## `component` `` renders a MUI `` by default. You replace it by any component you want as wrapper, just pass it as the `component` prop. diff --git a/docs/Upgrade.md b/docs/Upgrade.md index 96237d8dde6..6001febe5b1 100644 --- a/docs/Upgrade.md +++ b/docs/Upgrade.md @@ -1891,6 +1891,13 @@ const MyCustomForm = () => { } ``` +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ### `sanitizeEmptyValues` Works the Other Way Around React-hook-form doesn't remove empty values like react-final-fom did. Therefore, if you opted out of this behavior with `sanitizeEmptyValues={false}`, you no longer need that prop: @@ -1966,6 +1973,13 @@ const ReviewEditToolbar = (props: ToolbarProps) => { }; ``` +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ### ``'s `alwaysEnableSaveButton` Prop Has Been Removed This prop has been replaced by ``'s `alwaysEnable` with the same logic. @@ -2535,6 +2549,13 @@ const UserForm = () => ( ) ``` +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ### The `addLabel` prop Has Been Removed From All Inputs and Fields Inputs and fields used to support an `addLabel` prop that instructed components such as the `` to decorate the input or the field with a label. This is no longer the case as inputs are now responsible for their label display and you must wrap fields inside a `` to add a label for them. diff --git a/docs/useInput.md b/docs/useInput.md index accb40c188c..146de1570bd 100644 --- a/docs/useInput.md +++ b/docs/useInput.md @@ -100,6 +100,13 @@ const LatLngInput = props => { }; ``` +**Reminder:** [react-hook-form's `formState` is wrapped with a Proxy](https://react-hook-form.com/api/useformstate/#rules) to improve render performance and skip extra computation if specific state is not subscribed. So, make sure you deconstruct or read the `formState` before render in order to enable the subscription. + +```js +const { isDirty } = useFormState(); // ✅ +const formState = useFormState(); // ❌ should deconstruct the formState +``` + ## Usage with MUI `