Refactor DOM special cases per tags including controlled fields #26501

sebmarkbage · 2023-03-29T01:08:53Z

I use a shared helper when setting properties into a helper whether it's initial or update.

I moved the special cases per tag to commit phase so we can check it only once. This also effectively inlines getHostProps which can be done in a single check per prop key.

The diffProperties operation is simplified to mostly just generating a plain diff of all properties, generating an update payload. This might generate a few more entries that are now ignored in the commit phase. that previously would've been ignored earlier. We could skip this and just do the whole diff in the commit phase by always scheduling a commit phase update.

I tested the attribute table (one change documented below) and a few select DOM fixtures.

react-sizebot · 2023-03-29T01:14:10Z

Comparing: 5cbe625...08de5b1

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	162.97 kB	162.20 kB	=	51.50 kB	51.29 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	164.71 kB	163.85 kB	=	52.03 kB	51.79 kB
facebook-www/ReactDOM-prod.classic.js	=	556.92 kB	556.07 kB	=	98.54 kB	98.25 kB
facebook-www/ReactDOM-prod.modern.js	=	540.64 kB	539.78 kB	=	96.12 kB	95.83 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactDOMTesting-dev.classic.js	=	1,419.83 kB	1,416.81 kB	+0.02%	307.35 kB	307.40 kB
facebook-www/ReactDOM-dev.classic.js	=	1,401.43 kB	1,398.40 kB	+0.01%	303.07 kB	303.11 kB
facebook-www/ReactDOMTesting-dev.modern.js	=	1,392.11 kB	1,389.09 kB	=	302.18 kB	302.18 kB
facebook-www/ReactDOM-dev.modern.js	=	1,373.71 kB	1,370.69 kB	=	297.75 kB	297.75 kB
oss-stable/react-dom/umd/react-dom.profiling.min.js	=	171.84 kB	171.08 kB	=	54.14 kB	53.99 kB
oss-stable-semver/react-dom/umd/react-dom.profiling.min.js	=	171.77 kB	171.00 kB	=	54.12 kB	53.97 kB
oss-stable/react-dom/cjs/react-dom.profiling.min.js	=	172.58 kB	171.81 kB	=	53.98 kB	53.73 kB
oss-stable-semver/react-dom/cjs/react-dom.profiling.min.js	=	172.51 kB	171.74 kB	=	53.95 kB	53.71 kB
oss-stable/react-dom/umd/react-dom.production.min.js	=	162.87 kB	162.11 kB	=	51.87 kB	51.65 kB
oss-stable-semver/react-dom/umd/react-dom.production.min.js	=	162.80 kB	162.03 kB	=	51.84 kB	51.63 kB
oss-stable/react-dom/cjs/react-dom.production.min.js	=	162.97 kB	162.20 kB	=	51.50 kB	51.29 kB
oss-stable-semver/react-dom/cjs/react-dom.production.min.js	=	162.90 kB	162.13 kB	=	51.48 kB	51.27 kB
oss-experimental/react-dom/umd/react-dom.profiling.min.js	=	173.58 kB	172.73 kB	=	54.71 kB	54.54 kB
oss-experimental/react-dom/cjs/react-dom.profiling.min.js	=	174.33 kB	173.46 kB	=	54.49 kB	54.23 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.production.min.js	=	170.92 kB	170.06 kB	=	54.35 kB	54.13 kB
oss-experimental/react-dom/umd/react-dom.production.min.js	=	164.62 kB	163.76 kB	=	52.42 kB	52.18 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	164.71 kB	163.85 kB	=	52.03 kB	51.79 kB

Generated by 🚫 dangerJS against 08de5b1

sebmarkbage · 2023-03-29T01:18:32Z

packages/react-dom-bindings/src/client/ReactDOMTextarea.js

@@ -120,8 +95,10 @@ export function initWrapperState(element: Element, props: Object) {
    initialValue = defaultValue;
  }

