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
facebook · Jun 14, 2017 · 29eb21d · 29eb21d
1 parent 7dc27d3
commit 29eb21d
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 11 deletions.
diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
@@ -685,15 +685,17 @@ 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
 * should warn when updating hyphenated style names
 * 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

diff --git 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,13 +195,21 @@ var CSSPropertyOperations = {
       if (!styles.hasOwnProperty(styleName)) {
         continue;
       }
+      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) + ':';
-        serialized += dangerousStyleValue(styleName, styleValue, component);
+        serialized += dangerousStyleValue(
+          styleName,
+          styleValue,
+          component,
+          isCustomProperty,
+        );
 
         delimiter = ';';
       }
@@ -227,18 +231,22 @@ 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 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/__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',
@@ -254,7 +262,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 <div style={{'--foo-primary': 'red', backgroundColor: 'red'}} />;
@@ -287,4 +295,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 <div style={{'--foo': 5}} />;
+      }
+    }
+
+    var root = document.createElement('div');
+    ReactDOM.render(<Comp />, 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
@@ -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,6 +42,7 @@ function dangerousStyleValue(name, value, component) {
   }
 
   if (
+    !isCustomProperty &&
     typeof value === 'number' &&
     value !== 0 &&
     !(isUnitlessNumber.hasOwnProperty(name) && isUnitlessNumber[name])