@samitouri / QOS-React / commits / 29eb21dd04

Prevents adding units to css custom properties (#9966)

* Prevents adding units to css custom properties * Fix code style * Optimize custom property checking * Prevents adding units to css custom properties in markup creation * Update passing tests * Fix argument name and reuse check in DEV

Bogdan Chadkin committed Jun 15, 2017 at 01:15 UTC 29eb21dd0496cc7312b9c383d414da37750a8dfa
4 files changed +43 -11
scripts/fiber/tests-passing.txt
+3 -1
@@ -685,6 +685,7 @@ src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
685 * should trim values
686 * should not append `px` to styles that might need a number
687 * should create vendor-prefixed markup correctly
688 +* should create markup with unitless css custom property
689 * should set style attribute when styles exist
690 * should not set style attribute when no styles exist
691 * should warn when using hyphenated style names
@@ -692,8 +693,9 @@ src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
693 * warns when miscapitalizing vendored style names
694 * should warn about style having a trailing semicolon
695 * should warn about style containing a NaN value
695 -* should not warn when setting CSS variables
696 +* should not warn when setting CSS custom properties
697 * should warn about style containing a Infinity value
698 +* should not add units to CSS custom properties
699
700 src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js
701 * should create markup for simple properties
src/renderers/dom/shared/CSSPropertyOperations.js
+16 -8
@@ -149,10 +149,6 @@ if (__DEV__) {
149 * @param {ReactDOMComponent} component
150 */
151 var warnValidStyle = function(name, value, component) {
152 - // Don't warn for CSS variables
153 - if (name.indexOf('--') === 0) {
154 - return;
155 - }
152 var owner;
153 if (component) {
154 owner = component._currentElement._owner;
@@ -199,13 +195,21 @@ var CSSPropertyOperations = {
195 if (!styles.hasOwnProperty(styleName)) {
196 continue;
197 }
198 + var isCustomProperty = styleName.indexOf('--') === 0;
199 var styleValue = styles[styleName];
200 if (__DEV__) {
204 - warnValidStyle(styleName, styleValue, component);
201 + if (!isCustomProperty) {
202 + warnValidStyle(styleName, styleValue, component);
203 + }
204 }
205 if (styleValue != null) {
206 serialized += delimiter + processStyleName(styleName) + ':';
208 - serialized += dangerousStyleValue(styleName, styleValue, component);
207 + serialized += dangerousStyleValue(
208 + styleName,
209 + styleValue,
210 + component,
211 + isCustomProperty,
212 + );
213
214 delimiter = ';';
215 }
@@ -227,18 +231,22 @@ var CSSPropertyOperations = {
231 if (!styles.hasOwnProperty(styleName)) {
232 continue;
233 }
234 + var isCustomProperty = styleName.indexOf('--') === 0;
235 if (__DEV__) {
231 - warnValidStyle(styleName, styles[styleName], component);
236 + if (!isCustomProperty) {
237 + warnValidStyle(styleName, styles[styleName], component);
238 + }
239 }
240 var styleValue = dangerousStyleValue(
241 styleName,
242 styles[styleName],
243 component,
244 + isCustomProperty,
245 );
246 if (styleName === 'float') {
247 styleName = 'cssFloat';
248 }
241 - if (styleName.indexOf('--') === 0) {
249 + if (isCustomProperty) {
250 style.setProperty(styleName, styleValue);
251 } else if (styleValue) {
252 style[styleName] = styleValue;
src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
+22 -1
@@ -101,6 +101,14 @@ describe('CSSPropertyOperations', () => {
101 ).toBe('-ms-transition:none;-moz-transition:none');
102 });
103
104 + it('should create markup with unitless css custom property', () => {
105 + expect(
106 + CSSPropertyOperations.createMarkupForStyles({
107 + '--foo': 5,
108 + }),
109 + ).toBe('--foo:5');
110 + });
111 +
112 it('should set style attribute when styles exist', () => {
113 var styles = {
114 backgroundColor: '#000',
@@ -254,7 +262,7 @@ describe('CSSPropertyOperations', () => {
262 );
263 });
264
257 - it('should not warn when setting CSS variables', () => {
265 + it('should not warn when setting CSS custom properties', () => {
266 class Comp extends React.Component {
267 render() {
268 return <div style={{'--foo-primary': 'red', backgroundColor: 'red'}} />;
@@ -287,4 +295,17 @@ describe('CSSPropertyOperations', () => {
295 '\n\nCheck the render method of `Comp`.',
296 );
297 });
298 +
299 + it('should not add units to CSS custom properties', () => {
300 + class Comp extends React.Component {
301 + render() {
302 + return <div style={{'--foo': 5}} />;
303 + }
304 + }
305 +
306 + var root = document.createElement('div');
307 + ReactDOM.render(<Comp />, root);
308 +
309 + expect(root.children[0].style.Foo).toEqual('5');
310 + });
311 });
src/renderers/dom/shared/dangerousStyleValue.js
+2 -1
@@ -25,7 +25,7 @@ var isUnitlessNumber = CSSProperty.isUnitlessNumber;
25 * @param {ReactDOMComponent} component
26 * @return {string} Normalized style value with dimensions applied.
27 */
28 -function dangerousStyleValue(name, value, component) {
28 +function dangerousStyleValue(name, value, component, isCustomProperty) {
29 // Note that we've removed escapeTextForBrowser() calls here since the
30 // whole string will be escaped when the attribute is injected into
31 // the markup. If you provide unsafe user data here they can inject
@@ -42,6 +42,7 @@ function dangerousStyleValue(name, value, component) {
42 }
43
44 if (
45 + !isCustomProperty &&
46 typeof value === 'number' &&
47 value !== 0 &&
48 !(isUnitlessNumber.hasOwnProperty(name) && isUnitlessNumber[name])