From 863884568ff6927319d5c9f7b9dad3031ac5ccdd Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 14 Jun 2017 21:28:47 +0300 Subject: [PATCH 1/6] Prevents adding units to css custom properties --- .../__tests__/CSSPropertyOperations-test.js | 15 ++++++++++++++- src/renderers/dom/shared/dangerousStyleValue.js | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js index 0e17f3dacc2e7..c9113d250e587 100644 --- a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js @@ -254,7 +254,7 @@ describe('CSSPropertyOperations', () => { ); }); - it('should not warn when setting CSS variables', () => { + it('should not warn when setting CSS custom properties', () => { class Comp extends React.Component { render() { return
; @@ -287,4 +287,17 @@ describe('CSSPropertyOperations', () => { '\n\nCheck the render method of `Comp`.', ); }); + + it('should not add units to CSS custom properties', () => { + class Comp extends React.Component { + render() { + return
; + } + } + + var root = document.createElement('div'); + ReactDOM.render(, root); + + expect(root.children[0].style.Foo).toEqual('5'); + }); }); diff --git a/src/renderers/dom/shared/dangerousStyleValue.js b/src/renderers/dom/shared/dangerousStyleValue.js index 095be98840315..8649f786d508b 100644 --- a/src/renderers/dom/shared/dangerousStyleValue.js +++ b/src/renderers/dom/shared/dangerousStyleValue.js @@ -42,6 +42,7 @@ function dangerousStyleValue(name, value, component) { } if ( + name.indexOf('--') !== 0 && typeof value === 'number' && value !== 0 && !(isUnitlessNumber.hasOwnProperty(name) && isUnitlessNumber[name]) From 689bfe1d2fad8f0f1f0ba167fa3a5a9f31e58d17 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 14 Jun 2017 22:08:43 +0300 Subject: [PATCH 2/6] Fix code style --- .../dom/shared/__tests__/CSSPropertyOperations-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js index c9113d250e587..7f90819ec7cb8 100644 --- a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js @@ -291,7 +291,7 @@ describe('CSSPropertyOperations', () => { it('should not add units to CSS custom properties', () => { class Comp extends React.Component { render() { - return
; + return
; } } From 22763701d5b5ffa46ec03cbbe8f051495f0c1fbe Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 14 Jun 2017 22:12:52 +0300 Subject: [PATCH 3/6] Optimize custom property checking --- src/renderers/dom/shared/CSSPropertyOperations.js | 4 +++- src/renderers/dom/shared/dangerousStyleValue.js | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js index 073f64e579926..5636639102d58 100644 --- a/src/renderers/dom/shared/CSSPropertyOperations.js +++ b/src/renderers/dom/shared/CSSPropertyOperations.js @@ -230,15 +230,17 @@ var CSSPropertyOperations = { if (__DEV__) { warnValidStyle(styleName, styles[styleName], component); } + var isCustomProperty = styleName.indexOf('--') === 0; var styleValue = dangerousStyleValue( styleName, styles[styleName], component, + isCustomProperty, ); if (styleName === 'float') { styleName = 'cssFloat'; } - if (styleName.indexOf('--') === 0) { + if (isCustomProperty) { style.setProperty(styleName, styleValue); } else if (styleValue) { style[styleName] = styleValue; diff --git a/src/renderers/dom/shared/dangerousStyleValue.js b/src/renderers/dom/shared/dangerousStyleValue.js index 8649f786d508b..194debcb6559b 100644 --- a/src/renderers/dom/shared/dangerousStyleValue.js +++ b/src/renderers/dom/shared/dangerousStyleValue.js @@ -25,7 +25,7 @@ var isUnitlessNumber = CSSProperty.isUnitlessNumber; * @param {ReactDOMComponent} component * @return {string} Normalized style value with dimensions applied. */ -function dangerousStyleValue(name, value, component) { +function dangerousStyleValue(name, value, component, isCustomProperty) { // Note that we've removed escapeTextForBrowser() calls here since the // whole string will be escaped when the attribute is injected into // the markup. If you provide unsafe user data here they can inject @@ -42,7 +42,7 @@ function dangerousStyleValue(name, value, component) { } if ( - name.indexOf('--') !== 0 && + !isCustomProperty && typeof value === 'number' && value !== 0 && !(isUnitlessNumber.hasOwnProperty(name) && isUnitlessNumber[name]) From 1235b3927492c81a29aa0c24f584f266ab38549b Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Wed, 14 Jun 2017 22:57:50 +0300 Subject: [PATCH 4/6] Prevents adding units to css custom properties in markup creation --- src/renderers/dom/shared/CSSPropertyOperations.js | 8 +++++++- .../dom/shared/__tests__/CSSPropertyOperations-test.js | 8 ++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js index 5636639102d58..7f28aae81416a 100644 --- a/src/renderers/dom/shared/CSSPropertyOperations.js +++ b/src/renderers/dom/shared/CSSPropertyOperations.js @@ -199,13 +199,19 @@ var CSSPropertyOperations = { if (!styles.hasOwnProperty(styleName)) { continue; } + var isComputedProperty = styleName.indexOf('--') === 0; var styleValue = styles[styleName]; if (__DEV__) { warnValidStyle(styleName, styleValue, component); } if (styleValue != null) { serialized += delimiter + processStyleName(styleName) + ':'; - serialized += dangerousStyleValue(styleName, styleValue, component); + serialized += dangerousStyleValue( + styleName, + styleValue, + component, + isComputedProperty, + ); delimiter = ';'; } diff --git a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js index 7f90819ec7cb8..065ac059dccbe 100644 --- a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js @@ -101,6 +101,14 @@ describe('CSSPropertyOperations', () => { ).toBe('-ms-transition:none;-moz-transition:none'); }); + it('should create markup with unitless css custom property', () => { + expect( + CSSPropertyOperations.createMarkupForStyles({ + '--foo': 5, + }), + ).toBe('--foo:5'); + }); + it('should set style attribute when styles exist', () => { var styles = { backgroundColor: '#000', From 8b62b32ee8c4c7a67636d43269eb8acbab91ce49 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 14 Jun 2017 22:18:55 +0100 Subject: [PATCH 5/6] Update passing tests --- scripts/fiber/tests-passing.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 4dab926b03298..7a6dceb6c1eef 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -685,6 +685,7 @@ src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should trim values * should not append `px` to styles that might need a number * should create vendor-prefixed markup correctly +* should create markup with unitless css custom property * should set style attribute when styles exist * should not set style attribute when no styles exist * should warn when using hyphenated style names @@ -692,8 +693,9 @@ src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * warns when miscapitalizing vendored style names * should warn about style having a trailing semicolon * should warn about style containing a NaN value -* should not warn when setting CSS variables +* should not warn when setting CSS custom properties * should warn about style containing a Infinity value +* should not add units to CSS custom properties src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js * should create markup for simple properties From 97f8d00186c0c3fdb61467f710faa52a56abc73f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 14 Jun 2017 22:24:22 +0100 Subject: [PATCH 6/6] Fix argument name and reuse check in DEV --- .../dom/shared/CSSPropertyOperations.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js index 7f28aae81416a..a4d953335a0d2 100644 --- a/src/renderers/dom/shared/CSSPropertyOperations.js +++ b/src/renderers/dom/shared/CSSPropertyOperations.js @@ -149,10 +149,6 @@ if (__DEV__) { * @param {ReactDOMComponent} component */ var warnValidStyle = function(name, value, component) { - // Don't warn for CSS variables - if (name.indexOf('--') === 0) { - return; - } var owner; if (component) { owner = component._currentElement._owner; @@ -199,10 +195,12 @@ var CSSPropertyOperations = { if (!styles.hasOwnProperty(styleName)) { continue; } - var isComputedProperty = styleName.indexOf('--') === 0; + var isCustomProperty = styleName.indexOf('--') === 0; var styleValue = styles[styleName]; if (__DEV__) { - warnValidStyle(styleName, styleValue, component); + if (!isCustomProperty) { + warnValidStyle(styleName, styleValue, component); + } } if (styleValue != null) { serialized += delimiter + processStyleName(styleName) + ':'; @@ -210,7 +208,7 @@ var CSSPropertyOperations = { styleName, styleValue, component, - isComputedProperty, + isCustomProperty, ); delimiter = ';'; @@ -233,10 +231,12 @@ var CSSPropertyOperations = { if (!styles.hasOwnProperty(styleName)) { continue; } + var isCustomProperty = styleName.indexOf('--') === 0; if (__DEV__) { - warnValidStyle(styleName, styles[styleName], component); + if (!isCustomProperty) { + warnValidStyle(styleName, styles[styleName], component); + } } - var isCustomProperty = styleName.indexOf('--') === 0; var styleValue = dangerousStyleValue( styleName, styles[styleName],