@samitouri / QOS-React / commits / f56ca479be

Add component stack to invalid style warnings (#11282)

Dan Abramov committed Oct 19, 2017 at 16:46 UTC f56ca479be859c0fac416179a886962d90506425
6 files changed +75 -66
packages/react-dom/src/ReactDOMFiberComponent.js
+6 -2
@@ -61,7 +61,11 @@ var HTML = '__html';
61
62 var {Namespaces: {html: HTML_NAMESPACE}, getIntrinsicNamespace} = DOMNamespaces;
63
64 +var getStack = emptyFunction;
65 +
66 if (__DEV__) {
67 + getStack = getCurrentFiberStackAddendum;
68 +
69 var warnedUnknownTags = {
70 // Chrome is the only major browser not shipping <time>. But as of July
71 // 2017 it intends to ship it due to widespread usage. We intentionally
@@ -263,7 +267,7 @@ function setInitialDOMProperties(
267 }
268 }
269 // Relies on `updateStylesByID` not mutating `styleUpdates`.
266 - CSSPropertyOperations.setValueForStyles(domElement, nextProp);
270 + CSSPropertyOperations.setValueForStyles(domElement, nextProp, getStack);
271 } else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
272 var nextHtml = nextProp ? nextProp[HTML] : undefined;
273 if (nextHtml != null) {
@@ -319,7 +323,7 @@ function updateDOMProperties(
323 var propKey = updatePayload[i];
324 var propValue = updatePayload[i + 1];
325 if (propKey === STYLE) {
322 - CSSPropertyOperations.setValueForStyles(domElement, propValue);
326 + CSSPropertyOperations.setValueForStyles(domElement, propValue, getStack);
327 } else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
328 setInnerHTML(domElement, propValue);
329 } else if (propKey === CHILDREN) {
packages/react-dom/src/server/ReactPartialRenderer.js
+7 -5
@@ -130,7 +130,7 @@ var processStyleName = memoizeStringOnly(function(styleName) {
130 return hyphenateStyleName(styleName);
131 });
132
133 -function createMarkupForStyles(styles, component) {
133 +function createMarkupForStyles(styles) {
134 var serialized = '';
135 var delimiter = '';
136 for (var styleName in styles) {
@@ -141,7 +141,11 @@ function createMarkupForStyles(styles, component) {
141 var styleValue = styles[styleName];
142 if (__DEV__) {
143 if (!isCustomProperty) {
144 - warnValidStyle(styleName, styleValue, component);
144 + warnValidStyle(
145 + styleName,
146 + styleValue,
147 + () => '', // getCurrentFiberStackAddendum,
148 + );
149 }
150 }
151 if (styleValue != null) {
@@ -263,7 +267,6 @@ function createOpenTagMarkup(
267 namespace,
268 makeStaticMarkup,
269 isRootElement,
266 - instForDebug,
270 ) {
271 var ret = '<' + tagVerbatim;
272
@@ -276,7 +279,7 @@ function createOpenTagMarkup(
279 continue;
280 }
281 if (propKey === STYLE) {
279 - propValue = createMarkupForStyles(propValue, instForDebug);
282 + propValue = createMarkupForStyles(propValue);
283 }
284 var markup = null;
285 if (isCustomComponent(tagLowercase, props)) {
@@ -791,7 +794,6 @@ class ReactDOMServerRenderer {
794 namespace,
795 this.makeStaticMarkup,
796 this.stack.length === 1,
794 - null,
797 );
798 var footer = '';
799 if (omittedCloseTags.hasOwnProperty(tag)) {
packages/react-dom/src/shared/CSSPropertyOperations.js
+2 -3
@@ -57,9 +57,8 @@ var CSSPropertyOperations = {
57 *
58 * @param {DOMElement} node
59 * @param {object} styles
60 - * @param {ReactDOMComponent} component
60 */
62 - setValueForStyles: function(node, styles, component) {
61 + setValueForStyles: function(node, styles, getStack) {
62 var style = node.style;
63 for (var styleName in styles) {
64 if (!styles.hasOwnProperty(styleName)) {
@@ -68,7 +67,7 @@ var CSSPropertyOperations = {
67 var isCustomProperty = styleName.indexOf('--') === 0;
68 if (__DEV__) {
69 if (!isCustomProperty) {
71 - warnValidStyle(styleName, styles[styleName], component);
70 + warnValidStyle(styleName, styles[styleName], getStack);
71 }
72 }
73 var styleValue = dangerousStyleValue(
packages/react-dom/src/shared/__tests__/CSSPropertyOperations-test.js
+37 -23
@@ -13,6 +13,10 @@ var React = require('react');
13 var ReactDOM = require('react-dom');
14 var ReactDOMServer = require('react-dom/server');
15
16 +function normalizeCodeLocInfo(str) {
17 + return str && str.replace(/at .+?:\d+/g, 'at **');
18 +}
19 +
20 describe('CSSPropertyOperations', () => {
21 it('should automatically append `px` to relevant styles', () => {
22 var styles = {
@@ -91,9 +95,10 @@ describe('CSSPropertyOperations', () => {
95 var root = document.createElement('div');
96 ReactDOM.render(<Comp />, root);
97 expectDev(console.error.calls.count()).toBe(1);
94 - expectDev(console.error.calls.argsFor(0)[0]).toEqual(
98 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
99 'Warning: Unsupported style property background-color. Did you mean backgroundColor?' +
96 - '\n\nCheck the render method of `Comp`.',
100 + '\n in div (at **)' +
101 + '\n in Comp (at **)',
102 );
103 });
104
@@ -116,13 +121,15 @@ describe('CSSPropertyOperations', () => {
121 ReactDOM.render(<Comp style={styles} />, root);
122
123 expectDev(console.error.calls.count()).toBe(2);
119 - expectDev(console.error.calls.argsFor(0)[0]).toEqual(
124 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
125 'Warning: Unsupported style property -ms-transform. Did you mean msTransform?' +
121 - '\n\nCheck the render method of `Comp`.',
126 + '\n in div (at **)' +
127 + '\n in Comp (at **)',
128 );
123 - expectDev(console.error.calls.argsFor(1)[0]).toEqual(
129 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
130 'Warning: Unsupported style property -webkit-transform. Did you mean WebkitTransform?' +
125 - '\n\nCheck the render method of `Comp`.',
131 + '\n in div (at **)' +
132 + '\n in Comp (at **)',
133 );
134 });
135
@@ -148,13 +155,17 @@ describe('CSSPropertyOperations', () => {
155 ReactDOM.render(<Comp />, root);
156 // msTransform is correct already and shouldn't warn
157 expectDev(console.error.calls.count()).toBe(2);
151 - expectDev(console.error.calls.argsFor(0)[0]).toEqual(
158 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
159 'Warning: Unsupported vendor-prefixed style property oTransform. ' +
153 - 'Did you mean OTransform?\n\nCheck the render method of `Comp`.',
160 + 'Did you mean OTransform?' +
161 + '\n in div (at **)' +
162 + '\n in Comp (at **)',
163 );
155 - expectDev(console.error.calls.argsFor(1)[0]).toEqual(
164 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
165 'Warning: Unsupported vendor-prefixed style property webkitTransform. ' +
157 - 'Did you mean WebkitTransform?\n\nCheck the render method of `Comp`.',
166 + 'Did you mean WebkitTransform?' +
167 + '\n in div (at **)' +
168 + '\n in Comp (at **)',
169 );
170 });
171
@@ -180,13 +191,17 @@ describe('CSSPropertyOperations', () => {
191 var root = document.createElement('div');
192 ReactDOM.render(<Comp />, root);
193 expectDev(console.error.calls.count()).toBe(2);
183 - expectDev(console.error.calls.argsFor(0)[0]).toEqual(
184 - "Warning: Style property values shouldn't contain a semicolon." +
185 - '\n\nCheck the render method of `Comp`. Try "backgroundColor: blue" instead.',
194 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
195 + "Warning: Style property values shouldn't contain a semicolon. " +
196 + 'Try "backgroundColor: blue" instead.' +
197 + '\n in div (at **)' +
198 + '\n in Comp (at **)',
199 );
187 - expectDev(console.error.calls.argsFor(1)[0]).toEqual(
188 - "Warning: Style property values shouldn't contain a semicolon." +
189 - '\n\nCheck the render method of `Comp`. Try "color: red" instead.',
200 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
201 + "Warning: Style property values shouldn't contain a semicolon. " +
202 + 'Try "color: red" instead.' +
203 + '\n in div (at **)' +
204 + '\n in Comp (at **)',
205 );
206 });
207
@@ -204,9 +219,10 @@ describe('CSSPropertyOperations', () => {
219 ReactDOM.render(<Comp />, root);
220
221 expectDev(console.error.calls.count()).toBe(1);
207 - expectDev(console.error.calls.argsFor(0)[0]).toEqual(
222 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
223 'Warning: `NaN` is an invalid value for the `fontSize` css style property.' +
209 - '\n\nCheck the render method of `Comp`.',
224 + '\n in div (at **)' +
225 + '\n in Comp (at **)',
226 );
227 });
228
@@ -217,11 +233,8 @@ describe('CSSPropertyOperations', () => {
233 }
234 }
235
220 - spyOn(console, 'error');
236 var root = document.createElement('div');
237 ReactDOM.render(<Comp />, root);
223 -
224 - expectDev(console.error.calls.count()).toBe(0);
238 });
239
240 it('should warn about style containing a Infinity value', () => {
@@ -238,9 +251,10 @@ describe('CSSPropertyOperations', () => {
251 ReactDOM.render(<Comp />, root);
252
253 expectDev(console.error.calls.count()).toBe(1);
241 - expectDev(console.error.calls.argsFor(0)[0]).toEqual(
254 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
255 'Warning: `Infinity` is an invalid value for the `fontSize` css style property.' +
243 - '\n\nCheck the render method of `Comp`.',
256 + '\n in div (at **)' +
257 + '\n in Comp (at **)',
258 );
259 });
260
packages/react-dom/src/shared/__tests__/ReactDOMComponent-test.js
+5 -2
@@ -255,8 +255,11 @@ describe('ReactDOMComponent', () => {
255 ReactDOM.render(<span style={style} />, div);
256
257 expectDev(console.error.calls.count()).toBe(1);
258 - expectDev(console.error.calls.argsFor(0)[0]).toEqual(
259 - 'Warning: `NaN` is an invalid value for the `fontSize` css style property.',
258 + expectDev(
259 + normalizeCodeLocInfo(console.error.calls.argsFor(0)[0]),
260 + ).toEqual(
261 + 'Warning: `NaN` is an invalid value for the `fontSize` css style property.' +
262 + '\n in span (at **)',
263 );
264 });
265
packages/react-dom/src/shared/warnValidStyle.js
+18 -31
@@ -16,7 +16,6 @@ var warnValidStyle = emptyFunction;
16 if (__DEV__) {
17 var camelizeStyleName = require('fbjs/lib/camelizeStyleName');
18 var warning = require('fbjs/lib/warning');
19 - var {getCurrentFiberOwnerName} = require('ReactDebugCurrentFiber');
19
20 // 'msTransform' is correct, but the other prefixes should be capitalized
21 var badVendoredStyleNamePattern = /^(?:webkit|moz|o)[A-Z]/;
@@ -29,7 +28,7 @@ if (__DEV__) {
28 var warnedForNaNValue = false;
29 var warnedForInfinityValue = false;
30
32 - var warnHyphenatedStyleName = function(name) {
31 + var warnHyphenatedStyleName = function(name, getStack) {
32 if (warnedStyleNames.hasOwnProperty(name) && warnedStyleNames[name]) {
33 return;
34 }
@@ -40,11 +39,11 @@ if (__DEV__) {
39 'Unsupported style property %s. Did you mean %s?%s',
40 name,
41 camelizeStyleName(name),
43 - checkRenderMessage(),
42 + getStack(),
43 );
44 };
45
47 - var warnBadVendoredStyleName = function(name) {
46 + var warnBadVendoredStyleName = function(name, getStack) {
47 if (warnedStyleNames.hasOwnProperty(name) && warnedStyleNames[name]) {
48 return;
49 }
@@ -55,11 +54,11 @@ if (__DEV__) {
54 'Unsupported vendor-prefixed style property %s. Did you mean %s?%s',
55 name,
56 name.charAt(0).toUpperCase() + name.slice(1),
58 - checkRenderMessage(),
57 + getStack(),
58 );
59 };
60
62 - var warnStyleValueWithSemicolon = function(name, value) {
61 + var warnStyleValueWithSemicolon = function(name, value, getStack) {
62 if (warnedStyleValues.hasOwnProperty(value) && warnedStyleValues[value]) {
63 return;
64 }
@@ -67,15 +66,15 @@ if (__DEV__) {
66 warnedStyleValues[value] = true;
67 warning(
68 false,
70 - "Style property values shouldn't contain a semicolon.%s " +
71 - 'Try "%s: %s" instead.',
72 - checkRenderMessage(),
69 + "Style property values shouldn't contain a semicolon. " +
70 + 'Try "%s: %s" instead.%s',
71 name,
72 value.replace(badStyleValueWithSemicolonPattern, ''),
73 + getStack(),
74 );
75 };
76
78 - var warnStyleValueIsNaN = function(name, value) {
77 + var warnStyleValueIsNaN = function(name, value, getStack) {
78 if (warnedForNaNValue) {
79 return;
80 }
@@ -85,11 +84,11 @@ if (__DEV__) {
84 false,
85 '`NaN` is an invalid value for the `%s` css style property.%s',
86 name,
88 - checkRenderMessage(),
87 + getStack(),
88 );
89 };
90
92 - var warnStyleValueIsInfinity = function(name, value) {
91 + var warnStyleValueIsInfinity = function(name, value, getStack) {
92 if (warnedForInfinityValue) {
93 return;
94 }
@@ -99,36 +98,24 @@ if (__DEV__) {
98 false,
99 '`Infinity` is an invalid value for the `%s` css style property.%s',
100 name,
102 - checkRenderMessage(),
101 + getStack(),
102 );
103 };
104
106 - var checkRenderMessage = function() {
107 - var ownerName;
108 - // Fiber doesn't pass it but uses ReactDebugCurrentFiber to track it.
109 - // It is only enabled in development and tracks host components too.
110 - ownerName = getCurrentFiberOwnerName();
111 - // TODO: also report the stack.
112 - if (ownerName) {
113 - return '\n\nCheck the render method of `' + ownerName + '`.';
114 - }
115 - return '';
116 - };
117 -
118 - warnValidStyle = function(name, value, component) {
105 + warnValidStyle = function(name, value, getStack) {
106 if (name.indexOf('-') > -1) {
120 - warnHyphenatedStyleName(name);
107 + warnHyphenatedStyleName(name, getStack);
108 } else if (badVendoredStyleNamePattern.test(name)) {
122 - warnBadVendoredStyleName(name);
109 + warnBadVendoredStyleName(name, getStack);
110 } else if (badStyleValueWithSemicolonPattern.test(value)) {
124 - warnStyleValueWithSemicolon(name, value);
111 + warnStyleValueWithSemicolon(name, value, getStack);
112 }
113
114 if (typeof value === 'number') {
115 if (isNaN(value)) {
129 - warnStyleValueIsNaN(name, value);
116 + warnStyleValueIsNaN(name, value, getStack);
117 } else if (!isFinite(value)) {
131 - warnStyleValueIsInfinity(name, value);
118 + warnStyleValueIsInfinity(name, value, getStack);
119 }
120 }
121 };