Skip to content

[wip] migrate form to be a function component#5045

Draft
nickgros wants to merge 2 commits intorjsf-team:mainfrom
nickgros:form-fn
Draft

[wip] migrate form to be a function component#5045
nickgros wants to merge 2 commits intorjsf-team:mainfrom
nickgros:form-fn

Conversation

@nickgros
Copy link
Copy Markdown
Contributor

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

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@nickgros
Copy link
Copy Markdown
Contributor Author

Consider the ongoing refactor of @packages/core/src/components/Form.tsx described in @FORM_MIGRATION.md .

There is a lot of complexity in Form.tsx. This may indicate an issue with code quality, or the component supports use cases that likely have a better approach (we can consider making breaking changes if the behavior we are removing has a workaround/future path for end users that is more sensible or idiomatic).

Review the component and determine:

  • What are the sources of complexity?
  • Is this complexity essential, or is there an easier way to accomplish this?
  • If the complexity is essential but code is unexpectedly complex, what is the purpose? Is this idiomatic, or is there a better way to support this functionality?

Agent Response:

Form.tsx complexity analysis

A review of packages/core/src/components/Form.tsx (post-migration to function
component) and its supporting helpers (formStateHelpers.ts,
hooks/usePendingChanges.ts), evaluating which sources of complexity are
essential and which are accidental.

Sources of complexity

1. Dual controlled/uncontrolled mode with default injection

The biggest single source.

The form supports both formData (controlled) and initialFormData
(uncontrolled), AND injects schema defaults into the data. These three things
conflict: a controlled parent echoes back data → Form recomputes defaults →
may snap away from the user's last choice.

This is the entire reason for:

None of these would exist in a fully-controlled or fully-uncontrolled design.

2. State derivation during render

Schema/formData changes recompute state via synchronous dispatch during
render — the React docs' "last-resort" pattern
(Form.tsx:411-456).

Required because useEffect-based derivation would cause a visible flash. This
forces every prop-change into a comparison + dispatch dance with three
different deep-equality checks (schema, formData, formData-vs-state).

3. Multiple onChange notification timings

Three separate firing sites, each with its own ref:

These timings exist because the class constructor fired onChange synchronously
before mount, and tests + consumers rely on that ordering.

4. Pending-changes queue

