Skip to content

Commit

Permalink
Revert "Diff properties in the commit phase instead of generating an …
Browse files Browse the repository at this point in the history
…update payload (#26583)"

This reverts commit ca41adb.
  • Loading branch information
kassens committed Apr 17, 2023
1 parent 29f84bd commit 87fb8ea
Show file tree
Hide file tree
Showing 19 changed files with 130 additions and 675 deletions.
129 changes: 36 additions & 93 deletions packages/react-dom-bindings/src/client/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ 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.
Expand Down Expand Up @@ -65,50 +64,14 @@ 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.
*
* @param {DOMElement} node
* @param {object} styles
*/
export function setValueForStyles(node, styles, prevStyles) {
export function setValueForStyles(node, styles) {
if (styles != null && typeof styles !== 'object') {
throw new Error(
'The `style` prop expects a mapping from style properties to values, ' +
Expand All @@ -125,39 +88,42 @@ export function setValueForStyles(node, styles, prevStyles) {
}

const style = node.style;

if (diffInCommitPhase && prevStyles != null) {
if (__DEV__) {
validateShorthandPropertyCollisionInDev(prevStyles, styles);
for (const styleName in styles) {
if (!styles.hasOwnProperty(styleName)) {
continue;
}

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] = '';
}
const value = styles[styleName];
const isCustomProperty = styleName.indexOf('--') === 0;
if (__DEV__) {
if (!isCustomProperty) {
warnValidStyle(styleName, value);
}
}
for (const styleName in styles) {
const value = styles[styleName];
if (styles.hasOwnProperty(styleName) && prevStyles[styleName] !== value) {
setValueForStyle(style, styleName, value);

if (value == null || typeof value === 'boolean' || value === '') {
if (isCustomProperty) {
style.setProperty(styleName, '');
} else if (styleName === 'float') {
style.cssFloat = '';
} else {
style[styleName] = '';
}
}
} else {
for (const styleName in styles) {
if (styles.hasOwnProperty(styleName)) {
const value = styles[styleName];
setValueForStyle(style, styleName, value);
} 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();
}
}
}
Expand Down Expand Up @@ -201,38 +167,15 @@ function expandShorthandMap(styles) {
* becomes .style.fontVariant = ''
*/
export function validateShorthandPropertyCollisionInDev(
prevStyles,
styleUpdates,
nextStyles,
) {
if (__DEV__) {
if (!nextStyles) {
return;
}

// 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 expandedUpdates = expandShorthandMap(styleUpdates);
const expandedStyles = expandShorthandMap(nextStyles);
const warnedAbout = {};
for (const key in expandedUpdates) {
Expand All @@ -250,7 +193,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(nextStyles[originalKey]) ? 'Removing' : 'Updating',
isValueEmpty(styleUpdates[originalKey]) ? 'Removing' : 'Updating',
originalKey,
correctOriginalKey,
);
Expand Down
Loading

0 comments on commit 87fb8ea

Please sign in to comment.