From fc1026b8b3e6c63806cf3657e39081e097dc8762 Mon Sep 17 00:00:00 2001 From: sebmarkbage Date: Wed, 29 Mar 2023 02:45:08 +0000 Subject: [PATCH] Refactor DOM special cases per tags including controlled fields (#26501) 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 [85de6fde515148babd36eae2b7384ad8e62b732a](https://github.com/facebook/react/commit/85de6fde515148babd36eae2b7384ad8e62b732a) --- compiled/facebook-www/REVISION | 2 +- compiled/facebook-www/React-dev.modern.js | 2 +- compiled/facebook-www/ReactART-dev.classic.js | 2 +- compiled/facebook-www/ReactART-prod.modern.js | 4 +- compiled/facebook-www/ReactDOM-dev.classic.js | 1292 +++++++------- compiled/facebook-www/ReactDOM-dev.modern.js | 1292 +++++++------- .../facebook-www/ReactDOM-prod.classic.js | 1476 ++++++++-------- compiled/facebook-www/ReactDOM-prod.modern.js | 1242 +++++++------- .../ReactDOM-profiling.classic.js | 1520 ++++++++--------- .../facebook-www/ReactDOM-profiling.modern.js | 1228 +++++++------ .../ReactDOMTesting-dev.classic.js | 1300 +++++++------- .../ReactDOMTesting-dev.modern.js | 1292 +++++++------- .../ReactDOMTesting-prod.classic.js | 1238 +++++++------- .../ReactDOMTesting-prod.modern.js | 1242 +++++++------- .../ReactTestRenderer-dev.modern.js | 2 +- 15 files changed, 6251 insertions(+), 6883 deletions(-) diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION index 0e7e4f9207af0..dac374ec89a05 100644 --- a/compiled/facebook-www/REVISION +++ b/compiled/facebook-www/REVISION @@ -1 +1 @@ -f62cb39ee58e5918806218de2be8d2833023e7fc +85de6fde515148babd36eae2b7384ad8e62b732a diff --git a/compiled/facebook-www/React-dev.modern.js b/compiled/facebook-www/React-dev.modern.js index c9b8744e8bc1a..c5316fd9a2bf3 100644 --- a/compiled/facebook-www/React-dev.modern.js +++ b/compiled/facebook-www/React-dev.modern.js @@ -27,7 +27,7 @@ if ( } "use strict"; -var ReactVersion = "18.3.0-www-modern-0ec8c3f1"; +var ReactVersion = "18.3.0-www-modern-3ed4a7e4"; // ATTENTION // When adding new symbols to this file, diff --git a/compiled/facebook-www/ReactART-dev.classic.js b/compiled/facebook-www/ReactART-dev.classic.js index 7448444fa8004..200c405a1d053 100644 --- a/compiled/facebook-www/ReactART-dev.classic.js +++ b/compiled/facebook-www/ReactART-dev.classic.js @@ -69,7 +69,7 @@ function _assertThisInitialized(self) { return self; } -var ReactVersion = "18.3.0-www-classic-31f56934"; +var ReactVersion = "18.3.0-www-classic-aa994cf1"; var LegacyRoot = 0; var ConcurrentRoot = 1; diff --git a/compiled/facebook-www/ReactART-prod.modern.js b/compiled/facebook-www/ReactART-prod.modern.js index a15d1dcd2df7f..a57d5d28e03c2 100644 --- a/compiled/facebook-www/ReactART-prod.modern.js +++ b/compiled/facebook-www/ReactART-prod.modern.js @@ -9699,7 +9699,7 @@ var slice = Array.prototype.slice, return null; }, bundleType: 0, - version: "18.3.0-www-modern-c4381c45", + version: "18.3.0-www-modern-0ad65545", rendererPackageName: "react-art" }; var internals$jscomp$inline_1304 = { @@ -9730,7 +9730,7 @@ var internals$jscomp$inline_1304 = { scheduleRoot: null, setRefreshHandler: null, getCurrentFiber: null, - reconcilerVersion: "18.3.0-www-modern-c4381c45" + reconcilerVersion: "18.3.0-www-modern-0ad65545" }; if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) { var hook$jscomp$inline_1305 = __REACT_DEVTOOLS_GLOBAL_HOOK__; diff --git a/compiled/facebook-www/ReactDOM-dev.classic.js b/compiled/facebook-www/ReactDOM-dev.classic.js index 4f0cfaa17dc55..ef4bdc24aef7a 100644 --- a/compiled/facebook-www/ReactDOM-dev.classic.js +++ b/compiled/facebook-www/ReactDOM-dev.classic.js @@ -4569,17 +4569,6 @@ function isControlled(props) { * See http://www.w3.org/TR/2012/WD-html5-20121025/the-input-element.html */ -function getHostProps$2(element, props) { - var node = element; - var checked = props.checked; - var hostProps = assign({}, props, { - defaultChecked: undefined, - defaultValue: undefined, - value: undefined, - checked: checked != null ? checked : node._wrapperState.initialChecked - }); - return hostProps; -} function initWrapperState$2(element, props) { { checkControlledValueProps("input", props); @@ -4625,9 +4614,13 @@ function initWrapperState$2(element, props) { var node = element; var defaultValue = props.defaultValue == null ? "" : props.defaultValue; + var initialChecked = + props.checked != null ? props.checked : props.defaultChecked; node._wrapperState = { initialChecked: - props.checked != null ? props.checked : props.defaultChecked, + typeof initialChecked !== "function" && + typeof initialChecked !== "symbol" && + !!initialChecked, initialValue: getToStringValue( props.value != null ? props.value : defaultValue ), @@ -4813,19 +4806,20 @@ function postMountWrapper$3(element, props, isHydrating) { if (name !== "") { node.name = ""; + } // The checked property never gets assigned. It must be manually set. + // We don't want to do this when hydrating so that existing user input isn't + // modified + // 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; } if (disableInputAttributeSyncing) { - // When not syncing the checked attribute, the checked property - // never gets assigned. It must be manually set. We don't want - // to do this when hydrating so that existing user input isn't - // modified - if (!isHydrating) { - updateChecked(element, props); - } // Only assign the checked attribute if it is defined. This saves + // Only assign the checked attribute if it is defined. This saves // a DOM write when controlling the checked attribute isn't needed // (text inputs, submit/reset) - if (props.hasOwnProperty("defaultChecked")) { node.defaultChecked = !node.defaultChecked; node.defaultChecked = !!props.defaultChecked; @@ -5110,11 +5104,6 @@ function updateOptions(node, multiple, propValue, setDefaultSelected) { * selected. */ -function getHostProps$1(element, props) { - return assign({}, props, { - value: undefined - }); -} function initWrapperState$1(element, props) { var node = element; @@ -5199,28 +5188,6 @@ var didWarnValDefaultVal = false; * `defaultValue` if specified, or the children content (deprecated). */ -function getHostProps(element, props) { - var node = element; - - if (props.dangerouslySetInnerHTML != null) { - throw new Error( - "`dangerouslySetInnerHTML` does not make sense on