Skip to content

Commit d629b8b

Browse files
sebmarkbagekassens
authored andcommitted
- Refactor some controlled component stuff (#26573) - Don't update textarea defaultValue and input checked unnecessarily (#26580) - Diff properties in the commit phase instead of generating an update payload (#26583) - Move validation of text nesting into ReactDOMComponent (#26594) - Remove initOption special case (#26595) - Use already extracted values instead of reading off props for controlled components (#26596) - Fix input tracking bug (#26627)
1 parent 9d96f30 commit d629b8b

30 files changed

+1457
-512
lines changed

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

Lines changed: 93 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import hyphenateStyleName from '../shared/hyphenateStyleName';
1111
import warnValidStyle from '../shared/warnValidStyle';
1212
import isUnitlessNumber from '../shared/isUnitlessNumber';
1313
import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion';
14+
import {diffInCommitPhase} from 'shared/ReactFeatureFlags';
1415

1516
/**
1617
* Operations for dealing with CSS properties.
@@ -64,14 +65,50 @@ export function createDangerousStringForStyles(styles) {
6465
}
6566
}
6667

68+
function setValueForStyle(style, styleName, value) {
69+
const isCustomProperty = styleName.indexOf('--') === 0;
70+
if (__DEV__) {
71+
if (!isCustomProperty) {
72+
warnValidStyle(styleName, value);
73+
}
74+
}
75+
76+
if (value == null || typeof value === 'boolean' || value === '') {
77+
if (isCustomProperty) {
78+
style.setProperty(styleName, '');
79+
} else if (styleName === 'float') {
80+
style.cssFloat = '';
81+
} else {
82+
style[styleName] = '';
83+
}
84+
} else if (isCustomProperty) {
85+
style.setProperty(styleName, value);
86+
} else if (
87+
typeof value === 'number' &&
88+
value !== 0 &&
89+
!isUnitlessNumber(styleName)
90+
) {
91+
style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
92+
} else {
93+
if (styleName === 'float') {
94+
style.cssFloat = value;
95+
} else {
96+
if (__DEV__) {
97+
checkCSSPropertyStringCoercion(value, styleName);
98+
}
99+
style[styleName] = ('' + value).trim();
100+
}
101+
}
102+
}
103+
67104
/**
68105
* Sets the value for multiple styles on a node. If a value is specified as
69106
* '' (empty string), the corresponding style property will be unset.
70107
*
71108
* @param {DOMElement} node
72109
* @param {object} styles
73110
*/
74-
export function setValueForStyles(node, styles) {
111+
export function setValueForStyles(node, styles, prevStyles) {
75112
if (styles != null && typeof styles !== 'object') {
76113
throw new Error(
77114
'The `style` prop expects a mapping from style properties to values, ' +
@@ -88,42 +125,39 @@ export function setValueForStyles(node, styles) {
88125
}
89126

90127
const style = node.style;
91-
for (const styleName in styles) {
92-
if (!styles.hasOwnProperty(styleName)) {
93-
continue;
94-
}
95-
const value = styles[styleName];
96-
const isCustomProperty = styleName.indexOf('--') === 0;
128+
129+
if (diffInCommitPhase && prevStyles != null) {
97130
if (__DEV__) {
98-
if (!isCustomProperty) {
99-
warnValidStyle(styleName, value);
100-
}
131+
validateShorthandPropertyCollisionInDev(prevStyles, styles);
101132
}
102133

103-
if (value == null || typeof value === 'boolean' || value === '') {
104-
if (isCustomProperty) {
105-
style.setProperty(styleName, '');
106-
} else if (styleName === 'float') {
107-
style.cssFloat = '';
108-
} else {
109-
style[styleName] = '';
110-
}
111-
} else if (isCustomProperty) {
112-
style.setProperty(styleName, value);
113-
} else if (
114-
typeof value === 'number' &&
115-
value !== 0 &&
116-
!isUnitlessNumber(styleName)
117-
) {
118-
style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
119-
} else {
120-
if (styleName === 'float') {
121-
style.cssFloat = value;
122-
} else {
123-
if (__DEV__) {
124-
checkCSSPropertyStringCoercion(value, styleName);
134+
for (const styleName in prevStyles) {
135+
if (
136+
prevStyles.hasOwnProperty(styleName) &&
137+
(styles == null || !styles.hasOwnProperty(styleName))
138+
) {
139+
// Clear style
140+
const isCustomProperty = styleName.indexOf('--') === 0;
141+
if (isCustomProperty) {
142+
style.setProperty(styleName, '');
143+
} else if (styleName === 'float') {
144+
style.cssFloat = '';
145+
} else {
146+
style[styleName] = '';
125147
}
126-
style[styleName] = ('' + value).trim();
148+
}
149+
}
150+
for (const styleName in styles) {
151+
const value = styles[styleName];
152+
if (styles.hasOwnProperty(styleName) && prevStyles[styleName] !== value) {
153+
setValueForStyle(style, styleName, value);
154+
}
155+
}
156+
} else {
157+
for (const styleName in styles) {
158+
if (styles.hasOwnProperty(styleName)) {
159+
const value = styles[styleName];
160+
setValueForStyle(style, styleName, value);
127161
}
128162
}
129163
}
@@ -167,15 +201,38 @@ function expandShorthandMap(styles) {
167201
* becomes .style.fontVariant = ''
168202
*/
169203
export function validateShorthandPropertyCollisionInDev(
170-
styleUpdates,
204+
prevStyles,
171205
nextStyles,
172206
) {
173207
if (__DEV__) {
174208
if (!nextStyles) {
175209
return;
176210
}
177211

178-
const expandedUpdates = expandShorthandMap(styleUpdates);
212+
// Compute the diff as it would happen elsewhere.
213+
const expandedUpdates = {};
214+
if (prevStyles) {
215+
for (const key in prevStyles) {
216+
if (prevStyles.hasOwnProperty(key) && !nextStyles.hasOwnProperty(key)) {
217+
const longhands = shorthandToLonghand[key] || [key];
218+
for (let i = 0; i < longhands.length; i++) {
219+
expandedUpdates[longhands[i]] = key;
220+
}
221+
}
222+
}
223+
}
224+
for (const key in nextStyles) {
225+
if (
226+
nextStyles.hasOwnProperty(key) &&
227+
(!prevStyles || prevStyles[key] !== nextStyles[key])
228+
) {
229+
const longhands = shorthandToLonghand[key] || [key];
230+
for (let i = 0; i < longhands.length; i++) {
231+
expandedUpdates[longhands[i]] = key;
232+
}
233+
}
234+
}
235+
179236
const expandedStyles = expandShorthandMap(nextStyles);
180237
const warnedAbout = {};
181238
for (const key in expandedUpdates) {
@@ -193,7 +250,7 @@ export function validateShorthandPropertyCollisionInDev(
193250
"avoid this, don't mix shorthand and non-shorthand properties " +
194251
'for the same value; instead, replace the shorthand with ' +
195252
'separate values.',
196-
isValueEmpty(styleUpdates[originalKey]) ? 'Removing' : 'Updating',
253+
isValueEmpty(nextStyles[originalKey]) ? 'Removing' : 'Updating',
197254
originalKey,
198255
correctOriginalKey,
199256
);

0 commit comments

Comments
 (0)