[form] Fix clearErrors dropping updates when multiple fields change at once

[sai6855]

Issue reproduction

1. Open https://stackblitz.com/edit/wjn5ujru?file=src%2FApp.tsx,src%2Findex.tsx
2. Click on sign-up
3. wait for 1 sec and notice both errors appear
4. Now click on accept all, notice newsletter error survives when both should disappear

[pkg-pr-new]

• base-ui-tanstack-start

• vite-css-base-ui-example

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/react@5446
  

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/utils@5446
  

commit: 86e7ad0

[Copilot]

Pull request overview

Fixes concurrent field error clearing in Base UI forms.

Changes:

• Uses functional state updates to prevent dropped error removals.
• Adds regression coverage for simultaneous field changes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/react/src/form/Form.tsx	Safely composes concurrent error removals.
packages/react/src/form/Form.test.tsx	Tests clearing two errors in one commit.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[code-infra-dashboard]

Bundle size

Bundle	Parsed size	Gzip size
@base-ui/react	🔺+21B(0.00%)	🔺+6B(0.00%)

Details of bundle changes

Performance

Total duration: 1,362.51 ms +48.10 ms^(+3.7%) | Renders: 76 ⁽⁺⁰⁾ | Paint: 2,123.23 ms +37.07 ms^(+1.8%)

Test	Duration	Renders
Tabs mount (200 instances)	238.36 ms 🔺+47.06 ms(+24.6%)	3 (+0)

14 tests within noise — details

---

Check out the code infra dashboard for more information about this PR.

[sai6855]

This fix ensures state consistency by moving error-clearing logic into a functional update, preventing stale state during batched updates when multiple fields change simultaneously.

Previous code referred to errors object which could become stale when react batch updates

[netlify]

✅ Deploy Preview for base-ui ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	4d63010
🔍 Latest deploy log	https://app.netlify.com/projects/base-ui/deploys/6a795a2b81f2f9000822b7ea
😎 Deploy Preview	https://deploy-preview-5446--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[netlify]

✅ Deploy Preview for base-ui ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	86e7ad0
🔍 Latest deploy log	https://app.netlify.com/projects/base-ui/deploys/6a79693c81f2f9000827ad09
😎 Deploy Preview	https://deploy-preview-5446--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[sai6855]

@claude review

[github-actions]

PR review

The core fix is correct: clearErrors now bases each removal on the pending errors state instead of the render-scope closure, so N field controls whose useValueChanged layout effects fire in the same commit no longer clobber each other's removals. I traced all 13 clearErrors call sites (all in event handlers or useValueChanged layout effects — none during render), the layout-effect ordering against useValueChanged(externalErrors, …), and the [errors, focusFirstInvalid] submit-focus effect: the identity-preserving bail-out keeps that effect's deps stable, so submit-focus behavior is unchanged. Nothing is merge-blocking; the remaining notes are test-coverage and placement nits.

Bugs (1)

1. ℹ️ The fix only holds when the errors prop identity is stable (pre-existing, worth documenting)

Location: packages/react/src/form/Form.tsx:75

const [errors, setErrors] = React.useState(externalErrors);

useValueChanged(externalErrors, () => {
  setErrors(externalErrors);
});

Form is an ancestor of every field, so its layout effect runs after the descendants' clearErrors calls. If a consumer passes an inline object (<Form errors={{ a: '…', b: '…' }}>), every re-render gives externalErrors a fresh identity, useValueChanged fires, and the just-cleared errors are restored. This is identical on the base commit — not a regression introduced here — but it is the same user-visible symptom the PR is fixing, so someone who reads the fix may still report "errors don't clear."

Failure scenario: A consumer writes errors={{ email: 'Taken' }} inline; toggling the field clears the error, then Form's layout effect immediately re-applies it, so the error never disappears.

Fix: No change required in this PR. Consider a follow-up note in the errors prop JSDoc / docs that the object must be referentially stable (state-held or memoized), since Form re-syncs on identity change.

Tests (2)

1. 🟡 Batched clear is not asserted to leave untouched fields' errors alone

Location: packages/react/src/form/Form.test.tsx:288

const errors = { a: 'A error', b: 'B error' };

The new test proves both changed fields get cleared, but every key in errors is cleared, so it cannot catch the opposite failure mode the updater could introduce — over-clearing, i.e. a batched update dropping keys for fields that did not change. The existing focuses asynchronously replaced external errors and clears only the changed own property test (line 582) pins that invariant only for a single change, which is exactly the path this PR did not touch.

Failure scenario: A future refactor makes the updater return {} (or the last field's nextErrors computed from a stale base) and both tests still pass, silently wiping errors for fields the user never touched.

Fix: Add a third field c with an error that is never changed, and assert screen.queryByTestId('c-error') is still non-null after clicking "Change both".

2. 🟡 New test sits outside the describe('prop: errors') block that owns its siblings

Location: packages/react/src/form/Form.test.tsx:288

it('removes errors for every field that changes within a single commit', async () => {

This is a prop: errors behavior test, and its nearest sibling — removes errors upon change (line 697), which covers the same clearing path but with each change in its own commit — lives inside describe('prop: errors') at line 553. Splitting them makes the single-commit vs multi-commit pair easy to miss when someone next touches this code.

Failure scenario: A maintainer editing the batched-clear behavior reads the whole prop: errors block, sees only the sequential-change test, and does not realize the batched case is covered elsewhere in the file.

Fix: Move the test into describe('prop: errors') next to removes errors upon change. While there: onCheckedChange={setA} / setB are never exercised (the switches are driven by the button, not clicked), so they can be dropped, and the title reads more like the user-facing behavior as e.g. "removes errors for every field that changes in the same update" — "commit" is React-internal jargon.

Verdict

Approve after nits — the clearErrors change is correct and the added test genuinely fails on the base commit; only test coverage/placement nits remain.

---

🤖 Review generated with Claude Code · Opus 5 (High) · medium review depth · 24 turns · 14m5s · $3.69 · run