Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Form should only trigger field validation on submit if not told otherwise #8826

Merged
merged 3 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is weird as we don't use useFormState anywhere before or did I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use the formState (formState: { isSubmitted }) so I figured I'd rather add the Reminder here anyway. But I'm open to discussion 😁


```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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same


```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>
);
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

**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
};
};