Fix #1357. Warn when appending "px" to strings.

facebook · Oct 14, 2015 · a299a36 · a299a36
1 parent 01817c1
commit a299a36
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 4 deletions.
diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js
@@ -125,9 +125,10 @@ var CSSPropertyOperations = {
    * The result should be HTML-escaped before insertion into the DOM.
    *
    * @param {object} styles
+   * @param {ReactDOMComponent} component
    * @return {?string}
    */
-  createMarkupForStyles: function(styles) {
+  createMarkupForStyles: function(styles, component) {
     var serialized = '';
     for (var styleName in styles) {
       if (!styles.hasOwnProperty(styleName)) {
@@ -139,7 +140,7 @@ var CSSPropertyOperations = {
       }
       if (styleValue != null) {
         serialized += processStyleName(styleName) + ':';
-        serialized += dangerousStyleValue(styleName, styleValue) + ';';
+        serialized += dangerousStyleValue(styleName, styleValue, component) + ';';
       }
     }
     return serialized || null;

diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js
@@ -714,7 +714,7 @@ ReactDOMComponent.Mixin = {
             }
             propValue = this._previousStyleCopy = assign({}, props.style);
           }
-          propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
+          propValue = CSSPropertyOperations.createMarkupForStyles(propValue, this);
         }
         var markup = null;
         if (this._tag != null && isCustomComponent(this._tag, props)) {

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -147,6 +147,46 @@ describe('ReactDOMComponent', function() {
       expect(console.error.argsForCall.length).toBe(2);
     });
 
+    it('should warn about styles with numeric string values for non-unitless properties', function() {
+      spyOn(console, 'error');
+
+      var div = document.createElement('div');
+      var One = React.createClass({
+        render: function() {
+          return this.props.inline ? <span style={{fontSize: '1'}} /> : <div style={{fontSize: '1'}} />;
+        },
+      });
+      var Two = React.createClass({
+        render: function() {
+          return <div style={{fontSize: '1'}} />;
+        },
+      });
+      ReactDOM.render(<One inline={false} />, div);
+      expect(console.error.calls.length).toBe(1);
+      expect(console.error.argsForCall[0][0]).toBe(
+        'Warning: a `div` tag (owner: `One`) was passed a numeric string value ' +
+        'for CSS property `fontSize` (value: `1`) which will be treated ' +
+        'as a unitless number in a future version of React.'
+      );
+
+      // Don't warn again for the same component
+      ReactDOM.render(<One inline={true} />, div);
+      expect(console.error.calls.length).toBe(1);
+
+      // Do warn for different components
+      ReactDOM.render(<Two />, div);
+      expect(console.error.calls.length).toBe(2);
+      expect(console.error.argsForCall[1][0]).toBe(
+        'Warning: a `div` tag (owner: `Two`) was passed a numeric string value ' +
+        'for CSS property `fontSize` (value: `1`) which will be treated ' +
+        'as a unitless number in a future version of React.'
+      );
+
+      // Really don't warn again for the same component
+      ReactDOM.render(<One inline={true} />, div);
+      expect(console.error.calls.length).toBe(2);
+    });
+
     it('should warn semi-nicely about NaN in style', function() {
       spyOn(console, 'error');
 

diff --git a/src/renderers/dom/shared/dangerousStyleValue.js b/src/renderers/dom/shared/dangerousStyleValue.js
@@ -13,8 +13,10 @@
 'use strict';
 
 var CSSProperty = require('CSSProperty');
+var warning = require('warning');
 
 var isUnitlessNumber = CSSProperty.isUnitlessNumber;
+var styleWarnings = {};
 
 /**
  * Convert a value into the proper css writable value. The style name `name`
@@ -23,9 +25,10 @@ var isUnitlessNumber = CSSProperty.isUnitlessNumber;
  *
  * @param {string} name CSS property name such as `topMargin`.
  * @param {*} value CSS property value such as `10px`.
+ * @param {ReactDOMComponent} component
  * @return {string} Normalized style value with dimensions applied.
  */
-function dangerousStyleValue(name, value) {
+function dangerousStyleValue(name, value, component) {
   // 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
@@ -48,6 +51,35 @@ function dangerousStyleValue(name, value) {
   }
 
   if (typeof value === 'string') {
+    if (__DEV__) {
+      if (component) {
+        var owner = component._currentElement._owner;
+        var ownerName = owner ? owner.getName() : null;
+        if (ownerName && !styleWarnings[ownerName]) {
+          styleWarnings[ownerName] = {};
+        }
+        var warned = false;
+        if (ownerName) {
+          var warnings = styleWarnings[ownerName];
+          warned = warnings[name];
+          if (!warned) {
+            warnings[name] = true;
+          }
+        }
+        if (!warned) {
+          warning(
+            false,
+            'a `%s` tag (owner: `%s`) was passed a numeric string value ' +
+            'for CSS property `%s` (value: `%s`) which will be treated ' +
+            'as a unitless number in a future version of React.',
+            component._currentElement.type,
+            ownerName || 'unknown',
+            name,
+            value
+          );
+        }
+      }
+    }
     value = value.trim();
   }
   return value + 'px';