From bdd6ae36a3ae9fe5884214c01c6c1ca11d9757f1 Mon Sep 17 00:00:00 2001 From: sebmarkbage Date: Sun, 9 Apr 2023 22:11:13 +0000 Subject: [PATCH] Refactor some controlled component stuff (#26573) This is mainly renaming some stuff. The behavior change is hasOwnProperty to nullish check. I had a bigger refactor that was a dead-end but might as well land this part and see if I can pick it up later. DiffTrain build for [e5146cb5250be1a4e66511af91549859b36ed488](https://github.com/facebook/react/commit/e5146cb5250be1a4e66511af91549859b36ed488) --- 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 | 519 +++++++++--------- compiled/facebook-www/ReactDOM-dev.modern.js | 510 ++++++++--------- .../facebook-www/ReactDOM-prod.classic.js | 334 ++++++----- compiled/facebook-www/ReactDOM-prod.modern.js | 330 +++++------ .../ReactDOM-profiling.classic.js | 320 +++++------ .../facebook-www/ReactDOM-profiling.modern.js | 316 +++++------ .../ReactDOMTesting-dev.classic.js | 519 +++++++++--------- .../ReactDOMTesting-dev.modern.js | 510 ++++++++--------- .../ReactDOMTesting-prod.classic.js | 334 ++++++----- .../ReactDOMTesting-prod.modern.js | 330 +++++------ .../ReactTestRenderer-dev.modern.js | 2 +- 15 files changed, 1944 insertions(+), 2090 deletions(-) diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION index 6592a88a1ebd7..0e679bc9fa82b 100644 --- a/compiled/facebook-www/REVISION +++ b/compiled/facebook-www/REVISION @@ -1 +1 @@ -85bb7b685b7aec50879703b94dd31523cf69b34d +e5146cb5250be1a4e66511af91549859b36ed488 diff --git a/compiled/facebook-www/React-dev.modern.js b/compiled/facebook-www/React-dev.modern.js index 12b8029b9c57e..2176f7003f457 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-81d6a6e4"; +var ReactVersion = "18.3.0-www-modern-a9e6dcdb"; // 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 333e2911fa73c..1fa2505890c52 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-aefca35b"; +var ReactVersion = "18.3.0-www-classic-6b173260"; 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 bc4a930385da7..cfb15ae9d3ad8 100644 --- a/compiled/facebook-www/ReactART-prod.modern.js +++ b/compiled/facebook-www/ReactART-prod.modern.js @@ -9737,7 +9737,7 @@ var slice = Array.prototype.slice, return null; }, bundleType: 0, - version: "18.3.0-www-modern-50b32236", + version: "18.3.0-www-modern-e257717e", rendererPackageName: "react-art" }; var internals$jscomp$inline_1324 = { @@ -9768,7 +9768,7 @@ var internals$jscomp$inline_1324 = { scheduleRoot: null, setRefreshHandler: null, getCurrentFiber: null, - reconcilerVersion: "18.3.0-www-modern-50b32236" + reconcilerVersion: "18.3.0-www-modern-e257717e" }; if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) { var hook$jscomp$inline_1325 = __REACT_DEVTOOLS_GLOBAL_HOOK__; diff --git a/compiled/facebook-www/ReactDOM-dev.classic.js b/compiled/facebook-www/ReactDOM-dev.classic.js index fec0f79713e29..d2411f0a0cdca 100644 --- a/compiled/facebook-www/ReactDOM-dev.classic.js +++ b/compiled/facebook-www/ReactDOM-dev.classic.js @@ -2904,6 +2904,53 @@ var canUseDOM = !!( typeof window.document.createElement !== "undefined" ); +var hasReadOnlyValue = { + button: true, + checkbox: true, + image: true, + hidden: true, + radio: true, + reset: true, + submit: true +}; +function checkControlledValueProps(tagName, props) { + { + if ( + !( + hasReadOnlyValue[props.type] || + props.onChange || + props.onInput || + props.readOnly || + props.disabled || + props.value == null + ) + ) { + error( + "You provided a `value` prop to a form field without an " + + "`onChange` handler. This will render a read-only field. If " + + "the field should be mutable use `defaultValue`. Otherwise, " + + "set either `onChange` or `readOnly`." + ); + } + + if ( + !( + props.onChange || + props.readOnly || + props.disabled || + props.checked == null + ) + ) { + error( + "You provided a `checked` prop to a form field without an " + + "`onChange` handler. This will render a read-only field. If " + + "the field should be mutable use `defaultChecked`. Otherwise, " + + "set either `onChange` or `readOnly`." + ); + } + } +} + /* eslint-disable max-len */ var ATTRIBUTE_NAME_START_CHAR = @@ -3544,53 +3591,6 @@ function getToStringValue(value) { } } -var hasReadOnlyValue = { - button: true, - checkbox: true, - image: true, - hidden: true, - radio: true, - reset: true, - submit: true -}; -function checkControlledValueProps(tagName, props) { - { - if ( - !( - hasReadOnlyValue[props.type] || - props.onChange || - props.onInput || - props.readOnly || - props.disabled || - props.value == null - ) - ) { - error( - "You provided a `value` prop to a form field without an " + - "`onChange` handler. This will render a read-only field. If " + - "the field should be mutable use `defaultValue`. Otherwise, " + - "set either `onChange` or `readOnly`." - ); - } - - if ( - !( - props.onChange || - props.readOnly || - props.disabled || - props.checked == null - ) - ) { - error( - "You provided a `checked` prop to a form field without an " + - "`onChange` handler. This will render a read-only field. If " + - "the field should be mutable use `defaultChecked`. Otherwise, " + - "set either `onChange` or `readOnly`." - ); - } - } -} - function isCheckable(elem) { var type = elem.type; var nodeName = elem.nodeName; @@ -3698,7 +3698,7 @@ function trackValueOnNode(node) { function track(node) { if (getTracker(node)) { return; - } // TODO: Once it's just Fiber we can move this to node._wrapperState + } node._valueTracker = trackValueOnNode(node); } @@ -3741,13 +3741,6 @@ function getActiveElement(doc) { var didWarnValueDefaultValue$1 = false; var didWarnCheckedDefaultChecked = false; -var didWarnControlledToUncontrolled = false; -var didWarnUncontrolledToControlled = false; - -function isControlled(props) { - var usesChecked = props.type === "checkbox" || props.type === "radio"; - return usesChecked ? props.checked != null : props.value != null; -} /** * Implements an host component that allows setting these optional * props: `checked`, `value`, `defaultChecked`, and `defaultValue`. @@ -3765,10 +3758,11 @@ function isControlled(props) { * See http://www.w3.org/TR/2012/WD-html5-20121025/the-input-element.html */ -function initWrapperState$2(element, props) { +function validateInputProps(element, props) { { - checkControlledValueProps("input", props); - + // Normally we check for undefined and null the same, but explicitly specifying both + // properties, at all is probably worth warning for. We could move this either direction + // and just make it ok to pass null or just check hasOwnProperty. if ( props.checked !== undefined && props.defaultChecked !== undefined && @@ -3807,23 +3801,8 @@ function initWrapperState$2(element, props) { didWarnValueDefaultValue$1 = true; } } - - var node = element; - var defaultValue = props.defaultValue == null ? "" : props.defaultValue; - var initialChecked = - props.checked != null ? props.checked : props.defaultChecked; - node._wrapperState = { - initialChecked: - typeof initialChecked !== "function" && - typeof initialChecked !== "symbol" && - !!initialChecked, - initialValue: getToStringValue( - props.value != null ? props.value : defaultValue - ), - controlled: isControlled(props) - }; } -function updateChecked(element, props) { +function updateInputChecked(element, props) { var node = element; var checked = props.checked; @@ -3831,75 +3810,19 @@ function updateChecked(element, props) { node.checked = checked; } } -function updateWrapper$1(element, props) { +function updateInput(element, props) { var node = element; - - { - var controlled = isControlled(props); - - if ( - !node._wrapperState.controlled && - controlled && - !didWarnUncontrolledToControlled - ) { - error( - "A component is changing an uncontrolled input to be controlled. " + - "This is likely caused by the value changing from undefined to " + - "a defined value, which should not happen. " + - "Decide between using a controlled or uncontrolled input " + - "element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components" - ); - - didWarnUncontrolledToControlled = true; - } - - if ( - node._wrapperState.controlled && - !controlled && - !didWarnControlledToUncontrolled - ) { - error( - "A component is changing a controlled input to be uncontrolled. " + - "This is likely caused by the value changing from a defined to " + - "undefined, which should not happen. " + - "Decide between using a controlled or uncontrolled input " + - "element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components" - ); - - didWarnControlledToUncontrolled = true; - } - } - - updateChecked(element, props); var value = getToStringValue(props.value); var type = props.type; - if (value != null) { - if (type === "number") { - if ( - // $FlowFixMe[incompatible-type] - (value === 0 && node.value === "") || // We explicitly want to coerce to number here if possible. - // eslint-disable-next-line - node.value != value - ) { - node.value = toString(value); - } - } else if (node.value !== toString(value)) { - node.value = toString(value); - } - } else if (type === "submit" || type === "reset") { - // Submit/reset inputs need the attribute removed completely to avoid - // blank-text buttons. - node.removeAttribute("value"); - return; - } - if (disableInputAttributeSyncing) { // When not syncing the value attribute, React only assigns a new value // whenever the defaultValue React prop has changed. When not present, // React does nothing - if (props.hasOwnProperty("defaultValue")) { + if (props.defaultValue != null) { setDefaultValue(node, props.type, getToStringValue(props.defaultValue)); + } else { + node.removeAttribute("value"); } } else { // When syncing the value attribute, the value comes from a cascade of @@ -3907,10 +3830,12 @@ function updateWrapper$1(element, props) { // 1. The value React property // 2. The defaultValue React property // 3. Otherwise there should be no change - if (props.hasOwnProperty("value")) { + if (props.value != null) { setDefaultValue(node, props.type, value); - } else if (props.hasOwnProperty("defaultValue")) { + } else if (props.defaultValue != null) { setDefaultValue(node, props.type, getToStringValue(props.defaultValue)); + } else { + node.removeAttribute("value"); } } @@ -3930,12 +3855,33 @@ function updateWrapper$1(element, props) { node.defaultChecked = !!props.defaultChecked; } } + + updateInputChecked(element, props); + + if (value != null) { + if (type === "number") { + if ( + // $FlowFixMe[incompatible-type] + (value === 0 && node.value === "") || // We explicitly want to coerce to number here if possible. + // eslint-disable-next-line + node.value != value + ) { + node.value = toString(value); + } + } else if (node.value !== toString(value)) { + node.value = toString(value); + } + } else if (type === "submit" || type === "reset") { + // Submit/reset inputs need the attribute removed completely to avoid + // blank-text buttons. + node.removeAttribute("value"); + return; + } } -function postMountWrapper$3(element, props, isHydrating) { - var node = element; // Do not assign value if it is already set. This prevents user text input - // from being lost during SSR hydration. +function initInput(element, props, isHydrating) { + var node = element; - if (props.hasOwnProperty("value") || props.hasOwnProperty("defaultValue")) { + if (props.value != null || props.defaultValue != null) { var type = props.type; var isButton = type === "submit" || type === "reset"; // Avoid setting value attribute on submit/reset inputs as it overrides the // default value provided by the browser. See: #12872 @@ -3944,7 +3890,14 @@ function postMountWrapper$3(element, props, isHydrating) { return; } - var initialValue = toString(node._wrapperState.initialValue); // Do not assign value if it is already set. This prevents user text input + var defaultValue = + props.defaultValue != null + ? toString(getToStringValue(props.defaultValue)) + : ""; + var initialValue = + props.value != null + ? toString(getToStringValue(props.value)) + : defaultValue; // Do not assign value if it is already set. This prevents user text input // from being lost during SSR hydration. if (!isHydrating) { @@ -3981,10 +3934,8 @@ function postMountWrapper$3(element, props, isHydrating) { if (disableInputAttributeSyncing) { // When not syncing the value attribute, assign the value attribute // directly from the defaultValue React property (when present) - var defaultValue = getToStringValue(props.defaultValue); - - if (defaultValue != null) { - node.defaultValue = toString(defaultValue); + if (props.defaultValue != null) { + node.defaultValue = defaultValue; } } else { // Otherwise, the value attribute is synchronized to the property, @@ -4002,21 +3953,28 @@ function postMountWrapper$3(element, props, isHydrating) { if (name !== "") { node.name = ""; - } // The checked property never gets assigned. It must be manually set. + } + + var defaultChecked = + props.checked != null ? props.checked : props.defaultChecked; + var initialChecked = + typeof defaultChecked !== "function" && + typeof defaultChecked !== "symbol" && + !!defaultChecked; // 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; + node.checked = !!initialChecked; } if (disableInputAttributeSyncing) { // 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")) { + if (props.defaultChecked != null) { node.defaultChecked = !node.defaultChecked; node.defaultChecked = !!props.defaultChecked; } @@ -4028,16 +3986,16 @@ function postMountWrapper$3(element, props, isHydrating) { // 2. The defaultChecked React property when present // 3. Otherwise, false node.defaultChecked = !node.defaultChecked; - node.defaultChecked = !!node._wrapperState.initialChecked; + node.defaultChecked = !!initialChecked; } if (name !== "") { node.name = name; } } -function restoreControlledState$3(element, props) { +function restoreControlledInputState(element, props) { var node = element; - updateWrapper$1(node, props); + updateInput(node, props); updateNamedCousins(node, props); } @@ -4089,7 +4047,7 @@ function updateNamedCousins(rootNode, props) { // was previously checked to update will cause it to be come re-checked // as appropriate. - updateWrapper$1(otherNode, otherProps); + updateInput(otherNode, otherProps); } } } // In Chrome, assigning defaultValue to certain input types triggers input validation. @@ -4107,9 +4065,7 @@ function setDefaultValue(node, type, value) { type !== "number" || getActiveElement(node.ownerDocument) !== node ) { - if (value == null) { - node.defaultValue = toString(node._wrapperState.initialValue); - } else if (node.defaultValue !== toString(value)) { + if (node.defaultValue !== toString(value)) { node.defaultValue = toString(value); } } @@ -4122,7 +4078,7 @@ var didWarnInvalidInnerHTML = false; * Implements an