From 6b9fc81b642df27d43327e1cc606867ba5cddab4 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 23 Sep 2013 11:19:46 -0400 Subject: [PATCH 1/2] Space optimizations for ReactMount.findComponentRoot. --- src/core/ReactMount.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index 5d636a0a95e..3dff445bc5d 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -46,6 +46,9 @@ if (__DEV__) { var rootElementsByReactRootID = {}; } +/** Used to store breadth-first search state in findComponentRoot. */ +var reusableArray = []; + /** * @param {DOMElement} container DOM element that may contain a React component. * @return {?string} A "reactRoot" ID, if a React component is rendered. @@ -538,15 +541,19 @@ var ReactMount = { * @internal */ findComponentRoot: function(ancestorNode, id) { - var firstChildren = [ancestorNode.firstChild]; + var firstChildren = reusableArray; var childIndex = 0; + firstChildren.length = 0; + firstChildren.push(ancestorNode.firstChild); + while (childIndex < firstChildren.length) { var child = firstChildren[childIndex++]; while (child) { var childID = ReactMount.getID(child); if (childID) { if (id === childID) { + firstChildren.length = 0; return child; } else if (ReactInstanceHandles.isAncestorIDOf(childID, id)) { // If we find a child whose ID is an ancestor of the given ID, @@ -556,11 +563,6 @@ var ReactMount = { firstChildren.length = childIndex = 0; firstChildren.push(child.firstChild); break; - } else { - // TODO This should not be necessary if the ID hierarchy is - // correct, but is occasionally necessary if the DOM has been - // modified in unexpected ways. - firstChildren.push(child.firstChild); } } else { // If this child had no ID, then there's a chance that it was @@ -574,6 +576,8 @@ var ReactMount = { } } + firstChildren.length = 0; + if (__DEV__) { console.error( 'Error while invoking `findComponentRoot` with the following ' + From 87db648f3e1c0a7edfc5809ff0803f981264d7ae Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 24 Sep 2013 16:07:37 -0400 Subject: [PATCH 2/2] Address pull request feedback. --- src/core/ReactMount.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index 3dff445bc5d..a3b0440a1cf 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -46,8 +46,8 @@ if (__DEV__) { var rootElementsByReactRootID = {}; } -/** Used to store breadth-first search state in findComponentRoot. */ -var reusableArray = []; +// Used to store breadth-first search state in findComponentRoot. +var findComponentRootReusableArray = []; /** * @param {DOMElement} container DOM element that may contain a React component. @@ -541,11 +541,11 @@ var ReactMount = { * @internal */ findComponentRoot: function(ancestorNode, id) { - var firstChildren = reusableArray; + var firstChildren = findComponentRootReusableArray; var childIndex = 0; - firstChildren.length = 0; - firstChildren.push(ancestorNode.firstChild); + firstChildren[0] = ancestorNode.firstChild; + firstChildren.length = 1; while (childIndex < firstChildren.length) { var child = firstChildren[childIndex++]; @@ -553,17 +553,27 @@ var ReactMount = { var childID = ReactMount.getID(child); if (childID) { if (id === childID) { + // 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; + return child; - } else if (ReactInstanceHandles.isAncestorIDOf(childID, id)) { + } + + if (ReactInstanceHandles.isAncestorIDOf(childID, id)) { // 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); + + // Ignore the rest of this child's siblings and immediately + // continue the outer loop with child.firstChild as child. break; } + } else { // If this child had no ID, then there's a chance that it was // injected automatically by the browser, as when a `` @@ -572,6 +582,7 @@ var ReactMount = { // branch, but not before examining the other siblings. firstChildren.push(child.firstChild); } + child = child.nextSibling; } }