From 481793a6dc4d634adaafbe3b488fe07c869eee2b Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Mon, 17 Apr 2023 16:24:32 -0400 Subject: [PATCH] Revert "Refactor some controlled component stuff (#26573)" This reverts commit e5146cb5250be1a4e66511af91549859b36ed488. --- .../src/client/ReactDOMComponent.js | 148 +++++--------- .../src/client/ReactDOMInput.js | 191 +++++++++++------- .../src/client/ReactDOMOption.js | 4 +- .../src/client/ReactDOMSelect.js | 36 ++-- .../src/client/ReactDOMTextarea.js | 86 ++++---- .../src/client/inputValueTracking.js | 1 + .../src/events/plugins/ChangeEventPlugin.js | 21 +- .../__tests__/DOMPropertyOperations-test.js | 6 +- .../src/__tests__/ReactDOMComponent-test.js | 13 +- .../src/__tests__/ReactDOMInput-test.js | 38 +--- .../src/__tests__/ReactDOMTextarea-test.js | 22 -- 11 files changed, 267 insertions(+), 299 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 4e1ecda7857ff..4deb8943a196f 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -7,6 +7,8 @@ * @flow */ +import type {InputWithWrapperState} from './ReactDOMInput'; + import { registrationNameDependencies, possibleRegistrationNames, @@ -15,7 +17,6 @@ import { import {canUseDOM} from 'shared/ExecutionEnvironment'; import {checkHtmlStringCoercion} from 'shared/CheckStringCoercion'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; -import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes'; import { getValueForAttribute, @@ -26,24 +27,27 @@ import { setValueForNamespacedAttribute, } from './DOMPropertyOperations'; import { - validateInputProps, - initInput, - updateInputChecked, - updateInput, - restoreControlledInputState, + initWrapperState as ReactDOMInputInitWrapperState, + postMountWrapper as ReactDOMInputPostMountWrapper, + updateChecked as ReactDOMInputUpdateChecked, + updateWrapper as ReactDOMInputUpdateWrapper, + restoreControlledState as ReactDOMInputRestoreControlledState, } from './ReactDOMInput'; -import {initOption, validateOptionProps} from './ReactDOMOption'; import { - validateSelectProps, - initSelect, - restoreControlledSelectState, - updateSelect, + postMountWrapper as ReactDOMOptionPostMountWrapper, + validateProps as ReactDOMOptionValidateProps, +} from './ReactDOMOption'; +import { + initWrapperState as ReactDOMSelectInitWrapperState, + postMountWrapper as ReactDOMSelectPostMountWrapper, + restoreControlledState as ReactDOMSelectRestoreControlledState, + postUpdateWrapper as ReactDOMSelectPostUpdateWrapper, } from './ReactDOMSelect'; import { - validateTextareaProps, - initTextarea, - updateTextarea, - restoreControlledTextareaState, + initWrapperState as ReactDOMTextareaInitWrapperState, + postMountWrapper as ReactDOMTextareaPostMountWrapper, + updateWrapper as ReactDOMTextareaUpdateWrapper, + restoreControlledState as ReactDOMTextareaRestoreControlledState, } from './ReactDOMTextarea'; import {track} from './inputValueTracking'; import setInnerHTML from './setInnerHTML'; @@ -75,8 +79,6 @@ import { listenToNonDelegatedEvent, } from '../events/DOMPluginEventSystem'; -let didWarnControlledToUncontrolled = false; -let didWarnUncontrolledToControlled = false; let didWarnInvalidHydration = false; let canDiffStyleForHydrationWarning; if (__DEV__) { @@ -803,9 +805,7 @@ export function setInitialProperties( break; } case 'input': { - if (__DEV__) { - checkControlledValueProps('input', props); - } + ReactDOMInputInitWrapperState(domElement, props); // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); @@ -834,10 +834,10 @@ export function setInitialProperties( break; } case 'checked': { + const node = ((domElement: any): InputWithWrapperState); const checked = - propValue != null ? propValue : props.defaultChecked; - const inputElement: HTMLInputElement = (domElement: any); - inputElement.checked = + propValue != null ? propValue : node._wrapperState.initialChecked; + node.checked = !!checked && typeof checked !== 'function' && checked !== 'symbol'; @@ -866,14 +866,11 @@ export function setInitialProperties( // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. track((domElement: any)); - validateInputProps(domElement, props); - initInput(domElement, props, false); + ReactDOMInputPostMountWrapper(domElement, props, false); return; } case 'select': { - if (__DEV__) { - checkControlledValueProps('select', props); - } + ReactDOMSelectInitWrapperState(domElement, props); // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); @@ -896,14 +893,11 @@ export function setInitialProperties( } } } - validateSelectProps(domElement, props); - initSelect(domElement, props); + ReactDOMSelectPostMountWrapper(domElement, props); return; } case 'textarea': { - if (__DEV__) { - checkControlledValueProps('textarea', props); - } + ReactDOMTextareaInitWrapperState(domElement, props); // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); @@ -942,12 +936,11 @@ export function setInitialProperties( // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. track((domElement: any)); - validateTextareaProps(domElement, props); - initTextarea(domElement, props); + ReactDOMTextareaPostMountWrapper(domElement, props); return; } case 'option': { - validateOptionProps(domElement, props); + ReactDOMOptionValidateProps(domElement, props); for (const propKey in props) { if (!props.hasOwnProperty(propKey)) { continue; @@ -970,7 +963,7 @@ export function setInitialProperties( } } } - initOption(domElement, props); + ReactDOMOptionPostMountWrapper(domElement, props); return; } case 'dialog': { @@ -1220,17 +1213,17 @@ export function updateProperties( // In the middle of an update, it is possible to have multiple checked. // When a checked radio tries to change name, browser makes another radio's checked false. if (nextProps.type === 'radio' && nextProps.name != null) { - updateInputChecked(domElement, nextProps); + ReactDOMInputUpdateChecked(domElement, nextProps); } for (let i = 0; i < updatePayload.length; i += 2) { const propKey = updatePayload[i]; const propValue = updatePayload[i + 1]; switch (propKey) { case 'checked': { + const node = ((domElement: any): InputWithWrapperState); const checked = - propValue != null ? propValue : nextProps.defaultChecked; - const inputElement: HTMLInputElement = (domElement: any); - inputElement.checked = + propValue != null ? propValue : node._wrapperState.initialChecked; + node.checked = !!checked && typeof checked !== 'function' && checked !== 'symbol'; @@ -1256,50 +1249,10 @@ export function updateProperties( } } } - - if (__DEV__) { - const wasControlled = - lastProps.type === 'checkbox' || lastProps.type === 'radio' - ? lastProps.checked != null - : lastProps.value != null; - const isControlled = - nextProps.type === 'checkbox' || nextProps.type === 'radio' - ? nextProps.checked != null - : nextProps.value != null; - - if ( - !wasControlled && - isControlled && - !didWarnUncontrolledToControlled - ) { - console.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 ( - wasControlled && - !isControlled && - !didWarnControlledToUncontrolled - ) { - console.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; - } - } // Update the wrapper around inputs *after* updating props. This has to // happen after updating the rest of props. Otherwise HTML5 input validations // raise warnings and prevent the new value from being assigned. - updateInput(domElement, nextProps); + ReactDOMInputUpdateWrapper(domElement, nextProps); return; } case 'select': { @@ -1319,7 +1272,7 @@ export function updateProperties( } // host component that allows setting these optional @@ -39,11 +61,10 @@ let didWarnCheckedDefaultChecked = false; * See http://www.w3.org/TR/2012/WD-html5-20121025/the-input-element.html */ -export function validateInputProps(element: Element, props: Object) { +export function initWrapperState(element: Element, props: Object) { if (__DEV__) { - // 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. + checkControlledValueProps('input', props); + if ( props.checked !== undefined && props.defaultChecked !== undefined && @@ -79,30 +100,98 @@ export function validateInputProps(element: Element, props: Object) { didWarnValueDefaultValue = true; } } + + const node = ((element: any): InputWithWrapperState); + const defaultValue = props.defaultValue == null ? '' : props.defaultValue; + const 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), + }; } -export function updateInputChecked(element: Element, props: Object) { - const node: HTMLInputElement = (element: any); +export function updateChecked(element: Element, props: Object) { + const node = ((element: any): InputWithWrapperState); const checked = props.checked; if (checked != null) { node.checked = checked; } } -export function updateInput(element: Element, props: Object) { - const node: HTMLInputElement = (element: any); +export function updateWrapper(element: Element, props: Object) { + const node = ((element: any): InputWithWrapperState); + if (__DEV__) { + const controlled = isControlled(props); + + if ( + !node._wrapperState.controlled && + controlled && + !didWarnUncontrolledToControlled + ) { + console.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 + ) { + console.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); const value = getToStringValue(props.value); const 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: any) + ) { + node.value = toString((value: any)); + } + } else if (node.value !== toString((value: any))) { + node.value = toString((value: any)); + } + } 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.defaultValue != null) { + if (props.hasOwnProperty('defaultValue')) { setDefaultValue(node, props.type, getToStringValue(props.defaultValue)); - } else { - node.removeAttribute('value'); } } else { // When syncing the value attribute, the value comes from a cascade of @@ -110,12 +199,10 @@ export function updateInput(element: Element, props: Object) { // 1. The value React property // 2. The defaultValue React property // 3. Otherwise there should be no change - if (props.value != null) { + if (props.hasOwnProperty('value')) { setDefaultValue(node, props.type, value); - } else if (props.defaultValue != null) { + } else if (props.hasOwnProperty('defaultValue')) { setDefaultValue(node, props.type, getToStringValue(props.defaultValue)); - } else { - node.removeAttribute('value'); } } @@ -135,39 +222,18 @@ export function updateInput(element: Element, props: Object) { 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: any) - ) { - node.value = toString((value: any)); - } - } else if (node.value !== toString((value: any))) { - node.value = toString((value: any)); - } - } else if (type === 'submit' || type === 'reset') { - // Submit/reset inputs need the attribute removed completely to avoid - // blank-text buttons. - node.removeAttribute('value'); - return; - } } -export function initInput( +export function postMountWrapper( element: Element, props: Object, isHydrating: boolean, ) { - const node: HTMLInputElement = (element: any); + const node = ((element: any): InputWithWrapperState); - if (props.value != null || props.defaultValue != null) { + // Do not assign value if it is already set. This prevents user text input + // from being lost during SSR hydration. + if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) { const type = props.type; const isButton = type === 'submit' || type === 'reset'; @@ -177,14 +243,7 @@ export function initInput( return; } - const defaultValue = - props.defaultValue != null - ? toString(getToStringValue(props.defaultValue)) - : ''; - const initialValue = - props.value != null - ? toString(getToStringValue(props.value)) - : defaultValue; + const initialValue = toString(node._wrapperState.initialValue); // Do not assign value if it is already set. This prevents user text input // from being lost during SSR hydration. @@ -223,8 +282,9 @@ export function initInput( if (disableInputAttributeSyncing) { // When not syncing the value attribute, assign the value attribute // directly from the defaultValue React property (when present) - if (props.defaultValue != null) { - node.defaultValue = defaultValue; + const defaultValue = getToStringValue(props.defaultValue); + if (defaultValue != null) { + node.defaultValue = toString(defaultValue); } } else { // Otherwise, the value attribute is synchronized to the property, @@ -244,27 +304,20 @@ export function initInput( node.name = ''; } - const defaultChecked = - props.checked != null ? props.checked : props.defaultChecked; - const 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 = !!initialChecked; + node.checked = !!node._wrapperState.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.defaultChecked != null) { + if (props.hasOwnProperty('defaultChecked')) { node.defaultChecked = !node.defaultChecked; node.defaultChecked = !!props.defaultChecked; } @@ -276,7 +329,7 @@ export function initInput( // 2. The defaultChecked React property when present // 3. Otherwise, false node.defaultChecked = !node.defaultChecked; - node.defaultChecked = !!initialChecked; + node.defaultChecked = !!node._wrapperState.initialChecked; } if (name !== '') { @@ -284,13 +337,13 @@ export function initInput( } } -export function restoreControlledInputState(element: Element, props: Object) { - const node: HTMLInputElement = (element: any); - updateInput(node, props); +export function restoreControlledState(element: Element, props: Object) { + const node = ((element: any): InputWithWrapperState); + updateWrapper(node, props); updateNamedCousins(node, props); } -function updateNamedCousins(rootNode: HTMLInputElement, props: any) { +function updateNamedCousins(rootNode: InputWithWrapperState, props: any) { const name = props.name; if (props.type === 'radio' && name != null) { let queryRoot: Element = rootNode; @@ -338,7 +391,7 @@ function updateNamedCousins(rootNode: HTMLInputElement, props: any) { // If this is a controlled radio button group, forcing the input that // was previously checked to update will cause it to be come re-checked // as appropriate. - updateInput(otherNode, otherProps); + updateWrapper(otherNode, otherProps); } } } @@ -352,7 +405,7 @@ function updateNamedCousins(rootNode: HTMLInputElement, props: any) { // // https://github.com/facebook/react/issues/7253 export function setDefaultValue( - node: HTMLInputElement, + node: InputWithWrapperState, type: ?string, value: ToStringValue, ) { @@ -361,7 +414,9 @@ export function setDefaultValue( type !== 'number' || getActiveElement(node.ownerDocument) !== node ) { - if (node.defaultValue !== toString(value)) { + if (value == null) { + node.defaultValue = toString(node._wrapperState.initialValue); + } else if (node.defaultValue !== toString(value)) { node.defaultValue = toString(value); } } diff --git a/packages/react-dom-bindings/src/client/ReactDOMOption.js b/packages/react-dom-bindings/src/client/ReactDOMOption.js index 7907609ad88fe..f3b8f12a7f16a 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMOption.js +++ b/packages/react-dom-bindings/src/client/ReactDOMOption.js @@ -18,7 +18,7 @@ let didWarnInvalidInnerHTML = false; * Implements an