Fix #1357. Warn when appending "px" to strings.
Alan Plum committed
Oct 12, 2015 at 17:27 UTC
a299a366ce8380045b10aa76e6066f35f9f1dc1a
4 files changed
+77
-4
src/renderers/dom/shared/CSSPropertyOperations.js
+3
-2
@@ -125,9 +125,10 @@ var CSSPropertyOperations = {
125
* The result should be HTML-escaped before insertion into the DOM.
126
*
127
* @param {object} styles
128
+ * @param {ReactDOMComponent} component
129
* @return {?string}
130
*/
130
- createMarkupForStyles: function(styles) {
131
+ createMarkupForStyles: function(styles, component) {
132
var serialized = '';
133
for (var styleName in styles) {
134
if (!styles.hasOwnProperty(styleName)) {
@@ -139,7 +140,7 @@ var CSSPropertyOperations = {
140
}
141
if (styleValue != null) {
142
serialized += processStyleName(styleName) + ':';
142
- serialized += dangerousStyleValue(styleName, styleValue) + ';';
143
+ serialized += dangerousStyleValue(styleName, styleValue, component) + ';';
144
}
145
}
146
return serialized || null;
src/renderers/dom/shared/ReactDOMComponent.js
+1
-1
@@ -714,7 +714,7 @@ ReactDOMComponent.Mixin = {
714
}
715
propValue = this._previousStyleCopy = assign({}, props.style);
716
}
717
- propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
717
+ propValue = CSSPropertyOperations.createMarkupForStyles(propValue, this);
718
}
719
var markup = null;
720
if (this._tag != null && isCustomComponent(this._tag, props)) {
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+40
@@ -147,6 +147,46 @@ describe('ReactDOMComponent', function() {
147
expect(console.error.argsForCall.length).toBe(2);
148
});
149
150
+ it('should warn about styles with numeric string values for non-unitless properties', function() {
151
+ spyOn(console, 'error');
152
+
153
+ var div = document.createElement('div');
154
+ var One = React.createClass({
155
+ render: function() {
156
+ return this.props.inline ? <span style={{fontSize: '1'}} /> : <div style={{fontSize: '1'}} />;
157
+ },
158
+ });
159
+ var Two = React.createClass({
160
+ render: function() {
161
+ return <div style={{fontSize: '1'}} />;
162
+ },
163
+ });
164
+ ReactDOM.render(<One inline={false} />, div);
165
+ expect(console.error.calls.length).toBe(1);
166
+ expect(console.error.argsForCall[0][0]).toBe(
167
+ 'Warning: a `div` tag (owner: `One`) was passed a numeric string value ' +
168
+ 'for CSS property `fontSize` (value: `1`) which will be treated ' +
169
+ 'as a unitless number in a future version of React.'
170
+ );
171
+
172
+ // Don't warn again for the same component
173
+ ReactDOM.render(<One inline={true} />, div);
174
+ expect(console.error.calls.length).toBe(1);
175
+
176
+ // Do warn for different components
177
+ ReactDOM.render(<Two />, div);
178
+ expect(console.error.calls.length).toBe(2);
179
+ expect(console.error.argsForCall[1][0]).toBe(
180
+ 'Warning: a `div` tag (owner: `Two`) was passed a numeric string value ' +
181
+ 'for CSS property `fontSize` (value: `1`) which will be treated ' +
182
+ 'as a unitless number in a future version of React.'
183
+ );
184
+
185
+ // Really don't warn again for the same component
186
+ ReactDOM.render(<One inline={true} />, div);
187
+ expect(console.error.calls.length).toBe(2);
188
+ });
189
+
190
it('should warn semi-nicely about NaN in style', function() {
191
spyOn(console, 'error');
192
src/renderers/dom/shared/dangerousStyleValue.js
+33
-1
@@ -13,8 +13,10 @@
13
'use strict';
14
15
var CSSProperty = require('CSSProperty');
16
+var warning = require('warning');
17
18
var isUnitlessNumber = CSSProperty.isUnitlessNumber;
19
+var styleWarnings = {};
20
21
/**
22
* Convert a value into the proper css writable value. The style name `name`
@@ -23,9 +25,10 @@ var isUnitlessNumber = CSSProperty.isUnitlessNumber;
25
*
26
* @param {string} name CSS property name such as `topMargin`.
27
* @param {*} value CSS property value such as `10px`.
28
+ * @param {ReactDOMComponent} component
29
* @return {string} Normalized style value with dimensions applied.
30
*/
28
-function dangerousStyleValue(name, value) {
31
+function dangerousStyleValue(name, value, component) {
32
// Note that we've removed escapeTextForBrowser() calls here since the
33
// whole string will be escaped when the attribute is injected into
34
// the markup. If you provide unsafe user data here they can inject
@@ -48,6 +51,35 @@ function dangerousStyleValue(name, value) {
51
}
52
53
if (typeof value === 'string') {
54
+ if (__DEV__) {
55
+ if (component) {
56
+ var owner = component._currentElement._owner;
57
+ var ownerName = owner ? owner.getName() : null;
58
+ if (ownerName && !styleWarnings[ownerName]) {
59
+ styleWarnings[ownerName] = {};
60
+ }
61
+ var warned = false;
62
+ if (ownerName) {
63
+ var warnings = styleWarnings[ownerName];
64
+ warned = warnings[name];
65
+ if (!warned) {
66
+ warnings[name] = true;
67
+ }
68
+ }
69
+ if (!warned) {
70
+ warning(
71
+ false,
72
+ 'a `%s` tag (owner: `%s`) was passed a numeric string value ' +
73
+ 'for CSS property `%s` (value: `%s`) which will be treated ' +
74
+ 'as a unitless number in a future version of React.',
75
+ component._currentElement.type,
76
+ ownerName || 'unknown',
77
+ name,
78
+ value
79
+ );
80
+ }
81
+ }
82
+ }
83
value = value.trim();
84
}
85
return value + 'px';