diff --git a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js index 9c07945aec..521f4c67fc 100644 --- a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js @@ -11,6 +11,7 @@ import hyphenateStyleName from '../shared/hyphenateStyleName'; import warnValidStyle from '../shared/warnValidStyle'; import isUnitlessNumber from '../shared/isUnitlessNumber'; import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion'; +import {diffInCommitPhase} from 'shared/ReactFeatureFlags'; /** * Operations for dealing with CSS properties. @@ -64,6 +65,42 @@ export function createDangerousStringForStyles(styles) { } } +function setValueForStyle(style, styleName, value) { + const isCustomProperty = styleName.indexOf('--') === 0; + if (__DEV__) { + if (!isCustomProperty) { + warnValidStyle(styleName, value); + } + } + + if (value == null || typeof value === 'boolean' || value === '') { + if (isCustomProperty) { + style.setProperty(styleName, ''); + } else if (styleName === 'float') { + style.cssFloat = ''; + } else { + style[styleName] = ''; + } + } else if (isCustomProperty) { + style.setProperty(styleName, value); + } else if ( + typeof value === 'number' && + value !== 0 && + !isUnitlessNumber(styleName) + ) { + style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers + } else { + if (styleName === 'float') { + style.cssFloat = value; + } else { + if (__DEV__) { + checkCSSPropertyStringCoercion(value, styleName); + } + style[styleName] = ('' + value).trim(); + } + } +} + /** * Sets the value for multiple styles on a node. If a value is specified as * '' (empty string), the corresponding style property will be unset. @@ -71,7 +108,7 @@ export function createDangerousStringForStyles(styles) { * @param {DOMElement} node * @param {object} styles */ -export function setValueForStyles(node, styles) { +export function setValueForStyles(node, styles, prevStyles) { if (styles != null && typeof styles !== 'object') { throw new Error( 'The `style` prop expects a mapping from style properties to values, ' + @@ -88,42 +125,39 @@ export function setValueForStyles(node, styles) { } const style = node.style; - for (const styleName in styles) { - if (!styles.hasOwnProperty(styleName)) { - continue; - } - const value = styles[styleName]; - const isCustomProperty = styleName.indexOf('--') === 0; + + if (diffInCommitPhase && prevStyles != null) { if (__DEV__) { - if (!isCustomProperty) { - warnValidStyle(styleName, value); - } + validateShorthandPropertyCollisionInDev(prevStyles, styles); } - if (value == null || typeof value === 'boolean' || value === '') { - if (isCustomProperty) { - style.setProperty(styleName, ''); - } else if (styleName === 'float') { - style.cssFloat = ''; - } else { - style[styleName] = ''; - } - } else if (isCustomProperty) { - style.setProperty(styleName, value); - } else if ( - typeof value === 'number' && - value !== 0 && - !isUnitlessNumber(styleName) - ) { - style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers - } else { - if (styleName === 'float') { - style.cssFloat = value; - } else { - if (__DEV__) { - checkCSSPropertyStringCoercion(value, styleName); + for (const styleName in prevStyles) { + if ( + prevStyles.hasOwnProperty(styleName) && + (styles == null || !styles.hasOwnProperty(styleName)) + ) { + // Clear style + const isCustomProperty = styleName.indexOf('--') === 0; + if (isCustomProperty) { + style.setProperty(styleName, ''); + } else if (styleName === 'float') { + style.cssFloat = ''; + } else { + style[styleName] = ''; } - style[styleName] = ('' + value).trim(); + } + } + for (const styleName in styles) { + const value = styles[styleName]; + if (styles.hasOwnProperty(styleName) && prevStyles[styleName] !== value) { + setValueForStyle(style, styleName, value); + } + } + } else { + for (const styleName in styles) { + if (styles.hasOwnProperty(styleName)) { + const value = styles[styleName]; + setValueForStyle(style, styleName, value); } } } @@ -167,7 +201,7 @@ function expandShorthandMap(styles) { * becomes .style.fontVariant = '' */ export function validateShorthandPropertyCollisionInDev( - styleUpdates, + prevStyles, nextStyles, ) { if (__DEV__) { @@ -175,7 +209,30 @@ export function validateShorthandPropertyCollisionInDev( return; } - const expandedUpdates = expandShorthandMap(styleUpdates); + // Compute the diff as it would happen elsewhere. + const expandedUpdates = {}; + if (prevStyles) { + for (const key in prevStyles) { + if (prevStyles.hasOwnProperty(key) && !nextStyles.hasOwnProperty(key)) { + const longhands = shorthandToLonghand[key] || [key]; + for (let i = 0; i < longhands.length; i++) { + expandedUpdates[longhands[i]] = key; + } + } + } + } + for (const key in nextStyles) { + if ( + nextStyles.hasOwnProperty(key) && + (!prevStyles || prevStyles[key] !== nextStyles[key]) + ) { + const longhands = shorthandToLonghand[key] || [key]; + for (let i = 0; i < longhands.length; i++) { + expandedUpdates[longhands[i]] = key; + } + } + } + const expandedStyles = expandShorthandMap(nextStyles); const warnedAbout = {}; for (const key in expandedUpdates) { @@ -193,7 +250,7 @@ export function validateShorthandPropertyCollisionInDev( "avoid this, don't mix shorthand and non-shorthand properties " + 'for the same value; instead, replace the shorthand with ' + 'separate values.', - isValueEmpty(styleUpdates[originalKey]) ? 'Removing' : 'Updating', + isValueEmpty(nextStyles[originalKey]) ? 'Removing' : 'Updating', originalKey, correctOriginalKey, ); diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 4e1ecda785..e9398cf076 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -69,6 +69,7 @@ import { disableIEWorkarounds, enableTrustedTypesIntegration, enableFilterEmptyStringAttributesDOM, + diffInCommitPhase, } from 'shared/ReactFeatureFlags'; import { mediaEventTypes, @@ -273,6 +274,7 @@ function setProp( key: string, value: mixed, props: any, + prevValue: mixed, ): void { switch (key) { case 'children': { @@ -314,7 +316,7 @@ function setProp( break; } case 'style': { - setValueForStyles(domElement, value); + setValueForStyles(domElement, value, prevValue); break; } // These attributes accept URLs. These must not allow javascript: URLS. @@ -655,6 +657,21 @@ function setProp( ); break; // Properties that should not be allowed on custom elements. + case 'is': { + if (__DEV__) { + if (prevValue != null) { + console.error( + 'Cannot update the "is" prop after it has been initialized.', + ); + } + } + // TODO: We shouldn't actually set this attribute, because we've already + // passed it to createElement. We don't also need the attribute. + // However, our tests currently query for it so it's plausible someone + // else does too so it's break. + setValueForAttribute(domElement, 'is', value); + break; + } case 'innerText': case 'textContent': if (enableCustomElementPropertySupport) { @@ -689,10 +706,11 @@ function setPropOnCustomElement( key: string, value: mixed, props: any, + prevValue: mixed, ): void { switch (key) { case 'style': { - setValueForStyles(domElement, value); + setValueForStyles(domElement, value, prevValue); break; } case 'dangerouslySetInnerHTML': { @@ -859,7 +877,7 @@ export function setInitialProperties( } // defaultChecked and defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, props); + setProp(domElement, tag, propKey, propValue, props, null); } } } @@ -892,7 +910,7 @@ export function setInitialProperties( } // defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, props); + setProp(domElement, tag, propKey, propValue, props, null); } } } @@ -1047,7 +1065,7 @@ export function setInitialProperties( } // defaultChecked and defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, props); + setProp(domElement, tag, propKey, propValue, props, null); } } } @@ -1063,7 +1081,14 @@ export function setInitialProperties( if (propValue == null) { continue; } - setPropOnCustomElement(domElement, tag, propKey, propValue, props); + setPropOnCustomElement( + domElement, + tag, + propKey, + propValue, + props, + null, + ); } return; } @@ -1078,7 +1103,7 @@ export function setInitialProperties( if (propValue == null) { continue; } - setProp(domElement, tag, propKey, propValue, props); + setProp(domElement, tag, propKey, propValue, props, null); } } @@ -1188,15 +1213,428 @@ export function diffProperties( } if (styleUpdates) { if (__DEV__) { - validateShorthandPropertyCollisionInDev(styleUpdates, nextProps.style); + validateShorthandPropertyCollisionInDev(lastProps.style, nextProps.style); } (updatePayload = updatePayload || []).push('style', styleUpdates); } return updatePayload; } -// Apply the diff. export function updateProperties( + domElement: Element, + tag: string, + lastProps: Object, + nextProps: Object, +): void { + if (__DEV__) { + validatePropertiesInDevelopment(tag, nextProps); + } + + switch (tag) { + case 'div': + case 'span': + case 'svg': + case 'path': + case 'a': + case 'g': + case 'p': + case 'li': { + // Fast track the most common tag types + break; + } + case 'input': { + // Update checked *before* name. + // 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); + } + for (const propKey in lastProps) { + const lastProp = lastProps[propKey]; + if ( + lastProps.hasOwnProperty(propKey) && + lastProp != null && + !nextProps.hasOwnProperty(propKey) + ) { + switch (propKey) { + case 'checked': { + const checked = nextProps.defaultChecked; + const inputElement: HTMLInputElement = (domElement: any); + inputElement.checked = + !!checked && + typeof checked !== 'function' && + checked !== 'symbol'; + break; + } + case 'value': { + // This is handled by updateWrapper below. + break; + } + // defaultChecked and defaultValue are ignored by setProp + default: { + setProp(domElement, tag, propKey, null, nextProps, lastProp); + } + } + } + } + for (const propKey in nextProps) { + const nextProp = nextProps[propKey]; + const lastProp = lastProps[propKey]; + if ( + nextProps.hasOwnProperty(propKey) && + nextProp !== lastProp && + (nextProp != null || lastProp != null) + ) { + switch (propKey) { + case 'checked': { + const checked = + nextProp != null ? nextProp : nextProps.defaultChecked; + const inputElement: HTMLInputElement = (domElement: any); + inputElement.checked = + !!checked && + typeof checked !== 'function' && + checked !== 'symbol'; + break; + } + case 'value': { + // This is handled by updateWrapper below. + break; + } + case 'children': + case 'dangerouslySetInnerHTML': { + if (nextProp != null) { + throw new Error( + `${tag} is a void element tag and must neither have \`children\` nor ` + + 'use `dangerouslySetInnerHTML`.', + ); + } + break; + } + // defaultChecked and defaultValue are ignored by setProp + default: { + setProp(domElement, tag, propKey, nextProp, nextProps, lastProp); + } + } + } + } + + 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); + return; + } + case 'select': { + for (const propKey in lastProps) { + const lastProp = lastProps[propKey]; + if ( + lastProps.hasOwnProperty(propKey) && + lastProp != null && + !nextProps.hasOwnProperty(propKey) + ) { + switch (propKey) { + case 'value': { + // This is handled by updateWrapper below. + break; + } + // defaultValue are ignored by setProp + default: { + setProp(domElement, tag, propKey, null, nextProps, lastProp); + } + } + } + } + for (const propKey in nextProps) { + const nextProp = nextProps[propKey]; + const lastProp = lastProps[propKey]; + if ( + nextProps.hasOwnProperty(propKey) && + nextProp !== lastProp && + (nextProp != null || lastProp != null) + ) { + switch (propKey) { + case 'value': { + // This is handled by updateWrapper below. + break; + } + // defaultValue are ignored by setProp + default: { + setProp(domElement, tag, propKey, nextProp, nextProps, lastProp); + } + } + } + } + //