Necessary because computing next state for one change can cause children to
fire more changes during their re-render (e.g., default-filling a
newly-revealed object's required fields).

The queue + isLastInQueue gate exists to coalesce live-validation into one
run per burst (usePendingChanges.ts).

5. Three error-routing pathways in computeChangeNextState

newErrorSchema either updates originalErrorSchema (if there's already a
schema-validation error at that path) or routes to customErrors
(formStateHelpers.ts:599-625).

This dual bookkeeping is so the next live-validate can distinguish
"user-supplied error" from "schema-validation error" and merge correctly.

6. Three component-update strategies

customDeep / shallow / always wired through React.memo with a custom
comparator that reads the experimental_componentUpdateStrategy prop.

7. Many overloaded-meaning props

  • liveValidate / liveOmit accept boolean | 'onChange' | 'onBlur' — three
    deprecation paths in one
  • noValidate (deprecated, partially overlaps with validator being optional)
  • omitExtraData + liveOmit (two flags for one feature)
  • removeEmptyOptionalObjects (third hygiene flag)

Essential vs. accidental

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:

  • userChangeAppliedRef and 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 formData and reading onChange, you're already
    controlled — drop initialFormData.
  • If you're using initialFormData, drop the formData prop.

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:

  1. Applied by the consumer before rendering, or
  2. 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 boolean variants of liveValidate / liveOmit (already deprecated).
  • Collapse omitExtraData + liveOmit + removeEmptyOptionalObjects into a
    single cleanFormData flag.
  • Drop experimental_componentUpdateStrategy (or commit to one strategy).

Summary

The complexity falls into four buckets:

  1. Essential to JSON Schema: defaults, oneOf/anyOf, validation
  2. Essential to forms in general: controlled or uncontrolled, ref API,
    error display
  3. Accidental to supporting BOTH controlled AND uncontrolled with
    default-injection
    : the userChangeAppliedRef, the prop-echo guards, the
    deferred onChange notifications
  4. 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.

Comment on lines 9 to +11
export type {
FormProps,
FormRef,
Copy link
Copy Markdown
Member

@heath-freenome heath-freenome Apr 30, 2026

Choose a reason for hiding this comment

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

Suggested change
export type {
FormProps,
FormRef,
export type {
FormRef as Form, // For typing backwards compatibility
FormProps,
FormRef,

Comment on lines +41 to +43
import { ADDITIONAL_PROPERTY_KEY_REMOVE } from './constants';

import type { FormProps, FormState, IChangeEvent } from './Form';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
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';

@nickgros
Copy link
Copy Markdown
Contributor Author

nickgros commented May 1, 2026

Ran the complexity analysis prompt against the class version of Form:

Consider @packages/core/src/components/Form.tsx

There is a lot of complexity in Form.tsx. This may indicate an issue with code quality, or the component supports use cases that likely have a better approach (we can consider making breaking changes if the behavior we are removing has a workaround/future path for end users that is more sensible or idiomatic).

Review the component and determine:

  • What are the sources of complexity?
  • Is this complexity essential, or is there an easier way to accomplish this?
  • If the complexity is essential but code is unexpectedly complex, what is the purpose? Is this idiomatic, or is there a better way to support this functionality?

Review of Form.tsx Complexity

I've read the file in full. Here is my analysis.

Sources of complexity

1. State that mirrors and derives from props

The component holds in state: schema, uiSchema, formData, errors, errorSchema, schemaValidationErrors, schemaValidationErrorSchema, customErrors, retrievedSchema, schemaUtils, registry, fieldPathId, _prevExtraErrors, initialDefaultsGenerated. Most of these are derived from props. That requires three lifecycle hooks to keep them in sync:

  • getDerivedStateFromProps — re-merges errors when extraErrors changes
  • getSnapshotBeforeUpdate — runs the heavy getStateFromProps on any prop change
  • componentDidUpdate — applies the snapshot, fires onChange, with a flag to suppress reverts

2. Three independent error sources merged at multiple sites

schemaValidationErrors (AJV), extraErrors (props), customErrors (errorSchemas passed into onChange by fields). They're merged by mergeErrors / validationDataMerge from at least four call sites: getDerivedStateFromProps, getStateFromProps, liveValidate, processPendingChange. The recent mergeBaseErrorSchema work at L900–L972 exists specifically because passing state.errorSchema (which already had extraErrors merged in) caused duplication.

3. Pending-change queue with self-recursing drain (L871–L1020)

onChange pushes to pendingChanges, and processPendingChange shifts one entry, runs validation/defaults, calls setState, then recurses in the setState callback. This is hand-rolled batching — necessary because each entry's processing depends on the previous setState having committed.

4. The _isProcessingUserChange workaround (L383, L518–L524, L893)

Set to true at the start of processPendingChange, checked in componentDidUpdate to skip applying nextState when it would clobber a oneOf/anyOf option switch. This is a band-aid for a real race: processPendingChange setStates → fires props.onChange → parent re-renders with controlled formDatagetSnapshotBeforeUpdate recomputes state from props → and that recomputation can disagree with what we just set (because retrieveSchema resolves a different oneOf branch).

5. Sentinel values

  • IS_RESET (a symbol) is passed as formData through getStateFromProps to signal "treat as undefined" (L591).
  • ADDITIONAL_PROPERTY_KEY_REMOVE is passed as newValue to signal "unset this key" (L922).

Both stuff out-of-band intent into in-band parameters.

6. getStateFromProps is the kitchen sink (L545–L693)

~150 lines, called from the constructor, getSnapshotBeforeUpdate, processPendingChange, and reset. It (re)builds schemaUtils, computes defaults, retrieves the resolved schema, runs live validation, rebuilds the registry, computes a fieldPathId, and merges errors. Each caller passes different combinations of six positional arguments, and several behaviors hinge on whether arguments are undefined.

7. Three configurable shouldComponentUpdate strategies

experimental_componentUpdateStrategy: 'customDeep' | 'shallow' | 'always' (L702). The fact that we let users pick suggests customDeep is too slow on large forms but shallow breaks correctness for users who pass fresh references — i.e. the rendering pipeline isn't reference-stable.

8. Deprecated boolean modes

liveValidate and liveOmit accept boolean | 'onChange' | 'onBlur', with a comment noting booleans will be removed in the future major. Two-shape APIs duplicate every branch.

9. Validation done twice on submit

validateFormWithFormData runs this.validate, sets state from the result, then onSubmit fires with state's formData. Live validation paths run their own validate. Each path manages its own merge of extraErrors/customErrors.


What is essential vs. accidental

Essential (the domain genuinely requires it):

  • Computing JSON-Schema defaults
  • Resolving oneOf/anyOf/dependencies against current data via retrieveSchema
  • Validating with AJV and merging multiple error sources
  • Supporting controlled and uncontrolled modes
  • Theme/registry plugin extension

Accidental (avoidable with a different design):

  • The pending-changes queue and its self-recursion
  • _isProcessingUserChange and the componentDidUpdate revert race
  • Symbol sentinels (IS_RESET, ADDITIONAL_PROPERTY_KEY_REMOVE)
  • The user-selectable shouldComponentUpdate strategy
  • Two different error-merge code paths in getStateFromProps vs processPendingChange
  • getStateFromProps doing schema work, defaults, validation, and registry construction in one function
  • Maintaining schema, uiSchema, registry in state when they could be derived inline

Where idiomatic alternatives exist

1. Split derived state from view state. schemaUtils, registry, rootSchema, retrievedSchema, fieldPathId are pure functions of props + current formData. They belong in useMemo (or class equivalents like memoize-one), not in setState. Today they live in state because all the lifecycle juggling assumes one big nextState blob — splitting them dissolves most of getSnapshotBeforeUpdate. The view state (formData, error trio) is the only thing that genuinely needs to live in setState.

2. The race that motivates _isProcessingUserChange is a controlled-component anti-pattern. When the parent is controlled, the form should not also recompute formData from props on every render — props are the source of truth, full stop. The recompute exists because getStateFromProps rebuilds formData through getDefaultFormState and retrieveSchema even when called from getSnapshotBeforeUpdate. Splitting "rebuild defaults" (initial-only) from "resolve schema" (every render) removes the race entirely; the flag becomes unnecessary.

3. Replace IS_RESET and ADDITIONAL_PROPERTY_KEY_REMOVE symbols with explicit methods/paths. reset() already is a separate method — it shouldn't reuse getStateFromProps's formData parameter to communicate intent; it can compute defaults directly. ADDITIONAL_PROPERTY_KEY_REMOVE could be a separate removeField(path) action, or callers could just send undefined with a flag.

4. The pending-changes queue exists to compensate for setState async-ness, but that was already React's design. A simpler model: each onChange produces a partial reducer step; merge all queued steps into a single setState with a function. Or: rely on React 18's automatic batching and accept that getStateFromProps can run multiple times. The current self-recursion is hand-rolling what the runtime now does for free.

5. The experimental_componentUpdateStrategy escape hatch is a symptom of unstable references. The fix isn't to let users opt out of the deep check — it's to make registry, schemaUtils, templates, etc. reference-stable across renders. (useMemo again, or memoize-one in the class context.) The getRegistry + deepEquals(state.registry, newRegistry) dance at L669–L670 is doing this manually and incompletely.

6. Booleans in liveValidate / liveOmit should just be removed. The codebase already calls them deprecated; every branch checks both === true and === 'onChange'. Cutting the booleans is a clean breaking change with a one-line migration path.

7. The class form makes all of this harder than it has to be. Hooks would let useMemo for derived state, useReducer for the queue, and remove the lifecycle triangle entirely. This is a breaking change because the public API exposes a Ref<Form<T, S, F>> with imperative methods (reset, submit, validateForm, setFieldValue, focusOnError). But useImperativeHandle reproduces those exactly, so it's a major-version mechanical migration, not a behavior change.


Bottom line

The essential complexity is real — JSON Schema forms must compute defaults, resolve oneOf branches against live data, and merge three error sources. But probably 40–60% of the file is plumbing for a state model that conflates "derived view of props" with "owned mutable state." The race-condition flag, the pending-changes queue, the symbol sentinels, and the configurable update strategy are all symptoms of that conflation.

The highest-leverage change, even staying as a class component, is to move schemaUtils / registry / retrievedSchema / fieldPathId out of state into memoized derivations. That alone removes most of getSnapshotBeforeUpdate and probably eliminates the need for _isProcessingUserChange. A full hooks rewrite would compound that savings but is a larger, breaking move best paired with the liveValidate boolean removal in a major bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants