Skip to content

Commit 9ffc465

Browse files
acdliten8schloss
acdlite
authored and
n8schloss
committed
Wrap shorthand CSS property collision warning in feature flag (facebook#14245)
Disables the recently introduced (facebook#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.
1 parent da0f2a7 commit 9ffc465

11 files changed

+162
-135
lines changed

‎packages/react-dom/src/__tests__/ReactDOMComponent-test.js

Lines changed: 0 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -146,141 +146,6 @@ describe('ReactDOMComponent', () => {
146146
}
147147
});
148148

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-
284149
it('should warn for unknown prop', () => {
285150
const container = document.createElement('div');
286151
expect(() =>

‎packages/react-dom/src/__tests__/ReactDOMShorthandCSSPropertyCollision-test.internal.js

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import hyphenateStyleName from './hyphenateStyleName';
1212
import warnValidStyle from './warnValidStyle';
1313
import warning from 'shared/warning';
1414

15+
import {warnAboutShorthandPropertyCollision} from 'shared/ReactFeatureFlags';
16+
1517
/**
1618
* Operations for dealing with CSS properties.
1719
*/
@@ -123,6 +125,10 @@ export function validateShorthandPropertyCollisionInDev(
123125
styleUpdates,
124126
nextStyles,
125127
) {
128+
if (!warnAboutShorthandPropertyCollision) {
129+
return;
130+
}
131+
126132
if (!nextStyles) {
127133
return;
128134
}

‎packages/shared/ReactFeatureFlags.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,5 @@ export const disableInputAttributeSyncing = false;
4747
// These APIs will no longer be "unstable" in the upcoming 16.7 release,
4848
// Control this behavior with a flag to support 16.6 minor releases in the meanwhile.
4949
export const enableStableConcurrentModeAPIs = false;
50+
51+
export const warnAboutShorthandPropertyCollision = false;

‎packages/shared/forks/ReactFeatureFlags.native-fabric-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
2323
export const enableSuspenseServerRenderer = false;
2424
export const disableInputAttributeSyncing = false;
2525
export const enableStableConcurrentModeAPIs = false;
26+
export const warnAboutShorthandPropertyCollision = false;
2627

2728
// Only used in www builds.
2829
export function addUserTimingListener() {

‎packages/shared/forks/ReactFeatureFlags.native-fabric-oss.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
2323
export const enableSuspenseServerRenderer = false;
2424
export const disableInputAttributeSyncing = false;
2525
export const enableStableConcurrentModeAPIs = false;
26+
export const warnAboutShorthandPropertyCollision = false;
2627

2728
// Only used in www builds.
2829
export function addUserTimingListener() {

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export const enableProfilerTimer = __PROFILE__;
2828
export const enableSchedulerTracing = __PROFILE__;
2929
export const enableSuspenseServerRenderer = false;
3030
export const enableStableConcurrentModeAPIs = false;
31+
export const warnAboutShorthandPropertyCollision = false;
3132

3233
// Only used in www builds.
3334
export function addUserTimingListener() {

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
2323
export const enableSuspenseServerRenderer = false;
2424
export const disableInputAttributeSyncing = false;
2525
export const enableStableConcurrentModeAPIs = false;
26+
export const warnAboutShorthandPropertyCollision = false;
2627

2728
// Only used in www builds.
2829
export function addUserTimingListener() {

‎packages/shared/forks/ReactFeatureFlags.persistent.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = __PROFILE__;
2323
export const enableSuspenseServerRenderer = false;
2424
export const disableInputAttributeSyncing = false;
2525
export const enableStableConcurrentModeAPIs = false;
26+
export const warnAboutShorthandPropertyCollision = false;
2627

2728
// Only used in www builds.
2829
export function addUserTimingListener() {

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export const enableSchedulerTracing = false;
2323
export const enableSuspenseServerRenderer = false;
2424
export const disableInputAttributeSyncing = false;
2525
export const enableStableConcurrentModeAPIs = false;
26+
export const warnAboutShorthandPropertyCollision = false;
2627

2728
// Only used in www builds.
2829
export function addUserTimingListener() {

0 commit comments

Comments
 (0)