@samitouri / QOS-React / commits / b98adb648a

Simplify CSS shorthand property warning (#14183)

I figured out a simpler way to do #14181. It does allocate some but I think that's OK. Time complexity might even be better since we avoid the nested loops the old one had.

Sophie Alpert committed Nov 9, 2018 at 16:56 UTC b98adb648a27640db8467064e537b238b8c306ce
2 files changed +217 -301
packages/react-dom/src/shared/CSSPropertyOperations.js
+34 -55
@@ -5,11 +5,7 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import {
9 - overlappingShorthandsInDev,
10 - longhandToShorthandInDev,
11 - shorthandToLonghandInDev,
12 -} from './CSSShorthandProperty';
8 +import {shorthandToLonghand} from './CSSShorthandProperty';
9
10 import dangerousStyleValue from './dangerousStyleValue';
11 import hyphenateStyleName from './hyphenateStyleName';
@@ -90,6 +86,25 @@ function isValueEmpty(value) {
86 return value == null || typeof value === 'boolean' || value === '';
87 }
88
89 +/**
90 + * Given {color: 'red', overflow: 'hidden'} returns {
91 + * color: 'color',
92 + * overflowX: 'overflow',
93 + * overflowY: 'overflow',
94 + * }. This can be read as "the overflowY property was set by the overflow
95 + * shorthand". That is, the values are the property that each was derived from.
96 + */
97 +function expandShorthandMap(styles) {
98 + const expanded = {};
99 + for (const key in styles) {
100 + const longhands = shorthandToLonghand[key] || [key];
101 + for (let i = 0; i < longhands.length; i++) {
102 + expanded[longhands[i]] = key;
103 + }
104 + }
105 + return expanded;
106 +}
107 +
108 /**
109 * When mixing shorthand and longhand property names, we warn during updates if
110 * we expect an incorrect result to occur. In particular, we warn for:
@@ -112,52 +127,18 @@ export function validateShorthandPropertyCollisionInDev(
127 return;
128 }
129
115 - for (const key in styleUpdates) {
116 - const isEmpty = isValueEmpty(styleUpdates[key]);
117 - if (isEmpty) {
118 - // Property removal; check if we're removing a longhand property
119 - const shorthands = longhandToShorthandInDev[key];
120 - if (shorthands) {
121 - const conflicting = shorthands.filter(
122 - s => !isValueEmpty(nextStyles[s]),
123 - );
124 - if (conflicting.length) {
125 - warning(
126 - false,
127 - 'Removing a style property during rerender (%s) when a ' +
128 - 'conflicting property is set (%s) can lead to styling bugs. To ' +
129 - "avoid this, don't mix shorthand and non-shorthand properties " +
130 - 'for the same value; instead, replace the shorthand with ' +
131 - 'separate values.',
132 - key,
133 - conflicting.join(', '),
134 - );
135 - }
130 + const expandedUpdates = expandShorthandMap(styleUpdates);
131 + const expandedStyles = expandShorthandMap(nextStyles);
132 + const warnedAbout = {};
133 + for (const key in expandedUpdates) {
134 + const originalKey = expandedUpdates[key];
135 + const correctOriginalKey = expandedStyles[key];
136 + if (correctOriginalKey && originalKey !== correctOriginalKey) {
137 + const warningKey = originalKey + ',' + correctOriginalKey;
138 + if (warnedAbout[warningKey]) {
139 + continue;
140 }
137 - }
138 -
139 - // Updating or removing a property; check if it's a shorthand property
140 - const longhands = shorthandToLonghandInDev[key];
141 - const overlapping = overlappingShorthandsInDev[key];
142 - // eslint-disable-next-line no-var
143 - var conflicting = new Set();
144 - if (longhands) {
145 - longhands.forEach(l => {
146 - if (isValueEmpty(styleUpdates[l]) && !isValueEmpty(nextStyles[l])) {
147 - // ex: key = 'font', l = 'fontStyle'
148 - conflicting.add(l);
149 - }
150 - });
151 - }
152 - if (overlapping) {
153 - overlapping.forEach(l => {
154 - if (isValueEmpty(styleUpdates[l]) && !isValueEmpty(nextStyles[l])) {
155 - // ex: key = 'borderLeft', l = 'borderStyle'
156 - conflicting.add(l);
157 - }
158 - });
159 - }
160 - if (conflicting.size) {
141 + warnedAbout[warningKey] = true;
142 warning(
143 false,
144 '%s a style property during rerender (%s) when a ' +
@@ -165,11 +146,9 @@ export function validateShorthandPropertyCollisionInDev(
146 "avoid this, don't mix shorthand and non-shorthand properties " +
147 'for the same value; instead, replace the shorthand with ' +
148 'separate values.',
168 - isEmpty ? 'Removing' : 'Updating',
169 - key,
170 - Array.from(conflicting)
171 - .sort()
172 - .join(', '),
149 + isValueEmpty(styleUpdates[originalKey]) ? 'Removing' : 'Updating',
150 + originalKey,
151 + correctOriginalKey,
152 );
153 }
154 }
packages/react-dom/src/shared/CSSShorthandProperty.js
+183 -246
@@ -7,249 +7,186 @@
7
8 // List derived from Gecko source code:
9 // https://github.com/mozilla/gecko-dev/blob/4e638efc71/layout/style/test/property_database.js
10 -export const shorthandToLonghandInDev = __DEV__
11 - ? {
12 - animation: [
13 - 'animationDelay',
14 - 'animationDirection',
15 - 'animationDuration',
16 - 'animationFillMode',
17 - 'animationIterationCount',
18 - 'animationName',
19 - 'animationPlayState',
20 - 'animationTimingFunction',
21 - ],
22 - background: [
23 - 'backgroundAttachment',
24 - 'backgroundClip',
25 - 'backgroundColor',
26 - 'backgroundImage',
27 - 'backgroundOrigin',
28 - 'backgroundPositionX',
29 - 'backgroundPositionY',
30 - 'backgroundRepeat',
31 - 'backgroundSize',
32 - ],
33 - backgroundPosition: ['backgroundPositionX', 'backgroundPositionY'],
34 - border: [
35 - 'borderBottomColor',
36 - 'borderBottomStyle',
37 - 'borderBottomWidth',
38 - 'borderImageOutset',
39 - 'borderImageRepeat',
40 - 'borderImageSlice',
41 - 'borderImageSource',
42 - 'borderImageWidth',
43 - 'borderLeftColor',
44 - 'borderLeftStyle',
45 - 'borderLeftWidth',
46 - 'borderRightColor',
47 - 'borderRightStyle',
48 - 'borderRightWidth',
49 - 'borderTopColor',
50 - 'borderTopStyle',
51 - 'borderTopWidth',
52 - ],
53 - borderBlockEnd: [
54 - 'borderBlockEndColor',
55 - 'borderBlockEndStyle',
56 - 'borderBlockEndWidth',
57 - ],
58 - borderBlockStart: [
59 - 'borderBlockStartColor',
60 - 'borderBlockStartStyle',
61 - 'borderBlockStartWidth',
62 - ],
63 - borderBottom: [
64 - 'borderBottomColor',
65 - 'borderBottomStyle',
66 - 'borderBottomWidth',
67 - ],
68 - borderColor: [
69 - 'borderBottomColor',
70 - 'borderLeftColor',
71 - 'borderRightColor',
72 - 'borderTopColor',
73 - ],
74 - borderImage: [
75 - 'borderImageOutset',
76 - 'borderImageRepeat',
77 - 'borderImageSlice',
78 - 'borderImageSource',
79 - 'borderImageWidth',
80 - ],
81 - borderInlineEnd: [
82 - 'borderInlineEndColor',
83 - 'borderInlineEndStyle',
84 - 'borderInlineEndWidth',
85 - ],
86 - borderInlineStart: [
87 - 'borderInlineStartColor',
88 - 'borderInlineStartStyle',
89 - 'borderInlineStartWidth',
90 - ],
91 - borderLeft: ['borderLeftColor', 'borderLeftStyle', 'borderLeftWidth'],
92 - borderRadius: [
93 - 'borderBottomLeftRadius',
94 - 'borderBottomRightRadius',
95 - 'borderTopLeftRadius',
96 - 'borderTopRightRadius',
97 - ],
98 - borderRight: ['borderRightColor', 'borderRightStyle', 'borderRightWidth'],
99 - borderStyle: [
100 - 'borderBottomStyle',
101 - 'borderLeftStyle',
102 - 'borderRightStyle',
103 - 'borderTopStyle',
104 - ],
105 - borderTop: ['borderTopColor', 'borderTopStyle', 'borderTopWidth'],
106 - borderWidth: [
107 - 'borderBottomWidth',
108 - 'borderLeftWidth',
109 - 'borderRightWidth',
110 - 'borderTopWidth',
111 - ],
112 - columnRule: ['columnRuleColor', 'columnRuleStyle', 'columnRuleWidth'],
113 - columns: ['columnCount', 'columnWidth'],
114 - flex: ['flexBasis', 'flexGrow', 'flexShrink'],
115 - flexFlow: ['flexDirection', 'flexWrap'],
116 - font: [
117 - 'fontFamily',
118 - 'fontFeatureSettings',
119 - 'fontKerning',
120 - 'fontLanguageOverride',
121 - 'fontSize',
122 - 'fontSizeAdjust',
123 - 'fontStretch',
124 - 'fontStyle',
125 - 'fontVariant',
126 - 'fontVariantAlternates',
127 - 'fontVariantCaps',
128 - 'fontVariantEastAsian',
129 - 'fontVariantLigatures',
130 - 'fontVariantNumeric',
131 - 'fontVariantPosition',
132 - 'fontWeight',
133 - 'lineHeight',
134 - ],
135 - fontVariant: [
136 - 'fontVariantAlternates',
137 - 'fontVariantCaps',
138 - 'fontVariantEastAsian',
139 - 'fontVariantLigatures',
140 - 'fontVariantNumeric',
141 - 'fontVariantPosition',
142 - ],
143 - gap: ['columnGap', 'rowGap'],
144 - grid: [
145 - 'gridAutoColumns',
146 - 'gridAutoFlow',
147 - 'gridAutoRows',
148 - 'gridTemplateAreas',
149 - 'gridTemplateColumns',
150 - 'gridTemplateRows',
151 - ],
152 - gridArea: [
153 - 'gridColumnEnd',
154 - 'gridColumnStart',
155 - 'gridRowEnd',
156 - 'gridRowStart',
157 - ],
158 - gridColumn: ['gridColumnEnd', 'gridColumnStart'],
159 - gridColumnGap: ['columnGap'],
160 - gridGap: ['columnGap', 'rowGap'],
161 - gridRow: ['gridRowEnd', 'gridRowStart'],
162 - gridRowGap: ['rowGap'],
163 - gridTemplate: [
164 - 'gridTemplateAreas',
165 - 'gridTemplateColumns',
166 - 'gridTemplateRows',
167 - ],
168 - listStyle: ['listStyleImage', 'listStylePosition', 'listStyleType'],
169 - margin: ['marginBottom', 'marginLeft', 'marginRight', 'marginTop'],
170 - marker: ['markerEnd', 'markerMid', 'markerStart'],
171 - mask: [
172 - 'maskClip',
173 - 'maskComposite',
174 - 'maskImage',
175 - 'maskMode',
176 - 'maskOrigin',
177 - 'maskPositionX',
178 - 'maskPositionY',
179 - 'maskRepeat',
180 - 'maskSize',
181 - ],
182 - maskPosition: ['maskPositionX', 'maskPositionY'],
183 - outline: ['outlineColor', 'outlineStyle', 'outlineWidth'],
184 - overflow: ['overflowX', 'overflowY'],
185 - padding: ['paddingBottom', 'paddingLeft', 'paddingRight', 'paddingTop'],
186 - placeContent: ['alignContent', 'justifyContent'],
187 - placeItems: ['alignItems', 'justifyItems'],
188 - placeSelf: ['alignSelf', 'justifySelf'],
189 - textDecoration: [
190 - 'textDecorationColor',
191 - 'textDecorationLine',
192 - 'textDecorationStyle',
193 - ],
194 - textEmphasis: ['textEmphasisColor', 'textEmphasisStyle'],
195 - transition: [
196 - 'transitionDelay',
197 - 'transitionDuration',
198 - 'transitionProperty',
199 - 'transitionTimingFunction',
200 - ],
201 - wordWrap: ['overflowWrap'],
202 - }
203 - : {};
204 -
205 -export const longhandToShorthandInDev = {};
206 -
207 -export const overlappingShorthandsInDev = {};
208 -
209 -/**
210 - * obj[key].push(val), handling new keys.
211 - */
212 -function pushToValueArray(obj, key, val) {
213 - let arr = obj[key];
214 - if (!arr) {
215 - obj[key] = arr = [];
216 - }
217 - arr.push(val);
218 -}
219 -
220 -if (__DEV__) {
221 - // eslint-disable-next-line no-var
222 - var shorthand;
223 -
224 - // We want to treat properties like 'backgroundPosition' as one of the
225 - // properties that 'background' expands to, even though that's technically
226 - // incorrect.
227 - for (shorthand in shorthandToLonghandInDev) {
228 - const longhands = shorthandToLonghandInDev[shorthand];
229 - for (const shorthand2 in shorthandToLonghandInDev) {
230 - // eslint-disable-next-line no-var
231 - var longhands2 = shorthandToLonghandInDev[shorthand2];
232 - if (shorthand <= shorthand2) {
233 - continue;
234 - }
235 - if (longhands.every(l => l === shorthand || longhands2.includes(l))) {
236 - // ex: shorthand = 'backgroundPosition', shorthand2 = 'background'
237 - longhands2.push(shorthand);
238 - } else if (
239 - longhands.some(l => l !== shorthand && longhands2.includes(l))
240 - ) {
241 - // ex: shorthand = 'borderLeft', shorthand2 = 'borderStyle'
242 - pushToValueArray(overlappingShorthandsInDev, shorthand, shorthand2);
243 - pushToValueArray(overlappingShorthandsInDev, shorthand2, shorthand);
244 - }
245 - }
246 - }
247 -
248 - // Flip into {maskPositionX: ['mask', 'maskPosition']} for reverse lookups
249 - for (shorthand in shorthandToLonghandInDev) {
250 - const longhands = shorthandToLonghandInDev[shorthand];
251 - longhands.forEach(longhand => {
252 - pushToValueArray(longhandToShorthandInDev, longhand, shorthand);
253 - });
254 - }
255 -}
10 +export const shorthandToLonghand = {
11 + animation: [
12 + 'animationDelay',
13 + 'animationDirection',
14 + 'animationDuration',
15 + 'animationFillMode',
16 + 'animationIterationCount',
17 + 'animationName',
18 + 'animationPlayState',
19 + 'animationTimingFunction',
20 + ],
21 + background: [
22 + 'backgroundAttachment',
23 + 'backgroundClip',
24 + 'backgroundColor',
25 + 'backgroundImage',
26 + 'backgroundOrigin',
27 + 'backgroundPositionX',
28 + 'backgroundPositionY',
29 + 'backgroundRepeat',
30 + 'backgroundSize',
31 + ],
32 + backgroundPosition: ['backgroundPositionX', 'backgroundPositionY'],
33 + border: [
34 + 'borderBottomColor',
35 + 'borderBottomStyle',
36 + 'borderBottomWidth',
37 + 'borderImageOutset',
38 + 'borderImageRepeat',
39 + 'borderImageSlice',
40 + 'borderImageSource',
41 + 'borderImageWidth',
42 + 'borderLeftColor',
43 + 'borderLeftStyle',
44 + 'borderLeftWidth',
45 + 'borderRightColor',
46 + 'borderRightStyle',
47 + 'borderRightWidth',
48 + 'borderTopColor',
49 + 'borderTopStyle',
50 + 'borderTopWidth',
51 + ],
52 + borderBlockEnd: [
53 + 'borderBlockEndColor',
54 + 'borderBlockEndStyle',
55 + 'borderBlockEndWidth',
56 + ],
57 + borderBlockStart: [
58 + 'borderBlockStartColor',
59 + 'borderBlockStartStyle',
60 + 'borderBlockStartWidth',
61 + ],
62 + borderBottom: ['borderBottomColor', 'borderBottomStyle', 'borderBottomWidth'],
63 + borderColor: [
64 + 'borderBottomColor',
65 + 'borderLeftColor',
66 + 'borderRightColor',
67 + 'borderTopColor',
68 + ],
69 + borderImage: [
70 + 'borderImageOutset',
71 + 'borderImageRepeat',
72 + 'borderImageSlice',
73 + 'borderImageSource',
74 + 'borderImageWidth',
75 + ],
76 + borderInlineEnd: [
77 + 'borderInlineEndColor',
78 + 'borderInlineEndStyle',
79 + 'borderInlineEndWidth',
80 + ],
81 + borderInlineStart: [
82 + 'borderInlineStartColor',
83 + 'borderInlineStartStyle',
84 + 'borderInlineStartWidth',
85 + ],
86 + borderLeft: ['borderLeftColor', 'borderLeftStyle', 'borderLeftWidth'],
87 + borderRadius: [
88 + 'borderBottomLeftRadius',
89 + 'borderBottomRightRadius',
90 + 'borderTopLeftRadius',
91 + 'borderTopRightRadius',
92 + ],
93 + borderRight: ['borderRightColor', 'borderRightStyle', 'borderRightWidth'],
94 + borderStyle: [
95 + 'borderBottomStyle',
96 + 'borderLeftStyle',
97 + 'borderRightStyle',
98 + 'borderTopStyle',
99 + ],
100 + borderTop: ['borderTopColor', 'borderTopStyle', 'borderTopWidth'],
101 + borderWidth: [
102 + 'borderBottomWidth',
103 + 'borderLeftWidth',
104 + 'borderRightWidth',
105 + 'borderTopWidth',
106 + ],
107 + columnRule: ['columnRuleColor', 'columnRuleStyle', 'columnRuleWidth'],
108 + columns: ['columnCount', 'columnWidth'],
109 + flex: ['flexBasis', 'flexGrow', 'flexShrink'],
110 + flexFlow: ['flexDirection', 'flexWrap'],
111 + font: [
112 + 'fontFamily',
113 + 'fontFeatureSettings',
114 + 'fontKerning',
115 + 'fontLanguageOverride',
116 + 'fontSize',
117 + 'fontSizeAdjust',
118 + 'fontStretch',
119 + 'fontStyle',
120 + 'fontVariant',
121 + 'fontVariantAlternates',
122 + 'fontVariantCaps',
123 + 'fontVariantEastAsian',
124 + 'fontVariantLigatures',
125 + 'fontVariantNumeric',
126 + 'fontVariantPosition',
127 + 'fontWeight',
128 + 'lineHeight',
129 + ],
130 + fontVariant: [
131 + 'fontVariantAlternates',
132 + 'fontVariantCaps',
133 + 'fontVariantEastAsian',
134 + 'fontVariantLigatures',
135 + 'fontVariantNumeric',
136 + 'fontVariantPosition',
137 + ],
138 + gap: ['columnGap', 'rowGap'],
139 + grid: [
140 + 'gridAutoColumns',
141 + 'gridAutoFlow',
142 + 'gridAutoRows',
143 + 'gridTemplateAreas',
144 + 'gridTemplateColumns',
145 + 'gridTemplateRows',
146 + ],
147 + gridArea: ['gridColumnEnd', 'gridColumnStart', 'gridRowEnd', 'gridRowStart'],
148 + gridColumn: ['gridColumnEnd', 'gridColumnStart'],
149 + gridColumnGap: ['columnGap'],
150 + gridGap: ['columnGap', 'rowGap'],
151 + gridRow: ['gridRowEnd', 'gridRowStart'],
152 + gridRowGap: ['rowGap'],
153 + gridTemplate: [
154 + 'gridTemplateAreas',
155 + 'gridTemplateColumns',
156 + 'gridTemplateRows',
157 + ],
158 + listStyle: ['listStyleImage', 'listStylePosition', 'listStyleType'],
159 + margin: ['marginBottom', 'marginLeft', 'marginRight', 'marginTop'],
160 + marker: ['markerEnd', 'markerMid', 'markerStart'],
161 + mask: [
162 + 'maskClip',
163 + 'maskComposite',
164 + 'maskImage',
165 + 'maskMode',
166 + 'maskOrigin',
167 + 'maskPositionX',
168 + 'maskPositionY',
169 + 'maskRepeat',
170 + 'maskSize',
171 + ],
172 + maskPosition: ['maskPositionX', 'maskPositionY'],
173 + outline: ['outlineColor', 'outlineStyle', 'outlineWidth'],
174 + overflow: ['overflowX', 'overflowY'],
175 + padding: ['paddingBottom', 'paddingLeft', 'paddingRight', 'paddingTop'],
176 + placeContent: ['alignContent', 'justifyContent'],
177 + placeItems: ['alignItems', 'justifyItems'],
178 + placeSelf: ['alignSelf', 'justifySelf'],
179 + textDecoration: [
180 + 'textDecorationColor',
181 + 'textDecorationLine',
182 + 'textDecorationStyle',
183 + ],
184 + textEmphasis: ['textEmphasisColor', 'textEmphasisStyle'],
185 + transition: [
186 + 'transitionDelay',
187 + 'transitionDuration',
188 + 'transitionProperty',
189 + 'transitionTimingFunction',
190 + ],
191 + wordWrap: ['overflowWrap'],
192 +};