marmelab / marmelab/react-admin

`Maximum update depth exceeded` in `TabbedForm` under React 19 (`useFormGroup` schedules a commit-phase update per validation tick)

オープン
#11,368 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
TypeScript
スター
26.9k
フォーク
5.5k
平均マージ
2日 3時間
マージ済み PR(30日)
19

説明

Versions

  • react-admin / ra-core / ra-ui-materialui: 5.15.3
  • react-hook-form: 7.87.0
  • react/react-dom: 19.2.7

What happened

Submitting a <TabbedForm> with several tabs and enough fields throws:

Uncaught (in promise) Error: Maximum update depth exceeded. This can happen when a
component repeatedly calls setState inside componentWillUpdate or componentDidUpdate.
React limits the number of nested updates to prevent infinite loops.
    at getRootForUpdatedFiber (react-dom-client.development.js:4624:11)
    at enqueueConcurrentHookUpdate (react-dom-client.development.js:4584:14)
    at dispatchSetStateInternal (react-dom-client.development.js:9167:18)
    at dispatchSetState (react-dom-client.development.js:9127:7)
    at Object.callback (index.esm.mjs:208:17)
    at Object.next (index.esm.mjs:2000:23)
    at Object.next (index.esm.mjs:795:39)
    at _updateIsValidating (index.esm.mjs:1481:29)
    at executeBuiltInValidation (index.esm.mjs:1656:25)
    at async index.esm.mjs:2182:13
    at async SaveButton.js:83:13

React DevTools shows the crashing setState belongs to FormTabHeader (owner: FormTab), i.e. it comes from useFormGroup.

This looks like the same class of bug fixed for <ReferenceField> in #11329 ("Fix Maximum update depth exceeded when many ReferenceFields resolve at once"), just for a different consumer:

When a child updates its state during the commit phase (from a layout effect), those commits nest instead of batching, and past 50 React throws. [...] React 18.3.1 only counts a pending SyncLane at the end of a commit while React 19.1.0 also counts Default [...] under React 18 even 200 consumers with a three-deep chain of commit-phase updates give 601 commits and no error.

Root cause

useFormGroup (packages/ra-core/src/form/groups/useFormGroup.ts) derives its return value in a useState + useEffect:

const [state, setState] = useState<FormGroupState>({ ... });
const updateGroupState = useEvent(() => {
    // ... compute newState from useFormState()'s dirtyFields/touchedFields/validatingFields/errors
    setState(oldState => isEqual(oldState, newState) ? oldState : newState);
});
useEffect(() => {
    updateGroupState();
}, [JSON.stringify(dirtyFieldsNames), JSON.stringify(errorsNames), JSON.stringify(touchedFieldsNames), JSON.stringify(validatingFieldsNames), updateGroupState, name, formGroups]);

Because this setState fires from a useEffect that reacts to useFormState()'s snapshot, it lands in a separate commit from the one that already re-rendered FormTabHeader for the new form state — an extra "commit-phase update" per tick.

Separately, useInput (packages/ra-core/src/form/useInput.ts) registers an async validate for every field regardless of whether it has real validators:

rules: {
    validate: async (value, values) => {
        if (!sanitizedValidate) return true;
        // ...
    },
},

