[wip] migrate form to be a function component#5045
[wip] migrate form to be a function component#5045nickgros wants to merge 2 commits intorjsf-team:mainfrom
Conversation
Agent Response: Form.tsx complexity analysisA review of Sources of complexity1. Dual controlled/uncontrolled mode with default injectionThe biggest single source. The form supports both This is the entire reason for:
None of these would exist in a fully-controlled or fully-uncontrolled design. 2. State derivation during renderSchema/formData changes recompute state via synchronous Required because 3. Multiple onChange notification timingsThree separate firing sites, each with its own ref:
These timings exist because the class constructor fired onChange synchronously 4. Pending-changes queueNecessary because computing next state for one change can cause children to The queue + 5. Three error-routing pathways in
|
| Source | Essential? | Why / what could replace it |
|---|---|---|
| Default injection | Yes for JSON Schema | But it shouldn't happen during prop-echo. Could be a discrete pre-Form step (e.g., a getDefaultedFormData(schema, data) helper) callers run when they want defaults. |
| Live validation per-keystroke | Yes for UX | But the mode-toggle complexity is accidental. Pick one mode (or expose two) without legacy boolean overloads. |
| oneOf/anyOf option switching | Yes for JSON Schema | But the null→object heuristic would be unnecessary if the MultiSchemaField sent a clean value instead of an object-of-undefineds. |
| Dual controlled+uncontrolled | No | Modern React form libraries (RHF, Formik) pick one. Going fully-uncontrolled (with defaultFormData only) eliminates userChangeAppliedRef, the prop-echo guards, and the deferred-onChange useEffect entirely. Going fully-controlled eliminates most of computeStateFromProps's prevState branching. |
| Initial onChange before children render | No | Constructor-timing is a class-component artifact. Modern consumers should expect onChange post-mount or not at all. Removing it deletes didInitialOnChangeRef + the special first-render dispatch. |
| State derivation during render | Mostly yes given current API | Could be eliminated by moving form state into useSyncExternalStore or an external store (Zustand-style). Schema changes would be applied imperatively at the source instead of inferred via prop comparison. |
| Pending-changes queue | Yes given current child→parent change cascade | Could be eliminated if children didn't fire onChange during their render (e.g., defaults applied centrally rather than via mount-time onChange from leaf widgets). That's a deeper architectural shift. |
| 3 update strategies | No | Pick one. customDeep is the default and works for the documented use cases. |
liveValidate / liveOmit boolean overloads |
No | Already deprecated in JSDoc — remove in next major. |
omitExtraData + removeEmptyOptionalObjects as separate flags |
No | Could merge into one cleanFormData: 'never' | 'on-blur' | 'on-change' | 'on-submit' flag. |
Recommendations
Highest-leverage simplification: pick controlled OR uncontrolled, not both
That alone deletes:
userChangeAppliedRefand the bail-out logic- The deferred-onChange
useEffect - The prop-echo edge case in
computeChangeNextState
(hasOnlyUndefinedValues/wasPreviouslyNull) - ~30% of the branches in
computeStateFromProps
Migration story for end users:
- If you're currently passing
formDataand readingonChange, you're already
controlled — dropinitialFormData. - If you're using
initialFormData, drop theformDataprop.
The uncomfortable case is users who pass formData and expect Form to inject
defaults their parent state then absorbs — but that pattern is exactly what's
broken about the current design (the ref-based bail-outs are workarounds for
it). The replacement: a getDefaultedFormData(schema, data) helper they call
once before passing data in.
Second-highest leverage: remove the constructor-timing initial onChange
Defaults should either be:
- Applied by the consumer before rendering, or
- Reported via a post-mount effect.
The "fires before children render" guarantee leaks the class-component
lifecycle into a function-component API and forces ref-based coordination.
Removing it deletes didInitialOnChangeRef and the special first-render
dispatch path.
Lower-leverage but cheap cleanups
- Drop
booleanvariants ofliveValidate/liveOmit(already deprecated). - Collapse
omitExtraData+liveOmit+removeEmptyOptionalObjectsinto a
singlecleanFormDataflag. - Drop
experimental_componentUpdateStrategy(or commit to one strategy).
Summary
The complexity falls into four buckets:
- Essential to JSON Schema: defaults, oneOf/anyOf, validation
- Essential to forms in general: controlled or uncontrolled, ref API,
error display - Accidental to supporting BOTH controlled AND uncontrolled with
default-injection: theuserChangeAppliedRef, the prop-echo guards, the
deferred onChange notifications - Workarounds for React lifecycle quirks: the queue, the render-time
dispatch, the multiple ref-based timing tricks
Buckets 1 and 2 are essential. Bucket 3 could be eliminated by making the Form
fully controlled (or fully uncontrolled). Bucket 4 could be eliminated by
moving state outside React or by restructuring how default filling happens.
| export type { | ||
| FormProps, | ||
| FormRef, |
There was a problem hiding this comment.
| export type { | |
| FormProps, | |
| FormRef, | |
| export type { | |
| FormRef as Form, // For typing backwards compatibility | |
| FormProps, | |
| FormRef, |
| import { ADDITIONAL_PROPERTY_KEY_REMOVE } from './constants'; | ||
|
|
||
| import type { FormProps, FormState, IChangeEvent } from './Form'; |
There was a problem hiding this comment.
| import { ADDITIONAL_PROPERTY_KEY_REMOVE } from './constants'; | |
| import type { FormProps, FormState, IChangeEvent } from './Form'; | |
| import { ADDITIONAL_PROPERTY_KEY_REMOVE } from './constants'; | |
| import type { FormProps, FormState, IChangeEvent } from './Form'; |
|
Ran the complexity analysis prompt against the class version of
Review of
|
Reasons for making this change
[Please describe them here]
If this is related to existing tickets, include links to them as well. Use the syntax
fixes #[issue number](ex:fixes #123).If your PR is non-trivial and you'd like to schedule a synchronous review, please add it to the weekly meeting agenda: https://docs.google.com/document/d/12PjTvv21k6LIky6bNQVnsplMLLnmEuypTLQF8a-8Wss/edit
Checklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:updateto update snapshots, if needed.