From 087f5e9489004e3d018c2938dc4ad1ed376534e9 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 6 Apr 2023 21:18:47 -0400 Subject: [PATCH 01/14] Rename controlled component methods These used to be wrapper components which is where the name came from but that's not relevant anymore. --- .../src/client/ReactDOMComponent.js | 73 +++++++++---------- .../src/client/ReactDOMInput.js | 16 ++-- .../src/client/ReactDOMOption.js | 4 +- .../src/client/ReactDOMSelect.js | 8 +- .../src/client/ReactDOMTextarea.js | 13 ++-- 5 files changed, 57 insertions(+), 57 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 4deb8943a196..a3a8d6a33819 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -27,27 +27,24 @@ import { setValueForNamespacedAttribute, } from './DOMPropertyOperations'; import { - initWrapperState as ReactDOMInputInitWrapperState, - postMountWrapper as ReactDOMInputPostMountWrapper, - updateChecked as ReactDOMInputUpdateChecked, - updateWrapper as ReactDOMInputUpdateWrapper, - restoreControlledState as ReactDOMInputRestoreControlledState, + initInput, + postInitInput, + updateInputChecked, + updateInput, + restoreControlledInputState, } from './ReactDOMInput'; +import {postInitOption, validateOptionProps} from './ReactDOMOption'; import { - postMountWrapper as ReactDOMOptionPostMountWrapper, - validateProps as ReactDOMOptionValidateProps, -} from './ReactDOMOption'; -import { - initWrapperState as ReactDOMSelectInitWrapperState, - postMountWrapper as ReactDOMSelectPostMountWrapper, - restoreControlledState as ReactDOMSelectRestoreControlledState, - postUpdateWrapper as ReactDOMSelectPostUpdateWrapper, + initSelect, + postInitSelect, + restoreControlledSelectState, + postUpdateSelect, } from './ReactDOMSelect'; import { - initWrapperState as ReactDOMTextareaInitWrapperState, - postMountWrapper as ReactDOMTextareaPostMountWrapper, - updateWrapper as ReactDOMTextareaUpdateWrapper, - restoreControlledState as ReactDOMTextareaRestoreControlledState, + initTextarea, + postInitTextarea, + updateTextarea, + restoreControlledTextareaState, } from './ReactDOMTextarea'; import {track} from './inputValueTracking'; import setInnerHTML from './setInnerHTML'; @@ -805,7 +802,7 @@ export function setInitialProperties( break; } case 'input': { - ReactDOMInputInitWrapperState(domElement, props); + initInput(domElement, props); // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); @@ -866,11 +863,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)); - ReactDOMInputPostMountWrapper(domElement, props, false); + postInitInput(domElement, props, false); return; } case 'select': { - ReactDOMSelectInitWrapperState(domElement, props); + initSelect(domElement, props); // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); @@ -893,11 +890,11 @@ export function setInitialProperties( } } } - ReactDOMSelectPostMountWrapper(domElement, props); + postInitSelect(domElement, props); return; } case 'textarea': { - ReactDOMTextareaInitWrapperState(domElement, props); + initTextarea(domElement, props); // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); @@ -936,11 +933,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)); - ReactDOMTextareaPostMountWrapper(domElement, props); + postInitTextarea(domElement, props); return; } case 'option': { - ReactDOMOptionValidateProps(domElement, props); + validateOptionProps(domElement, props); for (const propKey in props) { if (!props.hasOwnProperty(propKey)) { continue; @@ -963,7 +960,7 @@ export function setInitialProperties( } } } - ReactDOMOptionPostMountWrapper(domElement, props); + postInitOption(domElement, props); return; } case 'dialog': { @@ -1213,7 +1210,7 @@ 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) { - ReactDOMInputUpdateChecked(domElement, nextProps); + updateInputChecked(domElement, nextProps); } for (let i = 0; i < updatePayload.length; i += 2) { const propKey = updatePayload[i]; @@ -1252,7 +1249,7 @@ export function updateProperties( // 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. - ReactDOMInputUpdateWrapper(domElement, nextProps); + updateInput(domElement, nextProps); return; } case 'select': { @@ -1272,7 +1269,7 @@ export function updateProperties( } // host component that allows setting these optional @@ -113,7 +106,6 @@ export function initInput(element: Element, props: Object) { initialValue: getToStringValue( props.value != null ? props.value : defaultValue, ), - controlled: isControlled(props), }; } @@ -127,38 +119,6 @@ export function updateInputChecked(element: Element, props: Object) { export function updateInput(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; - } - } updateInputChecked(element, props); diff --git a/packages/react-dom-bindings/src/client/inputValueTracking.js b/packages/react-dom-bindings/src/client/inputValueTracking.js index a2ad273a67d1..5e1ca5868781 100644 --- a/packages/react-dom-bindings/src/client/inputValueTracking.js +++ b/packages/react-dom-bindings/src/client/inputValueTracking.js @@ -123,7 +123,6 @@ export function track(node: ElementWithValueTracker) { return; } - // TODO: Once it's just Fiber we can move this to node._wrapperState node._valueTracker = trackValueOnNode(node); } diff --git a/packages/react-dom-bindings/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom-bindings/src/events/plugins/ChangeEventPlugin.js index baa33f1a7c5c..fddc50101351 100644 --- a/packages/react-dom-bindings/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom-bindings/src/events/plugins/ChangeEventPlugin.js @@ -263,16 +263,17 @@ function getTargetInstForInputOrChangeEvent( } } -function handleControlledInputBlur(node: HTMLInputElement) { - const state = (node: any)._wrapperState; - - if (!state || !state.controlled || node.type !== 'number') { +function handleControlledInputBlur(node: HTMLInputElement, props: any) { + if (node.type !== 'number') { return; } if (!disableInputAttributeSyncing) { - // If controlled, assign the value attribute to the current value on blur - setDefaultValue((node: any), 'number', (node: any).value); + const isControlled = props.value != null; + if (isControlled) { + // If controlled, assign the value attribute to the current value on blur + setDefaultValue((node: any), 'number', (node: any).value); + } } } @@ -335,8 +336,12 @@ function extractEvents( } // When blurring, set the value attribute for number inputs - if (domEventName === 'focusout') { - handleControlledInputBlur(((targetNode: any): HTMLInputElement)); + if (domEventName === 'focusout' && targetInst) { + // These props aren't necessarily the most current but we warn for changing + // between controlled and uncontrolled, so it doesn't matter and the previous + // code was also broken for changes. + const props = targetInst.memoizedProps; + handleControlledInputBlur(((targetNode: any): HTMLInputElement), props); } } diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index f373af9335ea..b8c91fb86e18 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -1083,10 +1083,17 @@ describe('ReactDOMComponent', () => { ); expect(nodeValueSetter).toHaveBeenCalledTimes(1); - ReactDOM.render( - , - container, + expect(() => { + ReactDOM.render( + , + container, + ); + }).toErrorDev( + ' 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.', ); + // TODO: Non-null values are updated twice on inputs. This is should ideally be fixed. expect(nodeValueSetter).toHaveBeenCalledTimes(3); From b3e97c59bd092a40560ffa9792a68f12ee7ee97b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 9 Apr 2023 15:41:46 -0400 Subject: [PATCH 09/14] Delete wrapperState We don't use it anymore. --- .../src/client/ReactDOMInput.js | 40 +++---------------- 1 file changed, 6 insertions(+), 34 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMInput.js b/packages/react-dom-bindings/src/client/ReactDOMInput.js index fa32139f7c6c..1416df94ebdc 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMInput.js +++ b/packages/react-dom-bindings/src/client/ReactDOMInput.js @@ -19,20 +19,6 @@ import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import type {ToStringValue} from './ToStringValue'; -export type InputWithWrapperState = HTMLInputElement & { - _wrapperState: { - initialValue: ToStringValue, - initialChecked: ?boolean, - controlled?: boolean, - ... - }, - checked: boolean, - value: string, - defaultChecked: boolean, - defaultValue: string, - ... -}; - let didWarnValueDefaultValue = false; let didWarnCheckedDefaultChecked = false; @@ -93,24 +79,10 @@ export function initInput(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, - ), - }; } export function updateInputChecked(element: Element, props: Object) { - const node = ((element: any): InputWithWrapperState); + const node: HTMLInputElement = (element: any); const checked = props.checked; if (checked != null) { node.checked = checked; @@ -118,7 +90,7 @@ export function updateInputChecked(element: Element, props: Object) { } export function updateInput(element: Element, props: Object) { - const node = ((element: any): InputWithWrapperState); + const node: HTMLInputElement = (element: any); updateInputChecked(element, props); @@ -189,7 +161,7 @@ export function postInitInput( props: Object, isHydrating: boolean, ) { - const node = ((element: any): InputWithWrapperState); + const node: HTMLInputElement = (element: any); if (props.value != null || props.defaultValue != null) { const type = props.type; @@ -309,12 +281,12 @@ export function postInitInput( } export function restoreControlledInputState(element: Element, props: Object) { - const node = ((element: any): InputWithWrapperState); + const node: HTMLInputElement = (element: any); updateInput(node, props); updateNamedCousins(node, props); } -function updateNamedCousins(rootNode: InputWithWrapperState, props: any) { +function updateNamedCousins(rootNode: HTMLInputElement, props: any) { const name = props.name; if (props.type === 'radio' && name != null) { let queryRoot: Element = rootNode; @@ -376,7 +348,7 @@ function updateNamedCousins(rootNode: InputWithWrapperState, props: any) { // // https://github.com/facebook/react/issues/7253 export function setDefaultValue( - node: InputWithWrapperState, + node: HTMLInputElement, type: ?string, value: ToStringValue, ) { From 63b40e53b120c78087dd216a440bae3a1919ee3c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 9 Apr 2023 16:29:56 -0400 Subject: [PATCH 10/14] Textarea only initializes the wrapper state to read it at the same time It can be derived from props. --- .../src/client/ReactDOMTextarea.js | 72 +++++++++---------- 1 file changed, 32 insertions(+), 40 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMTextarea.js b/packages/react-dom-bindings/src/client/ReactDOMTextarea.js index c30bd2d6a851..c76fada72359 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMTextarea.js +++ b/packages/react-dom-bindings/src/client/ReactDOMTextarea.js @@ -11,15 +11,10 @@ import isArray from 'shared/isArray'; import {getCurrentFiberOwnerNameInDevOrNull} from 'react-reconciler/src/ReactCurrentFiber'; import {getToStringValue, toString} from './ToStringValue'; -import type {ToStringValue} from './ToStringValue'; import {disableTextareaChildren} from 'shared/ReactFeatureFlags'; let didWarnValDefaultVal = false; -export type TextAreaWithWrapperState = HTMLTextAreaElement & { - _wrapperState: {initialValue: ToStringValue}, -}; - /** * Implements a