From 69e677e3f11bdfefd26502558058c3fe91283385 Mon Sep 17 00:00:00 2001 From: Alan Plum Date: Mon, 12 Oct 2015 17:27:25 +0200 Subject: [PATCH] Fix #1357. Warn when appending "px" to strings. --- .../dom/shared/CSSPropertyOperations.js | 5 ++- src/renderers/dom/shared/ReactDOMComponent.js | 2 +- .../__tests__/ReactDOMComponent-test.js | 40 +++++++++++++++++++ .../dom/shared/dangerousStyleValue.js | 34 +++++++++++++++- 4 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js index 7be7cb9a4e19e..af79e74f8ca37 100644 --- 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 index c7721cea8245d..5a0bd44f6de58 100644 --- 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 index 992a0abe00675..e3a764df9a721 100644 --- 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 ? :
; + }, + }); + var Two = React.createClass({ + render: function() { + return
; + }, + }); + ReactDOM.render(, 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(, div); + expect(console.error.calls.length).toBe(1); + + // Do warn for different components + ReactDOM.render(, 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(, 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 index 71e42467535e7..4d7189c3463c4 100644 --- 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';