facebook · syranide · Sep 27, 2014
diff --git a/perf/lib/BrowserPerfRunnerApp.react.js b/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)
+      )
     );
   }
 

diff --git a/perf/lib/perf-test-runner.browser.js b/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]}))
+        );
+      })
+    )
+  );
 }
diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js
@@ -56,9 +56,6 @@ if (__DEV__) {
   var rootElementsByReactRootID = {};
 }
 
-// Used to store breadth-first search state in findComponentRoot.
-var findComponentRootReusableArray = [];
-
 /**
  * @param {DOMElement} container DOM element that may contain a React component.
  * @return {?string} A "reactRoot" ID, if a React component is rendered.
@@ -172,33 +169,6 @@ function purgeID(id) {
   delete nodeCache[id];
 }
 
-var deepestNodeSoFar = null;
-function findDeepestCachedAncestorImpl(ancestorID) {
-  var ancestor = nodeCache[ancestorID];
-  if (ancestor && isValid(ancestor, ancestorID)) {
-    deepestNodeSoFar = ancestor;
-  } else {
-    // This node isn't populated in the cache, so presumably none of its
-    // descendants are. Break out of the loop.
-    return false;
-  }
-}
-
-/**
- * Return the deepest cached node whose ID is a prefix of `targetID`.
- */
-function findDeepestCachedAncestor(targetID) {
-  deepestNodeSoFar = null;
-  ReactInstanceHandles.traverseAncestors(
-    targetID,
-    findDeepestCachedAncestorImpl
-  );
-
-  var foundNode = deepestNodeSoFar;
-  deepestNodeSoFar = null;
-  return foundNode;
-}
-
 /**
  * Mounting is the process of initializing a React component by creatings its
  * representative DOM elements and inserting them into a supplied `container`.
@@ -587,81 +557,86 @@ var ReactMount = {
 
   /**
    * Finds a node with the supplied `targetID` inside of the supplied
-   * `ancestorNode`.  Exploits the ID naming scheme to perform the search
-   * quickly.
+   * `ancestorNode`.  Exploits the ID naming scheme to perform the search quickly.
    *
    * @param {DOMEventTarget} ancestorNode Search from this root.
    * @pararm {string} targetID ID of the DOM representation of the component.
    * @return {DOMEventTarget} DOM node with the supplied `targetID`.
    * @internal
    */
-  findComponentRoot: function(ancestorNode, targetID) {
-    var firstChildren = findComponentRootReusableArray;
-    var childIndex = 0;
-
-    var deepestAncestor = findDeepestCachedAncestor(targetID) || ancestorNode;
-
-    firstChildren[0] = deepestAncestor.firstChild;
-    firstChildren.length = 1;
-
-    while (childIndex < firstChildren.length) {
-      var child = firstChildren[childIndex++];
-      var targetChild;
-
-      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);
-          }
+  findComponentRoot: function(searchNode, targetID) {
+    var ancestorNode = searchNode;
+    var ancestorID = getID(ancestorNode);
+    var nextDescendantID, nextDescendantNode;
+
+    // Skip already cached IDs to the first uncached ID or targetID is found.
+    while (ancestorID !== targetID) {
+      nextDescendantID =
+        ReactInstanceHandles.getNextDescendantID(ancestorID, targetID);
+      nextDescendantNode = nodeCache[nextDescendantID];
+
+      // ID not found in cache.
+      if (!nextDescendantNode ||
+          !isValid(nextDescendantNode, nextDescendantID)) {
+        break;
+      }
 
-        } 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.
-          firstChildren.push(child.firstChild);
+      ancestorNode = nextDescendantNode;
+      ancestorID = nextDescendantID;
+    }
+
+    // Update the cache with getID for all siblings of the first child. Then
+    // simply get the next descendant node from the cache.
+    while (ancestorID !== targetID) {
+      var childNode = ancestorNode.firstChild;
+
+      while (childNode) {
+        // Populate cache with all siblings.
+        var childID = getID(childNode);
+
+        if (!childID) {
+          invariant(
+            false,
+            'findComponentRoot(..., %s): Found element without a 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,
+            ancestorID
+          );
         }
 
-        child = child.nextSibling;
+        childNode = childNode.nextSibling;
       }
 
-      if (targetChild) {
-        // Emptying firstChildren/findComponentRootReusableArray is
-        // not necessary for correctness, but it helps the GC reclaim
-        // any nodes that were left at the end of the search.
-        firstChildren.length = 0;
+      // Get next descendant node from cache.
+      nextDescendantID =
+        ReactInstanceHandles.getNextDescendantID(ancestorID, targetID);
+      nextDescendantNode = nodeCache[nextDescendantID];
 
-        return targetChild;
+      if (!nextDescendantNode) {
+        invariant(
+          false,
+          'findComponentRoot(..., %s): Unable to find element. ' +
+          '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,
+          ancestorID
+        );
       }
-    }
 
-    firstChildren.length = 0;
+      ancestorNode = nextDescendantNode;
+      ancestorID = nextDescendantID;
+    }
 
-    invariant(
-      false,
-      'findComponentRoot(..., %s): Unable to find element. 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)
-    );
+    return ancestorNode;
   },
 
 

diff --git a/src/core/ReactInstanceHandles.js b/src/core/ReactInstanceHandles.js
@@ -323,11 +323,7 @@ var ReactInstanceHandles = {
    */
   _getFirstCommonAncestorID: getFirstCommonAncestorID,
 
-  /**
-   * Exposed for unit testing.
-   * @private
-   */
-  _getNextDescendantID: getNextDescendantID,
+  getNextDescendantID: getNextDescendantID,
 
   isAncestorIDOf: isAncestorIDOf,
 

diff --git a/src/core/__tests__/ReactInstanceHandles-test.js b/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
         )