Skip to content

Commit

Permalink
Refactor ReactDOMComponent to use flatter property operations (#26433)
Browse files Browse the repository at this point in the history
This is in line with the refactor I already did on Fizz earlier and
brings Fiber up to a similar structure.

We end up with a lot of extra checks due the extra abstractions we use
to check the various properties. This uses a flatter and more inline
model which makes it easier to see what each property does. The tradeoff
is that a change might need changes in more places.

The general structure is that there's a switch for tag first, then a
switch for each attribute special case, then a switch for the value. So
it's easy to follow where each scenario will end up and there shouldn't
be any unnecessary code executed along the way.

My goal is to eventually get rid of the meta-programming in DOMProperty
and CSSProperty but I'm leaving that in for now - in line with Fizz.

My next step is moving around things a bit in the diff/commit phases.
This is the first step to more refactors for perf and size, but also
because I'm adding more special cases so I need to have a flatter
structure that I can reason about for those special cases.
  • Loading branch information
sebmarkbage committed Mar 21, 2023
1 parent 0131d0c commit 520f7f3
Show file tree
Hide file tree
Showing 12 changed files with 1,177 additions and 890 deletions.
88 changes: 62 additions & 26 deletions packages/react-dom-bindings/src/client/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@

import {shorthandToLonghand} from './CSSShorthandProperty';

import dangerousStyleValue from './dangerousStyleValue';
import hyphenateStyleName from '../shared/hyphenateStyleName';
import warnValidStyle from '../shared/warnValidStyle';
import {isUnitlessNumber} from '../shared/CSSProperty';
import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion';

/**
* Operations for dealing with CSS properties.
Expand All @@ -29,19 +30,36 @@ export function createDangerousStringForStyles(styles) {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
const styleValue = styles[styleName];
if (styleValue != null) {
const value = styles[styleName];
if (value != null && typeof value !== 'boolean' && value !== '') {
const isCustomProperty = styleName.indexOf('--') === 0;
serialized +=
delimiter +
(isCustomProperty ? styleName : hyphenateStyleName(styleName)) +
':';
serialized += dangerousStyleValue(
styleName,
styleValue,
isCustomProperty,
);

if (isCustomProperty) {
if (__DEV__) {
checkCSSPropertyStringCoercion(value, styleName);
}
serialized += delimiter + styleName + ':' + ('' + value).trim();
} else {
if (
typeof value === 'number' &&
value !== 0 &&
!(
isUnitlessNumber.hasOwnProperty(styleName) &&
isUnitlessNumber[styleName]
)
) {
serialized +=
delimiter + hyphenateStyleName(styleName) + ':' + value + 'px';
} else {
if (__DEV__) {
checkCSSPropertyStringCoercion(value, styleName);
}
serialized +=
delimiter +
hyphenateStyleName(styleName) +
':' +
('' + value).trim();
}
}
delimiter = ';';
}
}
Expand All @@ -58,28 +76,46 @@ export function createDangerousStringForStyles(styles) {
*/
export function setValueForStyles(node, styles) {
const style = node.style;
for (let styleName in styles) {
for (const styleName in styles) {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
const value = styles[styleName];
const isCustomProperty = styleName.indexOf('--') === 0;
if (__DEV__) {
if (!isCustomProperty) {
warnValidStyle(styleName, styles[styleName]);
warnValidStyle(styleName, value);
}
}
const styleValue = dangerousStyleValue(
styleName,
styles[styleName],
isCustomProperty,
);
if (styleName === 'float') {
styleName = 'cssFloat';
}
if (isCustomProperty) {
style.setProperty(styleName, styleValue);

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.hasOwnProperty(styleName) &&
isUnitlessNumber[styleName]
)
) {
style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
} else {
style[styleName] = styleValue;
if (styleName === 'float') {
style.cssFloat = value;
} else {
if (__DEV__) {
checkCSSPropertyStringCoercion(value, styleName);
}
style[styleName] = ('' + value).trim();
}
}
}
}
Expand Down
Loading

0 comments on commit 520f7f3

Please sign in to comment.