Skip to content
Open
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
66 changes: 66 additions & 0 deletions packages/ra-core/src/form/groups/useFormGroup.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
TextInput,
} from 'ra-ui-materialui';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { useFormContext } from 'react-hook-form';
import expect from 'expect';
import { FormGroupContextProvider } from './FormGroupContextProvider';
import { testDataProvider } from '../../dataProvider';
Expand Down Expand Up @@ -281,4 +282,69 @@ describe('useFormGroup', () => {
});
});
});

// https://github.com/marmelab/react-admin/issues/11290
it('should not re-render when a field validating state changes', async () => {
// useFormGroup must not subscribe to react-hook-form's validatingFields.
// Subscribing re-renders every form group on each per-field validation
// toggle, which makes large TabbedForms exceed React's maximum update
// depth on submit.
const asyncValidate = () => Promise.resolve(undefined);

let renderCount = 0;
const GroupRenderCounter = React.memo(() => {
useFormGroup('group');
renderCount++;
return null;
});
GroupRenderCounter.displayName = 'GroupRenderCounter';

// Re-validates the field without changing its value/dirty/touched state,
// so the only form state that changes is validatingFields.
const RevalidateButton = () => {
const { trigger } = useFormContext();
return (
<button type="button" onClick={() => trigger('title')}>
revalidate
</button>
);
};

render(
<AdminContext dataProvider={testDataProvider()}>
<ResourceContextProvider value="posts">
<SimpleForm mode="onChange" toolbar={false}>
<FormGroupContextProvider name="group">
<TextInput
source="title"
validate={asyncValidate}
/>
</FormGroupContextProvider>
<GroupRenderCounter />
<RevalidateButton />
</SimpleForm>
</ResourceContextProvider>
</AdminContext>
);

await waitFor(() => expect(renderCount).toBeGreaterThan(0));
await new Promise(resolve => setTimeout(resolve, 50));
const rendersBeforeRevalidation = renderCount;

// Re-validate the field many times. Each re-validation toggles
// validatingFields. When the group subscribes to validatingFields (the
// bug), every toggle re-renders the group, so the count grows with each
// re-validation; without that subscription it stays flat.
const REVALIDATIONS = 10;
for (let i = 0; i < REVALIDATIONS; i++) {
fireEvent.click(screen.getByText('revalidate'));
await new Promise(resolve => setTimeout(resolve, 10));
}
await new Promise(resolve => setTimeout(resolve, 50));

// The group must not re-render on each per-field validation toggle.
expect(renderCount - rendersBeforeRevalidation).toBeLessThan(
REVALIDATIONS
);
});
});
17 changes: 9 additions & 8 deletions packages/ra-core/src/form/groups/useFormGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,14 @@ type FormGroupState = {
* @returns {FormGroupState} The form group state
*/
export const useFormGroup = (name: string): FormGroupState => {
const { dirtyFields, touchedFields, validatingFields, errors } =
useFormState();
const { dirtyFields, touchedFields, errors } = useFormState();

// dirtyFields, touchedFields, validatingFields and errors are objects with keys being the field names
// dirtyFields, touchedFields 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();
Expand All @@ -97,8 +95,13 @@ export const useFormGroup = (name: string): FormGroupState => {
error: get(errors, field, undefined),
isDirty: get(dirtyFields, field, false) !== false,
isValid: get(errors, field, undefined) == null,
isValidating:
get(validatingFields, field, undefined) == null,
// We intentionally do not derive isValidating from
// react-hook-form's validatingFields. Subscribing to it
// re-renders every form group on each per-field validation
// toggle, which on a large TabbedForm exceeds React's
// maximum update depth on submit. The group-level
// isValidating value is not consumed anywhere.
isValidating: false,
isTouched: get(touchedFields, field, false) !== false,
};
})
Expand All @@ -123,8 +126,6 @@ export const useFormGroup = (name: string): FormGroupState => {
JSON.stringify(errorsNames),
// eslint-disable-next-line react-hooks/exhaustive-deps
JSON.stringify(touchedFieldsNames),
// eslint-disable-next-line react-hooks/exhaustive-deps
JSON.stringify(validatingFieldsNames),
updateGroupState,
name,
formGroups,
Expand Down
23 changes: 23 additions & 0 deletions packages/ra-ui-materialui/src/form/TabbedForm.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import {
RaRecord,
required,
ResourceContextProvider,
testDataProvider,
TestMemoryRouter,
Expand Down Expand Up @@ -184,3 +185,25 @@ export const EncodedPaths = () => (
</TabbedForm>
</Wrapper>
);

// https://github.com/marmelab/react-admin/issues/11290
// Submitting this form (with the fields empty) used to throw
// "Maximum update depth exceeded" because each per-tab FormGroup re-rendered
// on every per-field validation toggle. Click SAVE to check it no longer does.
export const ManyRequiredInputs = () => (
<Wrapper record={{ id: 1 }}>
<TabbedForm>
{Array.from({ length: 6 }, (_, tab) => (
<TabbedForm.Tab key={tab} label={`tab ${tab + 1}`}>
{Array.from({ length: 8 }, (_, field) => (
<TextInput
key={`field_${tab}_${field}`}
source={`field_${tab}_${field}`}
validate={required()}
/>
))}
</TabbedForm.Tab>
))}
</TabbedForm>
</Wrapper>
);
Loading