@samitouri / QOS-React / commits / 21d5f7d32d

Wrap shorthand CSS property collision warning in feature flag (#14245)

Disables the recently introduced (#14181) warning for shorthand CSS property collisions by wrapping in a feature flag. Let's hold off shipping this until at least the next minor.

Andrew Clark committed Nov 15, 2018 at 13:36 UTC 21d5f7d32d5becf5c8e986ff0202059be643dc15
11 files changed +162 -135
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
-135
@@ -146,141 +146,6 @@ describe('ReactDOMComponent', () => {
146 }
147 });
148
149 - it('should warn for conflicting CSS shorthand updates', () => {
150 - const container = document.createElement('div');
151 - ReactDOM.render(
152 - <div style={{font: 'foo', fontStyle: 'bar'}} />,
153 - container,
154 - );
155 - expect(() =>
156 - ReactDOM.render(<div style={{font: 'foo'}} />, container),
157 - ).toWarnDev(
158 - 'Warning: Removing a style property during rerender (fontStyle) ' +
159 - 'when a conflicting property is set (font) can lead to styling ' +
160 - "bugs. To avoid this, don't mix shorthand and non-shorthand " +
161 - 'properties for the same value; instead, replace the shorthand ' +
162 - 'with separate values.' +
163 - '\n in div (at **)',
164 - );
165 -
166 - // These updates are OK and don't warn:
167 - ReactDOM.render(
168 - <div style={{font: 'qux', fontStyle: 'bar'}} />,
169 - container,
170 - );
171 - ReactDOM.render(
172 - <div style={{font: 'foo', fontStyle: 'baz'}} />,
173 - container,
174 - );
175 -
176 - expect(() =>
177 - ReactDOM.render(
178 - <div style={{font: 'qux', fontStyle: 'baz'}} />,
179 - container,
180 - ),
181 - ).toWarnDev(
182 - 'Warning: Updating a style property during rerender (font) when ' +
183 - 'a conflicting property is set (fontStyle) can lead to styling ' +
184 - "bugs. To avoid this, don't mix shorthand and non-shorthand " +
185 - 'properties for the same value; instead, replace the shorthand ' +
186 - 'with separate values.' +
187 - '\n in div (at **)',
188 - );
189 - expect(() =>
190 - ReactDOM.render(<div style={{fontStyle: 'baz'}} />, container),
191 - ).toWarnDev(
192 - 'Warning: Removing a style property during rerender (font) when ' +
193 - 'a conflicting property is set (fontStyle) can lead to styling ' +
194 - "bugs. To avoid this, don't mix shorthand and non-shorthand " +
195 - 'properties for the same value; instead, replace the shorthand ' +
196 - 'with separate values.' +
197 - '\n in div (at **)',
198 - );
199 -
200 - // A bit of a special case: backgroundPosition isn't technically longhand
201 - // (it expands to backgroundPosition{X,Y} but so does background)
202 - ReactDOM.render(
203 - <div style={{background: 'yellow', backgroundPosition: 'center'}} />,
204 - container,
205 - );
206 - expect(() =>
207 - ReactDOM.render(<div style={{background: 'yellow'}} />, container),
208 - ).toWarnDev(
209 - 'Warning: Removing a style property during rerender ' +
210 - '(backgroundPosition) when a conflicting property is set ' +
211 - "(background) can lead to styling bugs. To avoid this, don't mix " +
212 - 'shorthand and non-shorthand properties for the same value; ' +
213 - 'instead, replace the shorthand with separate values.' +
214 - '\n in div (at **)',
215 - );
216 - ReactDOM.render(
217 - <div style={{background: 'yellow', backgroundPosition: 'center'}} />,
218 - container,
219 - );
220 - // But setting them at the same time is OK:
221 - ReactDOM.render(
222 - <div style={{background: 'green', backgroundPosition: 'top'}} />,
223 - container,
224 - );
225 - expect(() =>
226 - ReactDOM.render(<div style={{backgroundPosition: 'top'}} />, container),
227 - ).toWarnDev(
228 - 'Warning: Removing a style property during rerender (background) ' +
229 - 'when a conflicting property is set (backgroundPosition) can lead ' +
230 - "to styling bugs. To avoid this, don't mix shorthand and " +
231 - 'non-shorthand properties for the same value; instead, replace the ' +
232 - 'shorthand with separate values.' +
233 - '\n in div (at **)',
234 - );
235 -
236 - // A bit of an even more special case: borderLeft and borderStyle overlap.
237 - ReactDOM.render(
238 - <div style={{borderStyle: 'dotted', borderLeft: '1px solid red'}} />,
239 - container,
240 - );
241 - expect(() =>
242 - ReactDOM.render(
243 - <div style={{borderLeft: '1px solid red'}} />,
244 - container,
245 - ),
246 - ).toWarnDev(
247 - 'Warning: Removing a style property during rerender (borderStyle) ' +
248 - 'when a conflicting property is set (borderLeft) can lead to ' +
249 - "styling bugs. To avoid this, don't mix shorthand and " +
250 - 'non-shorthand properties for the same value; instead, replace the ' +
251 - 'shorthand with separate values.' +
252 - '\n in div (at **)',
253 - );
254 - expect(() =>
255 - ReactDOM.render(
256 - <div style={{borderStyle: 'dashed', borderLeft: '1px solid red'}} />,
257 - container,
258 - ),
259 - ).toWarnDev(
260 - 'Warning: Updating a style property during rerender (borderStyle) ' +
261 - 'when a conflicting property is set (borderLeft) can lead to ' +
262 - "styling bugs. To avoid this, don't mix shorthand and " +
263 - 'non-shorthand properties for the same value; instead, replace the ' +
264 - 'shorthand with separate values.' +
265 - '\n in div (at **)',
266 - );
267 - // But setting them at the same time is OK:
268 - ReactDOM.render(
269 - <div style={{borderStyle: 'dotted', borderLeft: '2px solid red'}} />,
270 - container,
271 - );
272 - expect(() =>
273 - ReactDOM.render(<div style={{borderStyle: 'dotted'}} />, container),
274 - ).toWarnDev(
275 - 'Warning: Removing a style property during rerender (borderLeft) ' +
276 - 'when a conflicting property is set (borderStyle) can lead to ' +
277 - "styling bugs. To avoid this, don't mix shorthand and " +
278 - 'non-shorthand properties for the same value; instead, replace the ' +
279 - 'shorthand with separate values.' +
280 - '\n in div (at **)',
281 - );
282 - });
283 -
149 it('should warn for unknown prop', () => {
150 const container = document.createElement('div');
151 expect(() =>
packages/react-dom/src/__tests__/ReactDOMShorthandCSSPropertyCollision-test.internal.js new
+147
@@ -0,0 +1,147 @@
1 +/**
2 + * Copyright (c) Facebook, Inc. and its affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @emails react-core
8 + */
9 +
10 +'use strict';
11 +
12 +describe('ReactDOMShorthandCSSPropertyCollision', () => {
13 + let ReactFeatureFlags;
14 + let React;
15 + let ReactDOM;
16 +
17 + beforeEach(() => {
18 + jest.resetModules();
19 + ReactFeatureFlags = require('shared/ReactFeatureFlags');
20 + ReactFeatureFlags.warnAboutShorthandPropertyCollision = true;
21 + React = require('react');
22 + ReactDOM = require('react-dom');
23 + });
24 +
25 + it('should warn for conflicting CSS shorthand updates', () => {
26 + const container = document.createElement('div');
27 + ReactDOM.render(<div style={{font: 'foo', fontStyle: 'bar'}} />, container);
28 + expect(() =>
29 + ReactDOM.render(<div style={{font: 'foo'}} />, container),
30 + ).toWarnDev(
31 + 'Warning: Removing a style property during rerender (fontStyle) ' +
32 + 'when a conflicting property is set (font) can lead to styling ' +
33 + "bugs. To avoid this, don't mix shorthand and non-shorthand " +
34 + 'properties for the same value; instead, replace the shorthand ' +
35 + 'with separate values.' +
36 + '\n in div (at **)',
37 + );
38 +
39 + // These updates are OK and don't warn:
40 + ReactDOM.render(<div style={{font: 'qux', fontStyle: 'bar'}} />, container);
41 + ReactDOM.render(<div style={{font: 'foo', fontStyle: 'baz'}} />, container);
42 +
43 + expect(() =>
44 + ReactDOM.render(
45 + <div style={{font: 'qux', fontStyle: 'baz'}} />,
46 + container,
47 + ),
48 + ).toWarnDev(
49 + 'Warning: Updating a style property during rerender (font) when ' +
50 + 'a conflicting property is set (fontStyle) can lead to styling ' +
51 + "bugs. To avoid this, don't mix shorthand and non-shorthand " +
52 + 'properties for the same value; instead, replace the shorthand ' +
53 + 'with separate values.' +
54 + '\n in div (at **)',
55 + );
56 + expect(() =>
57 + ReactDOM.render(<div style={{fontStyle: 'baz'}} />, container),
58 + ).toWarnDev(
59 + 'Warning: Removing a style property during rerender (font) when ' +
60 + 'a conflicting property is set (fontStyle) can lead to styling ' +
61 + "bugs. To avoid this, don't mix shorthand and non-shorthand " +
62 + 'properties for the same value; instead, replace the shorthand ' +
63 + 'with separate values.' +
64 + '\n in div (at **)',
65 + );
66 +
67 + // A bit of a special case: backgroundPosition isn't technically longhand
68 + // (it expands to backgroundPosition{X,Y} but so does background)
69 + ReactDOM.render(
70 + <div style={{background: 'yellow', backgroundPosition: 'center'}} />,
71 + container,
72 + );
73 + expect(() =>
74 + ReactDOM.render(<div style={{background: 'yellow'}} />, container),
75 + ).toWarnDev(
76 + 'Warning: Removing a style property during rerender ' +
77 + '(backgroundPosition) when a conflicting property is set ' +
78 + "(background) can lead to styling bugs. To avoid this, don't mix " +
79 + 'shorthand and non-shorthand properties for the same value; ' +
80 + 'instead, replace the shorthand with separate values.' +
81 + '\n in div (at **)',
82 + );
83 + ReactDOM.render(
84 + <div style={{background: 'yellow', backgroundPosition: 'center'}} />,
85 + container,
86 + );
87 + // But setting them at the same time is OK:
88 + ReactDOM.render(
89 + <div style={{background: 'green', backgroundPosition: 'top'}} />,
90 + container,
91 + );
92 + expect(() =>
93 + ReactDOM.render(<div style={{backgroundPosition: 'top'}} />, container),
94 + ).toWarnDev(
95 + 'Warning: Removing a style property during rerender (background) ' +
96 + 'when a conflicting property is set (backgroundPosition) can lead ' +
97 + "to styling bugs. To avoid this, don't mix shorthand and " +
98 + 'non-shorthand properties for the same value; instead, replace the ' +
99 + 'shorthand with separate values.' +
100 + '\n in div (at **)',
101 + );
102 +
103 + // A bit of an even more special case: borderLeft and borderStyle overlap.
104 + ReactDOM.render(
105 + <div style={{borderStyle: 'dotted', borderLeft: '1px solid red'}} />,
106 + container,
107 + );
108 + expect(() =>
109 + ReactDOM.render(<div style={{borderLeft: '1px solid red'}} />, container),
110 + ).toWarnDev(
111 + 'Warning: Removing a style property during rerender (borderStyle) ' +
112 + 'when a conflicting property is set (borderLeft) can lead to ' +
113 + "styling bugs. To avoid this, don't mix shorthand and " +
114 + 'non-shorthand properties for the same value; instead, replace the ' +
115 + 'shorthand with separate values.' +
116 + '\n in div (at **)',
117 + );
118 + expect(() =>
119 + ReactDOM.render(
120 + <div style={{borderStyle: 'dashed', borderLeft: '1px solid red'}} />,
121 + container,
122 + ),
123 + ).toWarnDev(
124 + 'Warning: Updating a style property during rerender (borderStyle) ' +
125 + 'when a conflicting property is set (borderLeft) can lead to ' +
126 + "styling bugs. To avoid this, don't mix shorthand and " +
127 + 'non-shorthand properties for the same value; instead, replace the ' +
128 + 'shorthand with separate values.' +
129 + '\n in div (at **)',
130 + );
131 + // But setting them at the same time is OK:
132 + ReactDOM.render(
133 + <div style={{borderStyle: 'dotted', borderLeft: '2px solid red'}} />,
134 + container,
135 + );
136 + expect(() =>
137 + ReactDOM.render(<div style={{borderStyle: 'dotted'}} />, container),
138 + ).toWarnDev(
139 + 'Warning: Removing a style property during rerender (borderLeft) ' +
140 + 'when a conflicting property is set (borderStyle) can lead to ' +
141 + "styling bugs. To avoid this, don't mix shorthand and " +
142 + 'non-shorthand properties for the same value; instead, replace the ' +
143 + 'shorthand with separate values.' +
144 + '\n in div (at **)',
145 + );
146 + });
147 +});
packages/react-dom/src/shared/CSSPropertyOperations.js
+6
@@ -12,6 +12,8 @@ import hyphenateStyleName from './hyphenateStyleName';
12 import warnValidStyle from './warnValidStyle';
13 import warning from 'shared/warning';
14
15 +import {warnAboutShorthandPropertyCollision} from 'shared/ReactFeatureFlags';
16 +
17 /**
18 * Operations for dealing with CSS properties.
19 */
@@ -123,6 +125,10 @@ export function validateShorthandPropertyCollisionInDev(
125 styleUpdates,
126 nextStyles,
127 ) {
128 + if (!warnAboutShorthandPropertyCollision) {
129 + return;
130 + }
131 +
132 if (!nextStyles) {
133 return;
134 }
packages/shared/ReactFeatureFlags.js
+2
@@ -47,3 +47,5 @@ export const disableInputAttributeSyncing = false;
47 // These APIs will no longer be "unstable" in the upcoming 16.7 release,
48 // Control this behavior with a flag to support 16.6 minor releases in the meanwhile.
49 export const enableStableConcurrentModeAPIs = false;
50 +
51 +export const warnAboutShorthandPropertyCollision = false;
packages/shared/forks/ReactFeatureFlags.native-fabric-fb.js
+1
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
23 export const enableSuspenseServerRenderer = false;
24 export const disableInputAttributeSyncing = false;
25 export const enableStableConcurrentModeAPIs = false;
26 +export const warnAboutShorthandPropertyCollision = false;
27
28 // Only used in www builds.
29 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-fabric-oss.js
+1
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
23 export const enableSuspenseServerRenderer = false;
24 export const disableInputAttributeSyncing = false;
25 export const enableStableConcurrentModeAPIs = false;
26 +export const warnAboutShorthandPropertyCollision = false;
27
28 // Only used in www builds.
29 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -28,6 +28,7 @@ export const enableProfilerTimer = __PROFILE__;
28 export const enableSchedulerTracing = __PROFILE__;
29 export const enableSuspenseServerRenderer = false;
30 export const enableStableConcurrentModeAPIs = false;
31 +export const warnAboutShorthandPropertyCollision = false;
32
33 // Only used in www builds.
34 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
23 export const enableSuspenseServerRenderer = false;
24 export const disableInputAttributeSyncing = false;
25 export const enableStableConcurrentModeAPIs = false;
26 +export const warnAboutShorthandPropertyCollision = false;
27
28 // Only used in www builds.
29 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.persistent.js
+1
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
23 export const enableSuspenseServerRenderer = false;
24 export const disableInputAttributeSyncing = false;
25 export const enableStableConcurrentModeAPIs = false;
26 +export const warnAboutShorthandPropertyCollision = false;
27
28 // Only used in www builds.
29 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = false;
23 export const enableSuspenseServerRenderer = false;
24 export const disableInputAttributeSyncing = false;
25 export const enableStableConcurrentModeAPIs = false;
26 +export const warnAboutShorthandPropertyCollision = false;
27
28 // Only used in www builds.
29 export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -18,6 +18,7 @@ export const {
18 replayFailedUnitOfWorkWithInvokeGuardedCallback,
19 warnAboutDeprecatedLifecycles,
20 disableInputAttributeSyncing,
21 + warnAboutShorthandPropertyCollision,
22 } = require('ReactFeatureFlags');
23
24 // The rest of the flags are static for better dead code elimination.