Fix for facebook#6062 : Show source line number on unknown property …

…warning (facebook#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
zpao · May 3, 2016 · 7cf61db · 7cf61db
1 parent f25a88e
commit 7cf61db
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 5 deletions.
diff --git a/.babelrc b/.babelrc
@@ -22,6 +22,7 @@
     "transform-es2015-modules-commonjs",
     "transform-es3-member-expression-literals",
     "transform-es3-property-literals",
-    "./scripts/babel/transform-object-assign-require"
+    "./scripts/babel/transform-object-assign-require",
+    "babel-preset-react/node_modules/babel-plugin-transform-react-jsx-source"
   ]
 }
diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js
@@ -29,6 +29,7 @@ var ReactDOMButton = require('ReactDOMButton');
 var ReactDOMComponentFlags = require('ReactDOMComponentFlags');
 var ReactDOMComponentTree = require('ReactDOMComponentTree');
 var ReactDOMInput = require('ReactDOMInput');
+var ReactDOMInstrumentation = require('ReactDOMInstrumentation');
 var ReactDOMOption = require('ReactDOMOption');
 var ReactDOMSelect = require('ReactDOMSelect');
 var ReactDOMTextarea = require('ReactDOMTextarea');
@@ -579,6 +580,10 @@ ReactDOMComponent.Mixin = {
         validateDOMNesting.updatedAncestorInfo(parentInfo, this._tag, this);
     }
 
+    if (__DEV__) {
+      ReactDOMInstrumentation.debugTool.onMountDOMComponent(this._debugID, this._currentElement);
+    }
+
     var mountImage;
     if (transaction.useCreateElement) {
       var ownerDocument = nativeContainerInfo._ownerDocument;
@@ -854,6 +859,10 @@ ReactDOMComponent.Mixin = {
       context
     );
 
+    if (__DEV__) {
+      ReactDOMInstrumentation.debugTool.onUpdateDOMComponent(this._debugID, this._currentElement);
+    }
+
     if (this._tag === 'select') {
       // <select> value update needs to occur after <option> children
       // reconciliation

diff --git a/src/renderers/dom/shared/ReactDOMDebugTool.js b/src/renderers/dom/shared/ReactDOMDebugTool.js
@@ -62,6 +62,12 @@ var ReactDOMDebugTool = {
   onTestEvent() {
     emitEvent('onTestEvent');
   },
+  onMountDOMComponent(debugID, element) {
+    emitEvent('onMountDOMComponent', debugID, element);
+  },
+  onUpdateDOMComponent(debugID, element) {
+    emitEvent('onMountDOMComponent', debugID, element);
+  },
 };
 
 ReactDOMDebugTool.addDevtool(ReactDOMUnknownPropertyDevtool);

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -1300,5 +1300,102 @@ describe('ReactDOMComponent', function() {
       ReactTestUtils.renderIntoDocument(<div onFocusOut={() => {}} />);
       expect(console.error.argsForCall.length).toBe(2);
     });
+
+    it('gives source code refs for unknown prop warning', function() {
+      spyOn(console, 'error');
+      ReactDOMServer.renderToString(<div class="paladin"/>);
+      ReactDOMServer.renderToString(<input type="text" onclick="1"/>);
+      expect(console.error.argsForCall.length).toBe(2);
+      expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/);
+      expect(console.error.argsForCall[1][0]).toMatch(/.*onClick.*\(.*:\d+\)/);
+    });
+
+    it('gives source code refs for unknown prop warning for update render', function() {
+      spyOn(console, 'error');
+      var container = document.createElement('div');
+
+      ReactDOMServer.renderToString(<div className="paladin" />, container);
+      expect(console.error.argsForCall.length).toBe(0);
+
+      ReactDOMServer.renderToString(<div class="paladin" />, container);
+      expect(console.error.argsForCall.length).toBe(1);
+      expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/);
+    });
+
+    it('gives source code refs for unknown prop warning for exact elements ', function() {
+      spyOn(console, 'error');
+
+      ReactDOMServer.renderToString(
+        <div className="foo1">
+        <div class="foo2"/>
+        <div onClick="foo3"/>
+        <div onclick="foo4"/>
+        <div className="foo5"/>
+        <div className="foo6"/>
+        </div>
+      );
+
+      expect(console.error.argsForCall.length).toBe(2);
+
+      var matches = console.error.argsForCall[0][0].match(/.*className.*\(.*:(\d+)\)/);
+      var previousLine = matches[1];
+
+      matches = console.error.argsForCall[1][0].match(/.*onClick.*\(.*:(\d+)\)/);
+      var currentLine = matches[1];
+
+      //verify line number has a proper relative difference,
+      //since hard coding the line number would make test too brittle
+      expect(parseInt(previousLine, 10) + 2).toBe(parseInt(currentLine, 10));
+    });
+
+    it('gives source code refs for unknown prop warning for exact elements in composition ', function() {
+      spyOn(console, 'error');
+      var container = document.createElement('div');
+
+      var Parent = React.createClass({
+        render: function() {
+          return <div><Child1 /><Child2 /><Child3 /><Child4 /></div>;
+        },
+      });
+
+      var Child1 = React.createClass({
+        render: function() {
+          return <div class="paladin">Child1</div>;
+        },
+      });
+
+      var Child2 = React.createClass({
+        render: function() {
+          return <div>Child2</div>;
+        },
+      });
+
+      var Child3 = React.createClass({
+        render: function() {
+          return <div onclick="1">Child3</div>;
+        },
+      });
+
+      var Child4 = React.createClass({
+        render: function() {
+          return <div>Child4</div>;
+        },
+      });
+
+      ReactDOMServer.renderToString(<Parent />, container);
+
+      expect(console.error.argsForCall.length).toBe(2);
+
+      var matches = console.error.argsForCall[0][0].match(/.*className.*\(.*:(\d+)\)/);
+      var previousLine = matches[1];
+
+      matches = console.error.argsForCall[1][0].match(/.*onClick.*\(.*:(\d+)\)/);
+      var currentLine = matches[1];
+
+      //verify line number has a proper relative difference,
+      //since hard coding the line number would make test too brittle
+      expect(parseInt(previousLine, 10) + 12).toBe(parseInt(currentLine, 10));
+
+    });
   });
 });
