Fix unnecessary DOM mutations in updateInput for unchanged inputs (#36229)

[aryashashank]

Summary

Fixes #36229

React 19 introduced a performance regression in updateInput() (ReactDOMInput.js). On every commit, the function unconditionally performs three DOM mutations for every <input> element — even when none of the input-specific props (type, name, value, checked) have changed:

1. node.name = '' — disconnects the input from its radio group
2. node.type = type — rewrites the type property
3. node.name = name — restores the name property

On pages with many <input> elements (e.g. 500+), this causes hundreds of unnecessary DOM writes per render, making updates significantly slower than React 18.

This PR adds change detection by comparing new values against the current DOM state before writing. The node.name = '' disconnection trick — which exists to ensure atomicity when changing type or name alongside checked for radio buttons — is now only performed when type or name is actually changing. All existing semantics, including radio button group behavior, are preserved.

How did you test this change?

Added two new tests to ReactDOMInput-test.js:

1. "should not perform unnecessary DOM mutations for unchanged inputs" — Renders two inputs under a shared parent, installs an Object.defineProperty setter trap on the unchanged input's name property, then updates only the first input's value. The test verifies that the second input's name is never written to.

2. "should not write type or name when they have not changed" — Renders an input, installs a setter trap on type that counts writes, then re-renders with identical props. The test verifies zero writes to type.

Ran the full test suites:

$ node scripts/jest/jest-cli.js packages/react-dom/src/tests/ReactDOMInput-test.js
Test Suites: 1 passed, 1 total
Tests: 126 passed, 126 total

$ node scripts/jest/jest-cli.js packages/react-dom/src/tests/ReactDOMServerIntegrationInput-test.js
Test Suites: 1 passed, 1 total
Tests: 35 passed, 35 total

ESLint and Prettier also pass on the changed files.

[meta-cla]

Hi @aryashashank!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[afurm]

Great fix for this regression. A few questions from reading the diff:

On typeChanged comparison: You compare node.type !== type where node.type is the DOM property (always lowercased per HTML spec) against the React prop value. Since React normalizes type props to lowercase before passing them here, this should always be safe — but is that normalization guaranteed at this call site, or could someone pass an uppercase type prop directly?

On the name attribute branch: The condition if (typeChanged || nameChanged) that guards node.name = nameStr means the name gets written even when only typeChanged is true and name itself has not changed. This restores the radio group disconnection behavior correctly, but — is there a case where typeChanged is true but name should not be written at all (e.g., name prop is null)? Looking at the code flow, when name is null, isNameValid is false and nameChanged would be true (because the DOM has an attribute), so we would write node.removeAttribute('name') — which seems correct. But worth verifying the null-name + type-change case.

Test question: The property descriptor override on secondInput.name uses Object.getOwnPropertyDescriptor to get the original descriptor. For input elements, name is a reflected IDL attribute with a native getter/setter. Does Object.getOwnPropertyDescriptor reliably capture that native behavior, or could it return undefined in some browsers? The fallback handles undefined, just want to confirm the test is actually catching writes in all cases.

[aryashashank]

@afurm

Good points, here's where I landed on each:

1. typeChanged lowercase — Updated. Now using ('' + type).toLowerCase() with the proper DEV coercion check before it. This way even if someone passes type="Text", we won't get a false positive from the browser lowercasing node.type.

2. Null name + type change — Looked into this and it's fine as-is. When name is null and the node has no name attribute, node.name is already '' (DOM default), so node.name = '' is a same-value assignment. And removeAttribute('name') on a non-existent attribute is a no-op. So no real DOM mutations happen. This only fires when type changes anyway, which is rare — the perf problem from #36229 was about these writes hitting every input on every re-render, not the occasional type switch.

3. Test Object.getOwnPropertyDescriptor — Yeah, Object.getOwnPropertyDescriptor(secondInput, 'name') returns undefined since name lives on HTMLInputElement.prototype, not the instance. That's what the fallback is for. We then define an own getter/setter that shadows the prototype and catches writes. Same pattern as the existing "only assigns defaultValue if it changes" test. Works fine in JSDOM.