So on a full-form submit validation, react-hook-form flips validatingFields for every field, one microtask at a time (this is the same trigger that caused #10068 / #10032, fixed upstream in react-hook-form 7.53 — we're already on 7.87, so that fix doesn't help here). With a TabbedForm that has several tabs (each mounting a useFormGroup instance, since TabbedFormView deliberately mounts all tabs' content at once "to allow validation on tabs not in focus") and many fields, this produces a burst of extra commit-phase updates. React 18 tolerated this (per the reasoning in #11329); React 19 does not, and throws "Maximum update depth exceeded" partway through handleSubmit's validation pass — so the mutation never runs and the form silently fails to save.

Reproduction

A <TabbedForm> with e.g. 5-7 <FormTab>s, several TextInputs per tab (some with validate={required()}, most without), submitted via <SaveButton>. The larger the tab × field count, the more reliably it reproduces; our production form (multi-step hiring-request edit form, 7 tabs) hits it consistently. I can try to put together a minimal reproduction if useful — happy to help.

Patch we're running locally

Two changes, applied via yarn patch against ra-core@5.15.3:

  1. useFormGroup: compute the group state synchronously with useMemo instead of useState + useEffect, so it's folded into the same commit as the useFormState() update that triggered it, instead of scheduling a new one:
export const useFormGroup = (name: string): FormGroupState => {
    const { dirtyFields, touchedFields, validatingFields, errors } = useFormState();
    const formGroups = useFormGroups();

    const [, forceRerenderOnGroupFieldsChange] = useReducer(c => c + 1, 0);
    useEffect(() => {
        if (!formGroups) return;
        return formGroups.subscribe(name, forceRerenderOnGroupFieldsChange);
    }, [formGroups, name]);

    return useMemo(() => {
        if (!formGroups) {
            return { errors: undefined, isDirty: false, isTouched: false, isValid: true, isValidating: true };
        }
        const fields = formGroups.getGroupFields(name);
        const fieldStates = fields.map(field => ({
            name: field,
            error: get(errors, field, undefined),
            isDirty: get(dirtyFields, field, false) !== false,
            isValid: get(errors, field, undefined) == null,
            isValidating: get(validatingFields, field, undefined) == null,
            isTouched: get(touchedFields, field, false) !== false,
        })).filter(fieldState => fieldState != undefined);
        return getFormGroupState(fieldStates);
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [
        name,
        formGroups,
        JSON.stringify(Object.keys(dirtyFields)),
        JSON.stringify(Object.keys(errors)),
        JSON.stringify(Object.keys(touchedFields)),
        JSON.stringify(Object.keys(validatingFields)),
    ]);
};
  1. useInput / ArrayInputBase: only register rules.validate when there is an actual validator, instead of always registering an async no-op — reduces (but on its own does not eliminate) how often validatingFields churns:
rules: sanitizedValidate
    ? {
        validate: async (value, values) => {
            const error = await sanitizedValidate(value, values, { ...props, finalSource });
            if (!error) return true;
            return `@@react-admin@@${JSON.stringify(error)}`;
        },
    }
    : undefined,

We verified (via git stash) that change 1 alone (the useFormGroup rewrite) is sufficient to fix the crash in our app; change 2 is a smaller-impact mitigation we kept anyway. Full patch (applied to the compiled dist output, both ESM and CJS) attached below for reference — happy to open a PR against packages/ra-core/src instead if that's preferred.

Patch (dist, both useFormGroup and useInput/ArrayInputBase, ESM+CJS)
diff --git a/dist/controller/input/ArrayInputBase.cjs b/dist/controller/input/ArrayInputBase.cjs
index a6de40ea7ec4972300cd4d732004ee4bd7d6511a..f4408dbf97bc62e869fc81b900d096be72beec4d 100644
--- a/dist/controller/input/ArrayInputBase.cjs
+++ b/dist/controller/input/ArrayInputBase.cjs
@@ -94,18 +94,19 @@ const ArrayInputBase = (props) => {
         : validate;
     const getValidationErrorMessage = (0, useGetValidationErrorMessage_1.useGetValidationErrorMessage)();
     const { getValues } = (0, react_hook_form_1.useFormContext)();
+    // Local patch: see dist/form/useInput.js for the rationale.
     const fieldProps = (0, react_hook_form_1.useFieldArray)({
         name: finalSource,
-        rules: {
-            validate: async (value) => {
-                if (!sanitizedValidate)
-                    return true;
-                const error = await sanitizedValidate(value, getValues(), props);
-                if (!error)
-                    return true;
-                return getValidationErrorMessage(error);
-            },
-        },
+        rules: sanitizedValidate
+            ? {
+                validate: async (value) => {
+                    const error = await sanitizedValidate(value, getValues(), props);
+                    if (!error)
+                        return true;
+                    return getValidationErrorMessage(error);
+                },
+            }
+            : undefined,
     });
     (0, react_1.useEffect)(() => {
         if (formGroups && formGroupName != null) {
diff --git a/dist/controller/input/ArrayInputBase.js b/dist/controller/input/ArrayInputBase.js
index 47a1ea4fb3c30114a2e4799f9248eebbcefd6f4b..d8eddce3078f08342f7ff8f3f7f9476174fe7945 100644
--- a/dist/controller/input/ArrayInputBase.js
+++ b/dist/controller/input/ArrayInputBase.js
@@ -58,18 +58,20 @@ export const ArrayInputBase = (props) => {
         : validate;
     const getValidationErrorMessage = useGetValidationErrorMessage();
     const { getValues } = useFormContext();
+    // Local patch: see useInput.js for the rationale — only register
+    // an async validate when there is an actual validator.
     const fieldProps = useFieldArray({
         name: finalSource,
-        rules: {
-            validate: async (value) => {
-                if (!sanitizedValidate)
-                    return true;
-                const error = await sanitizedValidate(value, getValues(), props);
-                if (!error)
-                    return true;
-                return getValidationErrorMessage(error);
-            },
-        },
+        rules: sanitizedValidate
+            ? {
+                validate: async (value) => {
+                    const error = await sanitizedValidate(value, getValues(), props);
+                    if (!error)
+                        return true;
+                    return getValidationErrorMessage(error);
+                },
+            }
+            : undefined,
     });
     useEffect(() => {
         if (formGroups && formGroupName != null) {
diff --git a/dist/form/groups/useFormGroup.cjs b/dist/form/groups/useFormGroup.cjs
index c4c0b2f2195c46f6c0cb526178b27d2c8c24188b..4bb3ebd6ea5221ab39b3e575285e62231a6219cc 100644
--- a/dist/form/groups/useFormGroup.cjs
+++ b/dist/form/groups/useFormGroup.cjs
@@ -6,10 +6,8 @@ Object.defineProperty(exports, "__esModule", { value: true });
 exports.getFormGroupState = exports.useFormGroup = void 0;
 const react_1 = require("react");
 const get_js_1 = __importDefault(require("lodash/get.js"));
-const isEqual_js_1 = __importDefault(require("lodash/isEqual.js"));
 const react_hook_form_1 = require("react-hook-form");
 const useFormGroups_1 = require("./useFormGroups.cjs");
-const util_1 = require("../../util/index.cjs");
 /**
  * Retrieve a specific form group data such as its validation status (valid/invalid) or
  * or whether its inputs have been updated (dirty/pristine)
@@ -58,22 +56,31 @@ const useFormGroup = (name) => {
     // 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
+    // To avoid recomputing when they should not, 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 = (0, useFormGroups_1.useFormGroups)();
-    const [state, setState] = (0, react_1.useState)({
-        errors: undefined,
-        isDirty: false,
-        isTouched: false,
-        isValid: true,
-        isValidating: true,
-    });
-    const updateGroupState = (0, util_1.useEvent)(() => {
+    // Local patch: see dist/form/groups/useFormGroup.js for the rationale.
+    const [, forceRerenderOnGroupFieldsChange] = (0, react_1.useReducer)((c) => c + 1, 0);
+    (0, react_1.useEffect)(() => {
         if (!formGroups)
             return;
+        // Whenever the group content changes (input are added or removed)
+        // we must recompute its state
+        return formGroups.subscribe(name, forceRerenderOnGroupFieldsChange);
+    }, [formGroups, name]);
+    return (0, react_1.useMemo)(() => {
+        if (!formGroups) {
+            return {
+                errors: undefined,
+                isDirty: false,
+                isTouched: false,
+                isValid: true,
+                isValidating: true,
+            };
+        }
         const fields = formGroups.getGroupFields(name);
         const fieldStates = fields
             .map(field => {
@@ -87,40 +94,16 @@ const useFormGroup = (name) => {
             };
         })
             .filter(fieldState => fieldState != undefined); // eslint-disable-line
-        const newState = (0, exports.getFormGroupState)(fieldStates);
-        setState(oldState => {
-            if (!(0, isEqual_js_1.default)(oldState, newState)) {
-                return newState;
-            }
-            return oldState;
-        });
-    });
-    (0, react_1.useEffect)(() => {
-        updateGroupState();
-    }, [
+        return (0, exports.getFormGroupState)(fieldStates);
         // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [
+        name,
+        formGroups,
         JSON.stringify(dirtyFieldsNames),
-        // 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
         JSON.stringify(validatingFieldsNames),
-        updateGroupState,
-        name,
-        formGroups,
     ]);
-    (0, react_1.useEffect)(() => {
-        if (!formGroups)
-            return;
-        // Whenever the group content changes (input are added or removed)
-        // we must update its state
-        const unsubscribe = formGroups.subscribe(name, () => {
-            updateGroupState();
-        });
-        return unsubscribe;
-    }, [formGroups, name, updateGroupState]);
-    return state;
 };
 exports.useFormGroup = useFormGroup;
 /**
diff --git a/dist/form/groups/useFormGroup.js b/dist/form/groups/useFormGroup.js
index 38cff4869ec03ff5ea758c3d2a5ee613232c0ff8..ee3a36c89c1b498ccfb3fc687e80195a0367219f 100644
--- a/dist/form/groups/useFormGroup.js
+++ b/dist/form/groups/useFormGroup.js
@@ -1,9 +1,7 @@
-import { useEffect, useState } from 'react';
+import { useEffect, useMemo, useReducer } from 'react';
 import get from 'lodash/get.js';
-import isEqual from 'lodash/isEqual.js';
 import { useFormState } from 'react-hook-form';
 import { useFormGroups } from "./useFormGroups.js";
-import { useEvent } from "../../util/index.js";
 /**
  * Retrieve a specific form group data such as its validation status (valid/invalid) or
  * or whether its inputs have been updated (dirty/pristine)
@@ -52,22 +50,43 @@ export const useFormGroup = (name) => {
     // 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
+    // To avoid recomputing when they should not, 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({
-        errors: undefined,
-        isDirty: false,
-        isTouched: false,
-        isValid: true,
-        isValidating: true,
-    });
-    const updateGroupState = useEvent(() => {
+    // Local patch: the group state used to live in its own useState,
+    // recomputed from a useEffect that re-ran whenever useFormState()'s snapshot
+    // changed. That effect fires (and calls setState) in a *separate* commit from
+    // the one that already re-rendered this component for the new formState, i.e.
+    // an extra commit-phase update per validate/dirty/touch tick. With many fields
+    // resolving their (mostly async, mostly no-op) validation one microtask at a
+    // time during a full-form submit, and several of these hooks mounted at once
+    // (one per TabbedForm tab), those extra commits chain into each other. React 19
+    // counts this nesting far more aggressively than React 18 did, so what used to
+    // be merely wasteful now throws "Maximum update depth exceeded" on forms with
+    // many tabs/fields. Computing the group state synchronously with useMemo folds
+    // it into the SAME commit as the triggering useFormState() update instead of
+    // scheduling a new one.
+    const [, forceRerenderOnGroupFieldsChange] = useReducer(c => c + 1, 0);
+    useEffect(() => {
         if (!formGroups)
             return;
+        // Whenever the group content changes (input are added or removed)
+        // we must recompute its state
+        return formGroups.subscribe(name, forceRerenderOnGroupFieldsChange);
+    }, [formGroups, name]);
+    return useMemo(() => {
+        if (!formGroups) {
+            return {
+                errors: undefined,
+                isDirty: false,
+                isTouched: false,
+                isValid: true,
+                isValidating: true,
+            };
+        }
         const fields = formGroups.getGroupFields(name);
         const fieldStates = fields
             .map(field => {
@@ -81,40 +100,16 @@ export const useFormGroup = (name) => {
             };
         })
             .filter(fieldState => fieldState != undefined); // eslint-disable-line
-        const newState = getFormGroupState(fieldStates);
-        setState(oldState => {
-            if (!isEqual(oldState, newState)) {
-                return newState;
-            }
-            return oldState;
-        });
-    });
-    useEffect(() => {
-        updateGroupState();
-    }, [
+        return getFormGroupState(fieldStates);
         // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [
+        name,
+        formGroups,
         JSON.stringify(dirtyFieldsNames),
-        // 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
         JSON.stringify(validatingFieldsNames),
-        updateGroupState,
-        name,
-        formGroups,
     ]);
-    useEffect(() => {
-        if (!formGroups)
-            return;
-        // Whenever the group content changes (input are added or removed)
-        // we must update its state
-        const unsubscribe = formGroups.subscribe(name, () => {
-            updateGroupState();
-        });
-        return unsubscribe;
-    }, [formGroups, name, updateGroupState]);
-    return state;
 };
 /**
  * Get the state of a form group
diff --git a/dist/form/useInput.cjs b/dist/form/useInput.cjs
index e677a4d381368e795ad630a9df1419bcbb590741..2889690d26d9d931832f10c4bcc9ec8aeb1ed3be 100644
--- a/dist/form/useInput.cjs
+++ b/dist/form/useInput.cjs
@@ -46,28 +46,29 @@ const useInput = (props) => {
     // This ensures dynamically added inputs have their value set correctly (ArrayInput for example).
     // We don't do this for the form level defaultValues so that it works as it should in react-hook-form
     // (i.e. field level defaultValue override form level defaultValues for this field).
+    // Local patch: see dist/form/useInput.js for the rationale.
     const { field: controllerField, fieldState, formState, } = (0, react_hook_form_1.useController)({
         name: finalName,
         defaultValue: (0, get_js_1.default)(record, finalSource, defaultValue),
-        rules: {
-            validate: async (value, values) => {
-                if (!sanitizedValidate)
-                    return true;
-                const error = await sanitizedValidate(value, values, {
-                    ...props,
-                    finalSource,
-                });
-                if (!error)
-                    return true;
-                // react-hook-form expects errors to be plain strings but our validators can return objects
-                // that have message and args.
-                // To avoid double translation for users that validate with a schema instead of our validators
-                // we use a special format for our validators errors.
-                // The ValidationError component will check for this format and extract the message and args
-                // to translate.
-                return `@@react-admin@@${JSON.stringify(error)}`;
-            },
-        },
+        rules: sanitizedValidate
+            ? {
+                validate: async (value, values) => {
+                    const error = await sanitizedValidate(value, values, {
+                        ...props,
+                        finalSource,
+                    });
+                    if (!error)
+                        return true;
+                    // react-hook-form expects errors to be plain strings but our validators can return objects
+                    // that have message and args.
+                    // To avoid double translation for users that validate with a schema instead of our validators
+                    // we use a special format for our validators errors.
+                    // The ValidationError component will check for this format and extract the message and args
+                    // to translate.
+                    return `@@react-admin@@${JSON.stringify(error)}`;
+                },
+            }
+            : undefined,
         ...options,
     });
     // Because our forms may receive an asynchronously loaded record for instance,
diff --git a/dist/form/useInput.js b/dist/form/useInput.js
index fcf726c966fa6aaf24243521326e24cc43e05023..14f5ed40507553fa553ea8c4c5ebb20b69f46f4f 100644
--- a/dist/form/useInput.js
+++ b/dist/form/useInput.js
@@ -40,28 +40,37 @@ export const useInput = (props) => {
     // This ensures dynamically added inputs have their value set correctly (ArrayInput for example).
     // We don't do this for the form level defaultValues so that it works as it should in react-hook-form
     // (i.e. field level defaultValue override form level defaultValues for this field).
+    // Local patch: react-admin used to register an async `validate`
+    // for every field regardless of whether it had real validators. On a form with
+    // many fields, react-hook-form flips `validatingFields` for each of them one
+    // microtask at a time during a full-form submit validation, and under React 19
+    // (which nests commit-phase updates more aggressively than React 18) that flood
+    // trips "Maximum update depth exceeded" in components subscribed to form state
+    // (e.g. TabbedForm's FormTabHeader via useFormGroup). Registering `validate`
+    // only when there is an actual validator removes the async churn for fields
+    // that never needed it.
     const { field: controllerField, fieldState, formState, } = useController({
         name: finalName,
         defaultValue: get(record, finalSource, defaultValue),
-        rules: {
-            validate: async (value, values) => {
-                if (!sanitizedValidate)
-                    return true;
-                const error = await sanitizedValidate(value, values, {
-                    ...props,
-                    finalSource,
-                });
-                if (!error)
-                    return true;
-                // react-hook-form expects errors to be plain strings but our validators can return objects
-                // that have message and args.
-                // To avoid double translation for users that validate with a schema instead of our validators
-                // we use a special format for our validators errors.
-                // The ValidationError component will check for this format and extract the message and args
-                // to translate.
-                return `@@react-admin@@${JSON.stringify(error)}`;
-            },
-        },
+        rules: sanitizedValidate
+            ? {
+                validate: async (value, values) => {
+                    const error = await sanitizedValidate(value, values, {
+                        ...props,
+                        finalSource,
+                    });
+                    if (!error)
+                        return true;
+                    // react-hook-form expects errors to be plain strings but our validators can return objects
+                    // that have message and args.
+                    // To avoid double translation for users that validate with a schema instead of our validators
+                    // we use a special format for our validators errors.
+                    // The ValidationError component will check for this format and extract the message and args
+                    // to translate.
+                    return `@@react-admin@@${JSON.stringify(error)}`;
+                },
+            }
+            : undefined,
         ...options,
     });
     // Because our forms may receive an asynchronously loaded record for instance,

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

packages/ra-core/src/form/groups/useFormGroup.ts から始め、次に packages/ra-core/src/form/useInput.ts と ArrayInputBase の実装を調べます。React 19 上の複数タブの TabbedForm で問題を再現し、SaveButton による送信時のバリデーションを追跡します。フォームが maximum update depth エラーなしで送信され、グループのバリデーション状態が正しいままであれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
react, typescript
領域
frontend
issue の種類
バグ
難易度
4/5
見積もり時間
3〜5日
活発さ
活発
明瞭さ
おおむね明確
初心者へのやさしさ
55/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。