diff --git a/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js b/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js
@@ -17,6 +17,7 @@ var EventPluginRegistry = require('EventPluginRegistry');
 var warning = require('warning');
 
 if (__DEV__) {
+  var cachedSource;
   var reactProps = {
     children: true,
     dangerouslySetInnerHTML: true,
@@ -50,9 +51,10 @@ if (__DEV__) {
     // logging too much when using transferPropsTo.
     warning(
       standardName == null,
-      'Unknown DOM property %s. Did you mean %s?',
+      'Unknown DOM property %s. Did you mean %s? %s',
       name,
-      standardName
+      standardName,
+      formatSource(cachedSource)
     );
 
     var registrationName = (
@@ -65,11 +67,17 @@ if (__DEV__) {
 
     warning(
       registrationName == null,
-      'Unknown event handler property %s. Did you mean `%s`?',
+      'Unknown event handler property %s. Did you mean `%s`? %s',
       name,
-      registrationName
+      registrationName,
+      formatSource(cachedSource)
     );
   };
+
+  var formatSource = function(source) {
+    return source ? `(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : '';
+  };
+
 }
 
 var ReactDOMUnknownPropertyDevtool = {
@@ -82,6 +90,12 @@ var ReactDOMUnknownPropertyDevtool = {
   onDeleteValueForProperty(node, name) {
     warnUnknownProperty(name);
   },
+  onMountDOMComponent(debugID, element) {
+    cachedSource = element ? element._source : null;
+  },
+  onUpdateDOMComponent(debugID, element) {
+    cachedSource = element ? element._source : null;
+  },
 };
 
 module.exports = ReactDOMUnknownPropertyDevtool;