ReactMount should not optimistically traverse unknown nodes

perf/lib/BrowserPerfRunnerApp.react.js

@@ -197,8 +197,10 @@ var GridViewTable = React.createClass({
 
   render: function(){
     return React.DOM.table(null,
-      this._renderRow(null, 0),
-      this.props.rows.map(this._renderRow, this)
+      React.DOM.tbody(null,
+        this._renderRow(null, 0),
+        this.props.rows.map(this._renderRow, this)
+      )
     );
   }
 

perf/lib/perf-test-runner.browser.js

@@ -195,10 +195,14 @@ perfRunner.ViewObject = function(props){
 
   if (typeof value != 'object') return React.DOM.span(props, [JSON.stringify(value), " ", typeof value]);
 
-  return React.DOM.table(props, Object.keys(value).map(function(key){
-    return React.DOM.tr(null,
-      React.DOM.th(null, key),
-      React.DOM.td(null, perfRunner.ViewObject({key:key, value:value[key]}))
-    );
-  }));
+  return React.DOM.table(props,
+    React.DOM.tbody(null,
+      Object.keys(value).map(function(key){
+        return React.DOM.tr(null,
+          React.DOM.th(null, key),
+          React.DOM.td(null, perfRunner.ViewObject({key:key, value:value[key]}))
+        );
+      })
+    )
+  );
 }

src/browser/ui/ReactMount.js

@@ -610,29 +610,35 @@ var ReactMount = {
 
       while (child) {
         var childID = ReactMount.getID(child);
-        if (childID) {
-          // Even if we find the node we're looking for, we finish looping
-          // through its siblings to ensure they're cached so that we don't have
-          // to revisit this node again. Otherwise, we make n^2 calls to getID
-          // when visiting the many children of a single node in order.
-
-          if (targetID === childID) {
-            targetChild = child;
-          } else if (ReactInstanceHandles.isAncestorIDOf(childID, targetID)) {
-            // If we find a child whose ID is an ancestor of the given ID,
-            // then we can be sure that we only want to search the subtree
-            // rooted at this child, so we can throw out the rest of the
-            // search state.
-            firstChildren.length = childIndex = 0;
-            firstChildren.push(child.firstChild);
-          }
 
-        } else {
-          // If this child had no ID, then there's a chance that it was
-          // injected automatically by the browser, as when a `<table>`
-          // element sprouts an extra `<tbody>` child as a side effect of
-          // `.innerHTML` parsing. Optimistically continue down this
-          // branch, but not before examining the other siblings.
+        if (!childID) {
+          invariant(
+            false,
+            'findComponentRoot(..., %s): Encountered element without React ' +
+            'ID. ' +
+            'This probably means the DOM was unexpectedly mutated (e.g., by ' +
+            'the browser), usually due to forgetting a <tbody> when using ' +
+            'tables, nesting tags like <form>, <p>, or <a>, or using non-SVG ' +
+            'elements in an <svg> parent. ' +
+            'Try inspecting the child nodes of the element with React ID `%s`.',
+            targetID,
+            ReactMount.getID(ancestorNode)
+          );
+        }
+
+        // Even if we find the node we're looking for, we finish looping
+        // through its siblings to ensure they're cached so that we don't have
+        // to revisit this node again. Otherwise, we make n^2 calls to getID
+        // when visiting the many children of a single node in order.
+
+        if (targetID === childID) {
+          targetChild = child;
+        } else if (ReactInstanceHandles.isAncestorIDOf(childID, targetID)) {
+          // If we find a child whose ID is an ancestor of the given ID,
+          // then we can be sure that we only want to search the subtree
+          // rooted at this child, so we can throw out the rest of the
+          // search state.
+          firstChildren.length = childIndex = 0;
           firstChildren.push(child.firstChild);
         }
 

src/core/__tests__/ReactInstanceHandles-test.js

@@ -103,25 +103,6 @@ describe('ReactInstanceHandles', function() {
       ).toBe(childNodeB);
     });
 
-    it('should work around unidentified nodes', function() {
-      var parentNode = document.createElement('div');
-      var childNodeA = document.createElement('div');
-      var childNodeB = document.createElement('div');
-      parentNode.appendChild(childNodeA);
-      parentNode.appendChild(childNodeB);
-
-      ReactMount.setID(parentNode, '.0');
-      // No ID on `childNodeA`.
-      ReactMount.setID(childNodeB, '.0.0:1');
-
-      expect(
-        ReactMount.findComponentRoot(
-          parentNode,
-          ReactMount.getID(childNodeB)
-        )
-      ).toBe(childNodeB);
-    });
-
     it('should throw if a rendered element cannot be found', function() {
       var parentNode = document.createElement('table');
       var childNodeA = document.createElement('tbody');
@@ -130,8 +111,8 @@ describe('ReactInstanceHandles', function() {
       childNodeA.appendChild(childNodeB);
 
       ReactMount.setID(parentNode, '.0');
-      // No ID on `childNodeA`, it was "rendered by the browser".
-      ReactMount.setID(childNodeB, '.0.1:0');
+      ReactMount.setID(childNodeA, '.0.1:0');
+      ReactMount.setID(childNodeB, '.0.1:0.0');
 
       expect(ReactMount.findComponentRoot(
         parentNode,
@@ -141,7 +122,7 @@ describe('ReactInstanceHandles', function() {
       expect(function() {
         ReactMount.findComponentRoot(
           parentNode,
-          ReactMount.getID(childNodeB) + ":junk"
+          ReactMount.getID(childNodeA) + ":junk"
         );
       }).toThrow(
         'Invariant Violation: findComponentRoot(..., .0.1:0:junk): ' +
@@ -326,21 +307,21 @@ describe('ReactInstanceHandles', function() {
     it("should return next descendent from window", function() {
       var parent = renderParentIntoDocument();
       expect(
-        ReactInstanceHandles._getNextDescendantID(
+        ReactInstanceHandles.getNextDescendantID(
           '',
           parent.refs.P_P1._rootNodeID
         )
       ).toBe(parent.refs.P._rootNodeID);
     });
 
     it("should return window for next descendent towards window", function() {
-      expect(ReactInstanceHandles._getNextDescendantID('', '')).toBe('');
+      expect(ReactInstanceHandles.getNextDescendantID('', '')).toBe('');
     });
 
     it("should return self for next descendent towards self", function() {
       var parent = renderParentIntoDocument();
       expect(
-        ReactInstanceHandles._getNextDescendantID(
+        ReactInstanceHandles.getNextDescendantID(
           parent.refs.P_P1._rootNodeID,
           parent.refs.P_P1._rootNodeID
         )