From 85de6fde515148babd36eae2b7384ad8e62b732a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 28 Mar 2023 22:40:03 -0400 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. --- .../src/client/ReactDOMComponent.js | 1123 ++++++++--------- .../src/client/ReactDOMInput.js | 45 +- .../src/client/ReactDOMSelect.js | 7 - .../src/client/ReactDOMTextarea.js | 31 +- .../__tests__/ChangeEventPlugin-test.js | 36 + 5 files changed, 592 insertions(+), 650 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index ecb08dda7d358..c1f5ed366ae2a 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, @@ -25,7 +27,6 @@ import { } from './DOMPropertyOperations'; import { initWrapperState as ReactDOMInputInitWrapperState, - getHostProps as ReactDOMInputGetHostProps, postMountWrapper as ReactDOMInputPostMountWrapper, updateChecked as ReactDOMInputUpdateChecked, updateWrapper as ReactDOMInputUpdateWrapper, @@ -37,14 +38,12 @@ import { } from './ReactDOMOption'; import { initWrapperState as ReactDOMSelectInitWrapperState, - getHostProps as ReactDOMSelectGetHostProps, postMountWrapper as ReactDOMSelectPostMountWrapper, restoreControlledState as ReactDOMSelectRestoreControlledState, postUpdateWrapper as ReactDOMSelectPostUpdateWrapper, } from './ReactDOMSelect'; import { initWrapperState as ReactDOMTextareaInitWrapperState, - getHostProps as ReactDOMTextareaGetHostProps, postMountWrapper as ReactDOMTextareaPostMountWrapper, updateWrapper as ReactDOMTextareaUpdateWrapper, restoreControlledState as ReactDOMTextareaRestoreControlledState, @@ -287,162 +286,131 @@ export function trapClickOnNonInteractiveElement(node: HTMLElement) { node.onclick = noop; } -function setInitialDOMProperties( - tag: string, +function setProp( domElement: Element, - nextProps: Object, + tag: string, + key: string, + value: mixed, isCustomComponentTag: boolean, + props: any, ): void { - for (const propKey in nextProps) { - if (!nextProps.hasOwnProperty(propKey)) { - continue; + switch (key) { + case 'style': { + if (value != null && typeof value !== 'object') { + throw new Error( + 'The `style` prop expects a mapping from style properties to values, ' + + "not a string. For example, style={{marginRight: spacing + 'em'}} when " + + 'using JSX.', + ); + } + if (__DEV__) { + if (value) { + // Freeze the next style object so that we can assume it won't be + // mutated. We have already warned for this in the past. + Object.freeze(value); + } + } + // Relies on `updateStylesByID` not mutating `styleUpdates`. + setValueForStyles(domElement, value); + break; } - const nextProp = nextProps[propKey]; - switch (propKey) { - case 'style': { - if (nextProp != null && typeof nextProp !== 'object') { + case 'dangerouslySetInnerHTML': { + if (value != null) { + if (typeof value !== 'object' || !('__html' in value)) { throw new Error( - 'The `style` prop expects a mapping from style properties to values, ' + - "not a string. For example, style={{marginRight: spacing + 'em'}} when " + - 'using JSX.', + '`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' + + 'Please visit https://reactjs.org/link/dangerously-set-inner-html ' + + 'for more information.', ); } - if (__DEV__) { - if (nextProp) { - // Freeze the next style object so that we can assume it won't be - // mutated. We have already warned for this in the past. - Object.freeze(nextProp); - } - } - // Relies on `updateStylesByID` not mutating `styleUpdates`. - setValueForStyles(domElement, nextProp); - break; - } - case 'dangerouslySetInnerHTML': { - if (nextProp != null) { - if (typeof nextProp !== 'object' || !('__html' in nextProp)) { + const nextHtml: any = value.__html; + if (nextHtml != null) { + if (props.children != null) { throw new Error( - '`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' + - 'Please visit https://reactjs.org/link/dangerously-set-inner-html ' + - 'for more information.', + 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', ); } - const nextHtml = nextProp.__html; - if (nextHtml != null) { - if (nextProps.children != null) { - throw new Error( - 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', - ); - } - if (disableIEWorkarounds) { - domElement.innerHTML = nextHtml; - } else { - setInnerHTML(domElement, nextHtml); - } + if (disableIEWorkarounds) { + domElement.innerHTML = nextHtml; + } else { + setInnerHTML(domElement, nextHtml); } } - break; } - case 'children': { - if (typeof nextProp === 'string') { - // Avoid setting initial textContent when the text is empty. In IE11 setting - // textContent on a