+  const stringValue = getToStringValue(initialValue);
+  node.defaultValue = (stringValue: any); // This will be toString:ed.


This used to be set by setting the children in getHostProps which used textContent which is the same as setting defaultValue of a text area.

sebmarkbage · 2023-03-29T01:20:55Z

packages/react-dom-bindings/src/client/ReactDOMInput.js

+  // TODO: I'm pretty sure this is a bug because initialValueTracking won't be
+  // correct for the hydration case then.
+  if (!isHydrating) {
+    node.checked = !!node._wrapperState.initialChecked;


This was set by getHostProps earlier and is now initialized here. Which then also ensures it's tracked by inputValueTracking by going through the setter which is how other values already worked.

sebmarkbage · 2023-03-29T02:14:20Z

packages/react-dom-bindings/src/client/ReactDOMInput.js

  node._wrapperState = {
    initialChecked:
-      props.checked != null ? props.checked : props.defaultChecked,
+      typeof initialChecked !== 'function' &&
+      typeof initialChecked !== 'symbol' &&


There's a subtle change to the attribute table here.

Because defaultChecked was set here to the value and then that was used by its truthy value in postMountWrapper to set the defaultChecked value. That's not the semantics for boolean properties elsewhere and not what the server does.

This fixes that.

This isn't necessary because we always call "updateWrapper" after the event handling. We can't rely on even getting here if no props have changed since we could've bailed on the props object alone before getting here.

We don't really do anything between init and post mount, other than scheduling for later. So there's no behavior difference.

sebmarkbage · 2023-03-29T02:37:28Z

packages/react-dom-bindings/src/client/ReactDOMComponent.js

+            const node = ((domElement: any): InputWithWrapperState);
+            const checked =
+              propValue != null ? propValue : node._wrapperState.initialChecked;
+            node.checked =


There's something iffy about this that we're updating this in ReactDOMInputUpdateChecked right above and then again here. Seems like we only need one. But this is effectively what was happening before.

I use a shared helper when setting properties into a helper whether it's initial or update. I moved the special cases per tag to commit phase so we can check it only once. This also effectively inlines getHostProps which can be done in a single check per prop key. The diffProperties operation is simplified to mostly just generating a plain diff of all properties, generating an update payload. This might generate a few more entries that are now ignored in the commit phase. that previously would've been ignored earlier. We could skip this and just do the whole diff in the commit phase by always scheduling a commit phase update. I tested the attribute table (one change documented below) and a few select DOM fixtures. DiffTrain build for [85de6fd](85de6fd)

Summary: This sync includes the following changes: - **[ca01f359b](facebook/react@ca01f359b )**: Remove skipUnmountedBoundaries ([#26489](facebook/react#26489)) //<Ricky>// - **[43a70a610](facebook/react@43a70a610 )**: Limit the meaning of "custom element" to not include `is` ([#26524](facebook/react#26524)) //<Sebastian Markbåge>// - **[1308e49a6](facebook/react@1308e49a6 )**: [Flight Plugin] Scan for "use client" ([#26474](facebook/react#26474)) //<dan>// - **[1a1d61fed](facebook/react@1a1d61fed )**: Warn for ARIA typos on custom elements ([#26523](facebook/react#26523)) //<Sebastian Markbåge>// - **[73deff0d5](facebook/react@73deff0d5 )**: Refactor DOMProperty and CSSProperty ([#26513](facebook/react#26513)) //<Sebastian Markbåge>// - **[2d51251e6](facebook/react@2d51251e6 )**: Clean up deferRenderPhaseUpdateToNextBatch ([#26511](facebook/react#26511)) //<Andrew Clark>// - **[0ffc7f632](facebook/react@0ffc7f632 )**: Update useMemoCache test to confirm that cache persists across errors ([#26510](facebook/react#26510)) //<Joseph Savona>// - **[29a3be78b](facebook/react@29a3be78b )**: Move ReactDOMFloat to react-dom/src/ ([#26514](facebook/react#26514)) //<Sebastian Markbåge>// - **[4c2fc0190](facebook/react@4c2fc0190 )**: Generate safe javascript url instead of throwing with disableJavaScriptURLs is on ([#26507](facebook/react#26507)) //<Sebastian Markbåge>// - **[f0aafa1a7](facebook/react@f0aafa1a7 )**: Convert a few more tests to waitFor test helpers ([#26509](facebook/react#26509)) //<Andrew Clark>// - **[90995ef8b](facebook/react@90995ef8b )**: Delete "triangle" resuming fuzz tester ([#26508](facebook/react#26508)) //<Andrew Clark>// - **[f118b7ceb](facebook/react@f118b7ceb )**: [Flight] Gated test for dropped transport of undefined object values ([#26478](facebook/react#26478)) //<Sebastian Silbermann>// - **[fd0511c72](facebook/react@fd0511c72 )**: [Flight] Add support BigInt support ([#26479](facebook/react#26479)) //<Sebastian Silbermann>// - **[85de6fde5](facebook/react@85de6fde5 )**: Refactor DOM special cases per tags including controlled fields ([#26501](facebook/react#26501)) //<Sebastian Markbåge>// - **[1f5cdf8c7](facebook/react@1f5cdf8c7 )**: Update Suspense fuzz tests to use `act` ([#26498](facebook/react#26498)) //<Andrew Clark>// - **[f62cb39ee](facebook/react@f62cb39ee )**: Make disableSchedulerTimeoutInWorkLoop a static ff ([#26497](facebook/react#26497)) //<Ricky>// - **[41b4714f1](facebook/react@41b4714f1 )**: Remove disableNativeComponentFrames ([#26490](facebook/react#26490)) //<Ricky>// - **[fc90eb636](facebook/react@fc90eb636 )**: Codemod more tests to waitFor pattern ([#26494](facebook/react#26494)) //<Andrew Clark>// - **[e0bbc2662](facebook/react@e0bbc2662 )**: Improve tests that deal with microtasks ([#26493](facebook/react#26493)) //<Andrew Clark>// - **[8faf75193](facebook/react@8faf75193 )**: Codemod some expiration tests to waitForExpired ([#26491](facebook/react#26491)) //<Andrew Clark>// - **[8342a0992](facebook/react@8342a0992 )**: Remove unused feature flag disableSchedulerTimeoutBasedOnReactExpirationTime ([#26488](facebook/react#26488)) //<Jan Kassens>// - **[afea1d0c5](facebook/react@afea1d0c5 )**: [flow] make Flow suppressions explicit on the error ([#26487](facebook/react#26487)) //<Jan Kassens>// - **[768f965de](facebook/react@768f965de )**: Suspensily committing a prerendered tree ([#26434](facebook/react#26434)) //<Andrew Clark>// - **[d12bdcda6](facebook/react@d12bdcda6 )**: Fix Flow types of useEffectEvent ([#26468](facebook/react#26468)) //<Sebastian Silbermann>// - **[73b6435ca](facebook/react@73b6435ca )**: [Float][Fiber] Implement waitForCommitToBeReady for stylesheet resources ([#26450](facebook/react#26450)) //<Josh Story>// - **[175962c10](facebook/react@175962c10 )**: Fix remaining CommonJS imports after Rollup upgrade ([#26473](facebook/react#26473)) //<dan>// - **[909c6dacf](facebook/react@909c6dacf )**: Update Rollup to 3.x ([#26442](facebook/react#26442)) //<Mark Erikson>// - **[9c54b29b4](facebook/react@9c54b29b4 )**: Remove ReactFabricPublicInstance and used definition from ReactNativePrivateInterface ([#26437](facebook/react#26437)) //<Rubén Norte>// - **[f77099b6f](facebook/react@f77099b6f )**: Remove layout effect warning on the server ([#26395](facebook/react#26395)) //<Ricky>// - **[51a7c45f8](facebook/react@51a7c45f8 )**: Bugfix: SuspenseList incorrectly forces a fallback ([#26453](facebook/react#26453)) //<Andrew Clark>// - **[afb3d51dc](facebook/react@afb3d51dc )**: Fix enableClientRenderFallbackOnTextMismatch flag ([#26457](facebook/react#26457)) //<Sebastian Markbåge>// - **[8e17bfd14](facebook/react@8e17bfd14 )**: Make InternalInstanceHandle type opaque in ReactNativeTypes ([#26461](facebook/react#26461)) //<Rubén Norte>// - **[b93b4f074](facebook/react@b93b4f074 )**: Should not throw for children of iframe or object ([#26458](facebook/react#26458)) //<Sebastian Markbåge>// - **[c0b34bc5f](facebook/react@c0b34bc5f )**: chore: update links of docs and api ([#26455](facebook/react#26455)) //<Leedom>// - **[ffb6733ee](facebook/react@ffb6733ee )**: fix docs link for useSyncExternalStore ([#26452](facebook/react#26452)) //<Valor(华洛)>// - **[12a1d140e](facebook/react@12a1d140e )**: Don't prerender siblings of suspended component ([#26380](facebook/react#26380)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions 77ba161...ca01f35 jest_e2e[run_all_tests] bypass-github-export-checks Reviewed By: sammy-SC Differential Revision: D44669450 fbshipit-source-id: f160aad4719a00df3ceeca78d5f3fcd0aa0f8437

Summary: This sync includes the following changes: - **[ca01f359b](facebook/react@ca01f359b )**: Remove skipUnmountedBoundaries ([facebook#26489](facebook/react#26489)) //<Ricky>// - **[43a70a610](facebook/react@43a70a610 )**: Limit the meaning of "custom element" to not include `is` ([facebook#26524](facebook/react#26524)) //<Sebastian Markbåge>// - **[1308e49a6](facebook/react@1308e49a6 )**: [Flight Plugin] Scan for "use client" ([facebook#26474](facebook/react#26474)) //<dan>// - **[1a1d61fed](facebook/react@1a1d61fed )**: Warn for ARIA typos on custom elements ([facebook#26523](facebook/react#26523)) //<Sebastian Markbåge>// - **[73deff0d5](facebook/react@73deff0d5 )**: Refactor DOMProperty and CSSProperty ([facebook#26513](facebook/react#26513)) //<Sebastian Markbåge>// - **[2d51251e6](facebook/react@2d51251e6 )**: Clean up deferRenderPhaseUpdateToNextBatch ([facebook#26511](facebook/react#26511)) //<Andrew Clark>// - **[0ffc7f632](facebook/react@0ffc7f632 )**: Update useMemoCache test to confirm that cache persists across errors ([facebook#26510](facebook/react#26510)) //<Joseph Savona>// - **[29a3be78b](facebook/react@29a3be78b )**: Move ReactDOMFloat to react-dom/src/ ([facebook#26514](facebook/react#26514)) //<Sebastian Markbåge>// - **[4c2fc0190](facebook/react@4c2fc0190 )**: Generate safe javascript url instead of throwing with disableJavaScriptURLs is on ([facebook#26507](facebook/react#26507)) //<Sebastian Markbåge>// - **[f0aafa1a7](facebook/react@f0aafa1a7 )**: Convert a few more tests to waitFor test helpers ([facebook#26509](facebook/react#26509)) //<Andrew Clark>// - **[90995ef8b](facebook/react@90995ef8b )**: Delete "triangle" resuming fuzz tester ([facebook#26508](facebook/react#26508)) //<Andrew Clark>// - **[f118b7ceb](facebook/react@f118b7ceb )**: [Flight] Gated test for dropped transport of undefined object values ([facebook#26478](facebook/react#26478)) //<Sebastian Silbermann>// - **[fd0511c72](facebook/react@fd0511c72 )**: [Flight] Add support BigInt support ([facebook#26479](facebook/react#26479)) //<Sebastian Silbermann>// - **[85de6fde5](facebook/react@85de6fde5 )**: Refactor DOM special cases per tags including controlled fields ([facebook#26501](facebook/react#26501)) //<Sebastian Markbåge>// - **[1f5cdf8c7](facebook/react@1f5cdf8c7 )**: Update Suspense fuzz tests to use `act` ([facebook#26498](facebook/react#26498)) //<Andrew Clark>// - **[f62cb39ee](facebook/react@f62cb39ee )**: Make disableSchedulerTimeoutInWorkLoop a static ff ([facebook#26497](facebook/react#26497)) //<Ricky>// - **[41b4714f1](facebook/react@41b4714f1 )**: Remove disableNativeComponentFrames ([facebook#26490](facebook/react#26490)) //<Ricky>// - **[fc90eb636](facebook/react@fc90eb636 )**: Codemod more tests to waitFor pattern ([facebook#26494](facebook/react#26494)) //<Andrew Clark>// - **[e0bbc2662](facebook/react@e0bbc2662 )**: Improve tests that deal with microtasks ([facebook#26493](facebook/react#26493)) //<Andrew Clark>// - **[8faf75193](facebook/react@8faf75193 )**: Codemod some expiration tests to waitForExpired ([facebook#26491](facebook/react#26491)) //<Andrew Clark>// - **[8342a0992](facebook/react@8342a0992 )**: Remove unused feature flag disableSchedulerTimeoutBasedOnReactExpirationTime ([facebook#26488](facebook/react#26488)) //<Jan Kassens>// - **[afea1d0c5](facebook/react@afea1d0c5 )**: [flow] make Flow suppressions explicit on the error ([facebook#26487](facebook/react#26487)) //<Jan Kassens>// - **[768f965de](facebook/react@768f965de )**: Suspensily committing a prerendered tree ([facebook#26434](facebook/react#26434)) //<Andrew Clark>// - **[d12bdcda6](facebook/react@d12bdcda6 )**: Fix Flow types of useEffectEvent ([facebook#26468](facebook/react#26468)) //<Sebastian Silbermann>// - **[73b6435ca](facebook/react@73b6435ca )**: [Float][Fiber] Implement waitForCommitToBeReady for stylesheet resources ([facebook#26450](facebook/react#26450)) //<Josh Story>// - **[175962c10](facebook/react@175962c10 )**: Fix remaining CommonJS imports after Rollup upgrade ([facebook#26473](facebook/react#26473)) //<dan>// - **[909c6dacf](facebook/react@909c6dacf )**: Update Rollup to 3.x ([facebook#26442](facebook/react#26442)) //<Mark Erikson>// - **[9c54b29b4](facebook/react@9c54b29b4 )**: Remove ReactFabricPublicInstance and used definition from ReactNativePrivateInterface ([facebook#26437](facebook/react#26437)) //<Rubén Norte>// - **[f77099b6f](facebook/react@f77099b6f )**: Remove layout effect warning on the server ([facebook#26395](facebook/react#26395)) //<Ricky>// - **[51a7c45f8](facebook/react@51a7c45f8 )**: Bugfix: SuspenseList incorrectly forces a fallback ([facebook#26453](facebook/react#26453)) //<Andrew Clark>// - **[afb3d51dc](facebook/react@afb3d51dc )**: Fix enableClientRenderFallbackOnTextMismatch flag ([facebook#26457](facebook/react#26457)) //<Sebastian Markbåge>// - **[8e17bfd14](facebook/react@8e17bfd14 )**: Make InternalInstanceHandle type opaque in ReactNativeTypes ([facebook#26461](facebook/react#26461)) //<Rubén Norte>// - **[b93b4f074](facebook/react@b93b4f074 )**: Should not throw for children of iframe or object ([facebook#26458](facebook/react#26458)) //<Sebastian Markbåge>// - **[c0b34bc5f](facebook/react@c0b34bc5f )**: chore: update links of docs and api ([facebook#26455](facebook/react#26455)) //<Leedom>// - **[ffb6733ee](facebook/react@ffb6733ee )**: fix docs link for useSyncExternalStore ([facebook#26452](facebook/react#26452)) //<Valor(华洛)>// - **[12a1d140e](facebook/react@12a1d140e )**: Don't prerender siblings of suspended component ([facebook#26380](facebook/react#26380)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions 77ba161...ca01f35 jest_e2e[run_all_tests] bypass-github-export-checks Reviewed By: sammy-SC Differential Revision: D44669450 fbshipit-source-id: f160aad4719a00df3ceeca78d5f3fcd0aa0f8437

I use a shared helper when setting properties into a helper whether it's initial or update. I moved the special cases per tag to commit phase so we can check it only once. This also effectively inlines getHostProps which can be done in a single check per prop key. The diffProperties operation is simplified to mostly just generating a plain diff of all properties, generating an update payload. This might generate a few more entries that are now ignored in the commit phase. that previously would've been ignored earlier. We could skip this and just do the whole diff in the commit phase by always scheduling a commit phase update. I tested the attribute table (one change documented below) and a few select DOM fixtures. DiffTrain build for commit 85de6fd.

sebmarkbage added 2 commits March 28, 2023 20:58

Extract setting properties into a helper whether it's initial or update

71a692f

Move special cases to commit phase and inline getHostProps

d741e00

sebmarkbage requested review from gaearon and acdlite March 29, 2023 01:08

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 29, 2023

sebmarkbage commented Mar 29, 2023

View reviewed changes

Ensure check is a boolean (function/symbol are false)

e4aeee8

sebmarkbage force-pushed the refactorspecialtags branch from 7d19cf1 to e4aeee8 Compare March 29, 2023 01:49

sebmarkbage commented Mar 29, 2023

View reviewed changes

sebmarkbage added 2 commits March 28, 2023 22:16

Remove force commit phase

9c3ef63

This isn't necessary because we always call "updateWrapper" after the event handling. We can't rely on even getting here if no props have changed since we could've bailed on the props object alone before getting here.

Unify switches when hydrating

08de5b1

We don't really do anything between init and post mount, other than scheduling for later. So there's no behavior difference.

sebmarkbage commented Mar 29, 2023

View reviewed changes

acdlite approved these changes Mar 29, 2023

View reviewed changes

sebmarkbage merged commit 85de6fd into facebook:main Mar 29, 2023

sammy-SC mentioned this pull request Apr 5, 2023

Use native scheduler if defined sammy-SC/react#1

Closed

eps1lon mentioned this pull request Sep 25, 2024

[React 19] [bug] SVG with dangerouslySetInnerHTML content does not trigger first click #30994

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor DOM special cases per tags including controlled fields #26501

Refactor DOM special cases per tags including controlled fields #26501

sebmarkbage commented Mar 29, 2023 •

edited

Loading

react-sizebot commented Mar 29, 2023 •

edited

Loading

sebmarkbage Mar 29, 2023

sebmarkbage Mar 29, 2023

sebmarkbage Mar 29, 2023

sebmarkbage Mar 29, 2023 •

edited

Loading

Refactor DOM special cases per tags including controlled fields #26501

Refactor DOM special cases per tags including controlled fields #26501

Conversation

sebmarkbage commented Mar 29, 2023 • edited Loading

react-sizebot commented Mar 29, 2023 • edited Loading

Critical size changes

Significant size changes

sebmarkbage Mar 29, 2023

Choose a reason for hiding this comment

sebmarkbage Mar 29, 2023

Choose a reason for hiding this comment

sebmarkbage Mar 29, 2023

Choose a reason for hiding this comment

sebmarkbage Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

sebmarkbage commented Mar 29, 2023 •

edited

Loading

react-sizebot commented Mar 29, 2023 •

edited

Loading

sebmarkbage Mar 29, 2023 •

edited

Loading