@samitouri / QOS-React-2 / commits / 7cf61db257

Fix for #6062 : Show source line number on unknown property warning (#6398)

Fix for #6062 : Show source line number on unknown property warning (#6398) * New approach for 6062 fix : Show source line number on unknown property warning * WIP: ReactDebugToolEventForwarderDevTool * Update event signature to debugID * Trigger events in ReactDOMComponent * Renamed to onMountDOMComponent; passing in element directly * Added debugID; updated simple test * Added test for multi-div JSX to ref exact line * Added test for composite component

Troy DeMonbreun committed May 3, 2016 at 17:51 UTC 7cf61db257707c49b26c94cd32d5a5d1a5d784ae
5 files changed +132 -5
.babelrc
+2 -1
@@ -22,6 +22,7 @@
22 "transform-es2015-modules-commonjs",
23 "transform-es3-member-expression-literals",
24 "transform-es3-property-literals",
25 - "./scripts/babel/transform-object-assign-require"
25 + "./scripts/babel/transform-object-assign-require",
26 + "babel-preset-react/node_modules/babel-plugin-transform-react-jsx-source"
27 ]
28 }
src/renderers/dom/shared/ReactDOMComponent.js
+9
@@ -29,6 +29,7 @@ var ReactDOMButton = require('ReactDOMButton');
29 var ReactDOMComponentFlags = require('ReactDOMComponentFlags');
30 var ReactDOMComponentTree = require('ReactDOMComponentTree');
31 var ReactDOMInput = require('ReactDOMInput');
32 +var ReactDOMInstrumentation = require('ReactDOMInstrumentation');
33 var ReactDOMOption = require('ReactDOMOption');
34 var ReactDOMSelect = require('ReactDOMSelect');
35 var ReactDOMTextarea = require('ReactDOMTextarea');
@@ -579,6 +580,10 @@ ReactDOMComponent.Mixin = {
580 validateDOMNesting.updatedAncestorInfo(parentInfo, this._tag, this);
581 }
582
583 + if (__DEV__) {
584 + ReactDOMInstrumentation.debugTool.onMountDOMComponent(this._debugID, this._currentElement);
585 + }
586 +
587 var mountImage;
588 if (transaction.useCreateElement) {
589 var ownerDocument = nativeContainerInfo._ownerDocument;
@@ -854,6 +859,10 @@ ReactDOMComponent.Mixin = {
859 context
860 );
861
862 + if (__DEV__) {
863 + ReactDOMInstrumentation.debugTool.onUpdateDOMComponent(this._debugID, this._currentElement);
864 + }
865 +
866 if (this._tag === 'select') {
867 // <select> value update needs to occur after <option> children
868 // reconciliation
src/renderers/dom/shared/ReactDOMDebugTool.js
+6
@@ -62,6 +62,12 @@ var ReactDOMDebugTool = {
62 onTestEvent() {
63 emitEvent('onTestEvent');
64 },
65 + onMountDOMComponent(debugID, element) {
66 + emitEvent('onMountDOMComponent', debugID, element);
67 + },
68 + onUpdateDOMComponent(debugID, element) {
69 + emitEvent('onMountDOMComponent', debugID, element);
70 + },
71 };
72
73 ReactDOMDebugTool.addDevtool(ReactDOMUnknownPropertyDevtool);
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+97
@@ -1300,5 +1300,102 @@ describe('ReactDOMComponent', function() {
1300 ReactTestUtils.renderIntoDocument(<div onFocusOut={() => {}} />);
1301 expect(console.error.argsForCall.length).toBe(2);
1302 });
1303 +
1304 + it('gives source code refs for unknown prop warning', function() {
1305 + spyOn(console, 'error');
1306 + ReactDOMServer.renderToString(<div class="paladin"/>);
1307 + ReactDOMServer.renderToString(<input type="text" onclick="1"/>);
1308 + expect(console.error.argsForCall.length).toBe(2);
1309 + expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/);
1310 + expect(console.error.argsForCall[1][0]).toMatch(/.*onClick.*\(.*:\d+\)/);
1311 + });
1312 +
1313 + it('gives source code refs for unknown prop warning for update render', function() {
1314 + spyOn(console, 'error');
1315 + var container = document.createElement('div');
1316 +
1317 + ReactDOMServer.renderToString(<div className="paladin" />, container);
1318 + expect(console.error.argsForCall.length).toBe(0);
1319 +
1320 + ReactDOMServer.renderToString(<div class="paladin" />, container);
1321 + expect(console.error.argsForCall.length).toBe(1);
1322 + expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/);
1323 + });
1324 +
1325 + it('gives source code refs for unknown prop warning for exact elements ', function() {
1326 + spyOn(console, 'error');
1327 +
1328 + ReactDOMServer.renderToString(
1329 + <div className="foo1">
1330 + <div class="foo2"/>
1331 + <div onClick="foo3"/>
1332 + <div onclick="foo4"/>
1333 + <div className="foo5"/>
1334 + <div className="foo6"/>
1335 + </div>
1336 + );
1337 +
1338 + expect(console.error.argsForCall.length).toBe(2);
1339 +
1340 + var matches = console.error.argsForCall[0][0].match(/.*className.*\(.*:(\d+)\)/);
1341 + var previousLine = matches[1];
1342 +
1343 + matches = console.error.argsForCall[1][0].match(/.*onClick.*\(.*:(\d+)\)/);
1344 + var currentLine = matches[1];
1345 +
1346 + //verify line number has a proper relative difference,
1347 + //since hard coding the line number would make test too brittle
1348 + expect(parseInt(previousLine, 10) + 2).toBe(parseInt(currentLine, 10));
1349 + });
1350 +
1351 + it('gives source code refs for unknown prop warning for exact elements in composition ', function() {
1352 + spyOn(console, 'error');
1353 + var container = document.createElement('div');
1354 +
1355 + var Parent = React.createClass({
1356 + render: function() {
1357 + return <div><Child1 /><Child2 /><Child3 /><Child4 /></div>;
1358 + },
1359 + });
1360 +
1361 + var Child1 = React.createClass({
1362 + render: function() {
1363 + return <div class="paladin">Child1</div>;
1364 + },
1365 + });
1366 +
1367 + var Child2 = React.createClass({
1368 + render: function() {
1369 + return <div>Child2</div>;
1370 + },
1371 + });
1372 +
1373 + var Child3 = React.createClass({
1374 + render: function() {
1375 + return <div onclick="1">Child3</div>;
1376 + },
1377 + });
1378 +
1379 + var Child4 = React.createClass({
1380 + render: function() {
1381 + return <div>Child4</div>;
1382 + },
1383 + });
1384 +
1385 + ReactDOMServer.renderToString(<Parent />, container);
1386 +
1387 + expect(console.error.argsForCall.length).toBe(2);
1388 +
1389 + var matches = console.error.argsForCall[0][0].match(/.*className.*\(.*:(\d+)\)/);
1390 + var previousLine = matches[1];
1391 +
1392 + matches = console.error.argsForCall[1][0].match(/.*onClick.*\(.*:(\d+)\)/);
1393 + var currentLine = matches[1];
1394 +
1395 + //verify line number has a proper relative difference,
1396 + //since hard coding the line number would make test too brittle
1397 + expect(parseInt(previousLine, 10) + 12).toBe(parseInt(currentLine, 10));
1398 +
1399 + });
1400 });
1401 });
src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js
+18 -4
@@ -17,6 +17,7 @@ var EventPluginRegistry = require('EventPluginRegistry');
17 var warning = require('warning');
18
19 if (__DEV__) {
20 + var cachedSource;
21 var reactProps = {
22 children: true,
23 dangerouslySetInnerHTML: true,
@@ -50,9 +51,10 @@ if (__DEV__) {
51 // logging too much when using transferPropsTo.
52 warning(
53 standardName == null,
53 - 'Unknown DOM property %s. Did you mean %s?',
54 + 'Unknown DOM property %s. Did you mean %s? %s',
55 name,
55 - standardName
56 + standardName,
57 + formatSource(cachedSource)
58 );
59
60 var registrationName = (
@@ -65,11 +67,17 @@ if (__DEV__) {
67
68 warning(
69 registrationName == null,
68 - 'Unknown event handler property %s. Did you mean `%s`?',
70 + 'Unknown event handler property %s. Did you mean `%s`? %s',
71 name,
70 - registrationName
72 + registrationName,
73 + formatSource(cachedSource)
74 );
75 };
76 +
77 + var formatSource = function(source) {
78 + return source ? `(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : '';
79 + };
80 +
81 }
82
83 var ReactDOMUnknownPropertyDevtool = {
@@ -82,6 +90,12 @@ var ReactDOMUnknownPropertyDevtool = {
90 onDeleteValueForProperty(node, name) {
91 warnUnknownProperty(name);
92 },
93 + onMountDOMComponent(debugID, element) {
94 + cachedSource = element ? element._source : null;
95 + },
96 + onUpdateDOMComponent(debugID, element) {
97 + cachedSource = element ? element._source : null;
98 + },
99 };
100
101 module.exports = ReactDOMUnknownPropertyDevtool;