Skip to content

Commit 520f7f3

Browse files
authored
Refactor ReactDOMComponent to use flatter property operations (#26433)
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.
1 parent 0131d0c commit 520f7f3

12 files changed

+1177
-890
lines changed

packages/react-dom-bindings/src/client/CSSPropertyOperations.js

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77

88
import {shorthandToLonghand} from './CSSShorthandProperty';
99

10-
import dangerousStyleValue from './dangerousStyleValue';
1110
import hyphenateStyleName from '../shared/hyphenateStyleName';
1211
import warnValidStyle from '../shared/warnValidStyle';
12+
import {isUnitlessNumber} from '../shared/CSSProperty';
13+
import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion';
1314

1415
/**
1516
* Operations for dealing with CSS properties.
@@ -29,19 +30,36 @@ export function createDangerousStringForStyles(styles) {
2930
if (!styles.hasOwnProperty(styleName)) {
3031
continue;
3132
}
32-
const styleValue = styles[styleName];
33-
if (styleValue != null) {
33+
const value = styles[styleName];
34+
if (value != null && typeof value !== 'boolean' && value !== '') {
3435
const isCustomProperty = styleName.indexOf('--') === 0;
35-
serialized +=
36-
delimiter +
37-
(isCustomProperty ? styleName : hyphenateStyleName(styleName)) +
38-
':';
39-
serialized += dangerousStyleValue(
40-
styleName,
41-
styleValue,
42-
isCustomProperty,
43-
);
44-
36+
if (isCustomProperty) {
37+
if (__DEV__) {
38+
checkCSSPropertyStringCoercion(value, styleName);
39+
}
40+
serialized += delimiter + styleName + ':' + ('' + value).trim();
41+
} else {
42+
if (
43+
typeof value === 'number' &&
44+
value !== 0 &&
45+
!(
46+
isUnitlessNumber.hasOwnProperty(styleName) &&
47+
isUnitlessNumber[styleName]
48+
)
49+
) {
50+
serialized +=
51+
delimiter + hyphenateStyleName(styleName) + ':' + value + 'px';
52+
} else {
53+
if (__DEV__) {
54+
checkCSSPropertyStringCoercion(value, styleName);
55+
}
56+
serialized +=
57+
delimiter +
58+
hyphenateStyleName(styleName) +
59+
':' +
60+
('' + value).trim();
61+
}
62+
}
4563
delimiter = ';';
4664
}
4765
}
@@ -58,28 +76,46 @@ export function createDangerousStringForStyles(styles) {
5876
*/
5977
export function setValueForStyles(node, styles) {
6078
const style = node.style;
61-
for (let styleName in styles) {
79+
for (const styleName in styles) {
6280
if (!styles.hasOwnProperty(styleName)) {
6381
continue;
6482
}
83+
const value = styles[styleName];
6584
const isCustomProperty = styleName.indexOf('--') === 0;
6685
if (__DEV__) {
6786
if (!isCustomProperty) {
68-
warnValidStyle(styleName, styles[styleName]);
87+
warnValidStyle(styleName, value);
6988
}
7089
}
71-
const styleValue = dangerousStyleValue(
72-
styleName,
73-
styles[styleName],
74-
isCustomProperty,
75-
);
76-
if (styleName === 'float') {
77-
styleName = 'cssFloat';
78-
}
79-
if (isCustomProperty) {
80-
style.setProperty(styleName, styleValue);
90+
91+
if (value == null || typeof value === 'boolean' || value === '') {
92+
if (isCustomProperty) {
93+
style.setProperty(styleName, '');
94+
} else if (styleName === 'float') {
95+
style.cssFloat = '';
96+
} else {
97+
style[styleName] = '';
98+
}
99+
} else if (isCustomProperty) {
100+
style.setProperty(styleName, value);
101+
} else if (
102+
typeof value === 'number' &&
103+
value !== 0 &&
104+
!(
105+
isUnitlessNumber.hasOwnProperty(styleName) &&
106+
isUnitlessNumber[styleName]
107+
)
108+
) {
109+
style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
81110
} else {
82-
style[styleName] = styleValue;
111+
if (styleName === 'float') {
112+
style.cssFloat = value;
113+
} else {
114+
if (__DEV__) {
115+
checkCSSPropertyStringCoercion(value, styleName);
116+
}
117+
style[styleName] = ('' + value).trim();
118+
}
83119
}
84120
}
85121
}

0 commit comments

Comments
 (0)