Skip to content

Commit

Permalink
Merge pull request #8826 from marmelab/fix-form-validate-on-submit
Browse files Browse the repository at this point in the history
Fix Form should only trigger field validation on submit if not told otherwise
  • Loading branch information
djhi authored Apr 13, 2023
2 parents 318cc33 + 09707bc commit 60155fe
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 40 deletions.
7 changes: 7 additions & 0 deletions docs/Form.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ Here are all the props you can set on the `<Form>` 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:
Expand Down
7 changes: 7 additions & 0 deletions docs/Inputs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions docs/SimpleForm.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ Here are all the props you can set on the `<SimpleForm>` 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`

`<SimpleForm>` renders a MUI `<CardContent>` by default. You replace it by any component you want as wrapper, just pass it as the `component` prop.
Expand Down
7 changes: 7 additions & 0 deletions docs/TabbedForm.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ Here are all the props you can set on the `<TabbedForm>` 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`

`<TabbedForm>` renders a MUI `<CardContent>` by default. You replace it by any component you want as wrapper, just pass it as the `component` prop.
Expand Down
21 changes: 21 additions & 0 deletions docs/Upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -1966,6 +1973,13 @@ const ReviewEditToolbar = (props: ToolbarProps<Review>) => {
};
```

**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
```

### `<Toolbar>`'s `alwaysEnableSaveButton` Prop Has Been Removed

This prop has been replaced by `<SaveButton>`'s `alwaysEnable` with the same logic.
Expand Down Expand Up @@ -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 `<SimpleForm>` 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 `<Labeled>` to add a label for them.
Expand Down
14 changes: 14 additions & 0 deletions docs/useInput.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<Select>`

```jsx
Expand Down Expand Up @@ -146,3 +153,10 @@ const PersonEdit = () => (
</Edit>
);
```

**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
```
28 changes: 25 additions & 3 deletions packages/ra-core/src/form/Form.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Input {...props} />;
};
Expand All @@ -128,7 +129,7 @@ describe('Form', () => {
fireEvent.click(screen.getByText('Submit'));

await waitFor(() => {
assert.equal(globalFormState.formState.isSubmitting, true);
assert.equal(isSubmitting, true);
});
});

Expand Down Expand Up @@ -639,4 +640,25 @@ describe('Form', () => {
render(<NullValue />);
// 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(
<CoreAdminContext>
<Form onSubmit={jest.fn()}>
<Input source="name" validate={validate} />
<button type="submit">Submit</button>
</Form>
</CoreAdminContext>
);

fireEvent.change(screen.getByLabelText('name'), {
target: { value: 'hello' },
});
expect(validate).not.toHaveBeenCalled();
fireEvent.click(screen.getByText('Submit'));
await waitFor(() => {
expect(validate).toHaveBeenCalled();
});
});
});
21 changes: 11 additions & 10 deletions packages/ra-core/src/form/FormDataConsumer.tsx
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -44,15 +45,15 @@ import get from 'lodash/get';
const FormDataConsumer = <TFieldValues extends FieldValues = FieldValues>(
props: ConnectedProps<TFieldValues>
) => {
const { getValues } = useFormContext<TFieldValues>();
let formData = (useWatch<TFieldValues>() 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<TFieldValues>();
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<TFieldValues>();
return (
<FormDataConsumerView<TFieldValues> formData={formData} {...props} />
);
Expand Down
27 changes: 0 additions & 27 deletions packages/ra-core/src/form/useAugmentedForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
13 changes: 13 additions & 0 deletions packages/ra-core/src/form/useFormValues.ts
Original file line number Diff line number Diff line change
@@ -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<TFieldValues>();

return {
...useWatch(), // subscribe to form value updates
...getValues(), // always merge with latest form values
};
};

0 comments on commit 60155fe

Please sign in to comment.