From 71bc932ce2d0efbe81c6dad1df18b10293f7743c Mon Sep 17 00:00:00 2001 From: Andreas Svensson Date: Mon, 19 May 2014 22:33:19 +0200 Subject: [PATCH 1/4] Remove data-reactid and traverse immediately after insertion instead --- perf/lib/BrowserPerfRunnerApp.react.js | 6 +- perf/lib/perf-test-runner.browser.js | 16 +- src/browser/ReactTextComponent.js | 2 +- src/browser/server/ReactMarkupChecksum.js | 11 + .../__tests__/ReactServerRendering-test.js | 32 +- .../ui/ReactComponentBrowserEnvironment.js | 9 +- src/browser/ui/ReactDOMComponent.js | 10 +- src/browser/ui/ReactDOMIDOperations.js | 9 +- src/browser/ui/ReactMount.js | 311 ++++++++---------- src/browser/ui/dom/DOMChildrenOperations.js | 16 +- src/browser/ui/dom/DOMProperty.js | 2 +- src/browser/ui/dom/DOMPropertyOperations.js | 11 - src/browser/ui/dom/Danger.js | 1 + src/core/ReactComponent.js | 4 +- src/core/ReactCompositeComponent.js | 3 +- src/core/ReactMultiChild.js | 8 +- .../__tests__/ReactInstanceHandles-test.js | 79 ----- src/core/__tests__/ReactTextComponent-test.js | 37 --- 18 files changed, 218 insertions(+), 349 deletions(-) delete mode 100644 src/core/__tests__/ReactTextComponent-test.js diff --git a/perf/lib/BrowserPerfRunnerApp.react.js b/perf/lib/BrowserPerfRunnerApp.react.js index 5b34f3bcaaec..9d25e700b884 100644 --- 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 index 26e059f8c38c..0579c2e7fe3a 100644 --- 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/ReactTextComponent.js b/src/browser/ReactTextComponent.js index e23c72023218..7411f53e22e5 100644 --- a/src/browser/ReactTextComponent.js +++ b/src/browser/ReactTextComponent.js @@ -78,7 +78,7 @@ mixInto(ReactTextComponent, { } return ( - '' + + '' + escapedText + '' ); diff --git a/src/browser/server/ReactMarkupChecksum.js b/src/browser/server/ReactMarkupChecksum.js index 2198c276805c..0852c614c68c 100644 --- a/src/browser/server/ReactMarkupChecksum.js +++ b/src/browser/server/ReactMarkupChecksum.js @@ -35,6 +35,17 @@ var ReactMarkupChecksum = { ); }, + /** + * @param {DOMElement} element root React element + * @returns {boolean} whether or not the element can be reused + */ + canReuseNode: function(element) { + return ( + element && element.hasAttribute && + element.hasAttribute(ReactMarkupChecksum.CHECKSUM_ATTR_NAME) + ); + }, + /** * @param {string} markup to use * @param {DOMElement} element root React element diff --git a/src/browser/server/__tests__/ReactServerRendering-test.js b/src/browser/server/__tests__/ReactServerRendering-test.js index 895ce51238e8..a584c995d8d9 100644 --- a/src/browser/server/__tests__/ReactServerRendering-test.js +++ b/src/browser/server/__tests__/ReactServerRendering-test.js @@ -38,8 +38,6 @@ var ReactServerRendering; var ReactMarkupChecksum; var ExecutionEnvironment; -var ID_ATTRIBUTE_NAME; - describe('ReactServerRendering', function() { beforeEach(function() { require('mock-modules').dumpCache(); @@ -50,9 +48,14 @@ describe('ReactServerRendering', function() { ExecutionEnvironment.canUseDOM = false; ReactServerRendering = require('ReactServerRendering'); ReactMarkupChecksum = require('ReactMarkupChecksum'); + }); - var DOMProperty = require('DOMProperty'); - ID_ATTRIBUTE_NAME = DOMProperty.ID_ATTRIBUTE_NAME; + describe('canReuseNode', function() { + it('should not crash on text nodes', function() { + expect(function() { + ReactMarkupChecksum.canReuseNode(document.createTextNode('yolo')); + }).not.toThrow(); + }); }); describe('renderComponentToString', function() { @@ -61,8 +64,9 @@ describe('ReactServerRendering', function() { hello world ); expect(response).toMatch( - 'hello world' + '' + + 'hello world' + + '' ); }); @@ -91,11 +95,10 @@ describe('ReactServerRendering', function() { ); expect(response).toMatch( - '
' + - '' + - 'My name is ' + - 'child' + + '
' + + '' + + 'My name is ' + + 'child' + '' + '
' ); @@ -141,10 +144,9 @@ describe('ReactServerRendering', function() { ); expect(response).toMatch( - '' + - 'Component name: ' + - 'TestComponent' + + '' + + 'Component name: ' + + 'TestComponent' + '' ); expect(lifecycle).toEqual( diff --git a/src/browser/ui/ReactComponentBrowserEnvironment.js b/src/browser/ui/ReactComponentBrowserEnvironment.js index a04bb7906717..c97d753f2aa3 100644 --- a/src/browser/ui/ReactComponentBrowserEnvironment.js +++ b/src/browser/ui/ReactComponentBrowserEnvironment.js @@ -64,7 +64,7 @@ var ReactComponentBrowserEnvironment = { mountImageIntoNode: ReactPerf.measure( 'ReactComponentBrowserEnvironment', 'mountImageIntoNode', - function(markup, container, shouldReuseMarkup) { + function(markup, container, instance, shouldReuseMarkup) { invariant( container && ( container.nodeType === ELEMENT_NODE_TYPE || @@ -74,9 +74,9 @@ var ReactComponentBrowserEnvironment = { ); if (shouldReuseMarkup) { - if (ReactMarkupChecksum.canReuseMarkup( - markup, - getReactRootElementInContainer(container))) { + var rootElement = getReactRootElementInContainer(container); + if (ReactMarkupChecksum.canReuseMarkup(markup, rootElement)) { + ReactMount.mountHierarchy(rootElement, instance); return; } else { invariant( @@ -115,6 +115,7 @@ var ReactComponentBrowserEnvironment = { ); setInnerHTML(container, markup); + ReactMount.mountHierarchy(container.firstChild, instance); } ) }; diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 6b65664e8673..843da20a3a9c 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -164,14 +164,7 @@ ReactDOMComponent.Mixin = { } } - // For static pages, no need to put React ID and checksum. Saves lots of - // bytes. - if (transaction.renderToStaticMarkup) { - return ret + '>'; - } - - var markupForID = DOMPropertyOperations.createMarkupForID(this._rootNodeID); - return ret + ' ' + markupForID + '>'; + return ret + '>'; }, /** @@ -405,6 +398,7 @@ ReactDOMComponent.Mixin = { unmountComponent: function() { this.unmountChildren(); ReactEventEmitter.deleteAllListeners(this._rootNodeID); + ReactMount.purgeID(this._rootNodeID); ReactComponent.Mixin.unmountComponent.call(this); } diff --git a/src/browser/ui/ReactDOMIDOperations.js b/src/browser/ui/ReactDOMIDOperations.js index ea206cd065b9..1752f4d9755b 100644 --- a/src/browser/ui/ReactDOMIDOperations.js +++ b/src/browser/ui/ReactDOMIDOperations.js @@ -161,9 +161,12 @@ var ReactDOMIDOperations = { dangerouslyReplaceNodeWithMarkupByID: ReactPerf.measure( 'ReactDOMIDOperations', 'dangerouslyReplaceNodeWithMarkupByID', - function(id, markup) { - var node = ReactMount.getNode(id); - DOMChildrenOperations.dangerouslyReplaceNodeWithMarkup(node, markup); + function(id, markup, instance) { + DOMChildrenOperations.dangerouslyReplaceNodeWithMarkup( + ReactMount.getNode(id), + markup, + instance + ); } ), diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js index e898b5899488..4ad5a444616e 100644 --- a/src/browser/ui/ReactMount.js +++ b/src/browser/ui/ReactMount.js @@ -23,6 +23,7 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactEventEmitter = require('ReactEventEmitter'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ReactPerf = require('ReactPerf'); +var ReactMarkupChecksum = require('ReactMarkupChecksum'); var containsNode = require('containsNode'); var getReactRootElementInContainer = require('getReactRootElementInContainer'); @@ -33,8 +34,13 @@ var warning = require('warning'); var SEPARATOR = ReactInstanceHandles.SEPARATOR; -var ATTR_NAME = DOMProperty.ID_ATTRIBUTE_NAME; -var nodeCache = {}; +var ID_PROP_NAME = DOMProperty.ID_PROPERTY_NAME; +var nodesByReactRootID = {}; + +if (__DEV__) { + /** Mapping from reactRootID to first React component instance in hierarchy.*/ + var firstInstancesForNodeByReactRootID = {}; +} var ELEMENT_NODE_TYPE = 1; var DOC_NODE_TYPE = 9; @@ -50,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. @@ -63,42 +66,28 @@ function getReactRootID(container) { } /** - * Accessing node[ATTR_NAME] or calling getAttribute(ATTR_NAME) on a form - * element can return its control whose name or ID equals ATTR_NAME. All - * DOM nodes support `getAttributeNode` but this can also get called on - * other objects so just return '' if we're given something other than a - * DOM node (such as window). - * * @param {?DOMElement|DOMWindow|DOMDocument|DOMTextNode} node DOM node. * @return {string} ID of the supplied `domNode`. */ function getID(node) { var id = internalGetID(node); - if (id) { - if (nodeCache.hasOwnProperty(id)) { - var cached = nodeCache[id]; - if (cached !== node) { - invariant( - !isValid(cached, id), - 'ReactMount: Two valid but unequal nodes with the same `%s`: %s', - ATTR_NAME, id - ); - nodeCache[id] = node; - } - } else { - nodeCache[id] = node; - } + if (id) { + invariant( + nodesByReactRootID.hasOwnProperty(id), + 'getID(): Element not in node lookup.' + ); + invariant( + nodesByReactRootID[id] === node, + 'getID(): Node is different from node in nodesByReactRootID.' + ); } return id; } function internalGetID(node) { - // If node is something like a window, document, or text node, none of - // which support attributes or a .getAttribute method, gracefully return - // the empty string, as if the attribute were missing. - return node && node.getAttribute && node.getAttribute(ATTR_NAME) || ''; + return node && node[ID_PROP_NAME] || ''; } /** @@ -110,10 +99,10 @@ function internalGetID(node) { function setID(node, id) { var oldID = internalGetID(node); if (oldID !== id) { - delete nodeCache[oldID]; + delete nodesByReactRootID[oldID]; } - node.setAttribute(ATTR_NAME, id); - nodeCache[id] = node; + node[ID_PROP_NAME] = id; + nodesByReactRootID[id] = node; } /** @@ -124,10 +113,19 @@ function setID(node, id) { * @internal */ function getNode(id) { - if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { - nodeCache[id] = ReactMount.findReactNodeByID(id); - } - return nodeCache[id]; + invariant( + nodesByReactRootID.hasOwnProperty(id), + 'getNode(): Node does not exist in nodesByReactRootID.' + ); + + var node = nodesByReactRootID[id]; + + invariant( + isValid(node, id), + 'getNode(): Node is not valid.' + ); + + return node; } /** @@ -145,7 +143,7 @@ function isValid(node, id) { invariant( internalGetID(node) === id, 'ReactMount: Unexpected modification of `%s`', - ATTR_NAME + id ); var container = ReactMount.findReactContainerForID(id); @@ -158,39 +156,116 @@ function isValid(node, id) { } /** - * Causes the cache to forget about one React-specific ID. + * Causes the lookup to forget about one React-specific ID. * * @param {string} id The ID to forget. */ function purgeID(id) { - delete nodeCache[id]; + delete nodesByReactRootID[id]; + + if (__DEV__) { + delete firstInstancesForNodeByReactRootID[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; +function getHumanReadablePathTo(node) { + var nodes = []; + + while (node && node !== document) { + nodes.push(node); + node = node.parentNode; } + + var humanReadablePath = []; + + for (var i = 0; node = nodes[i]; i++) { + var parentID = internalGetID(node); + + if (parentID) { + var parentInstance = firstInstancesForNodeByReactRootID[parentID]; + + while (parentInstance._renderedComponent) { + humanReadablePath.push( + parentInstance.displayName || 'UnnamedComponent' + ); + parentInstance = parentInstance._renderedComponent; + } + } + + humanReadablePath.push(node.tagName); + } + + humanReadablePath.reverse(); + + return humanReadablePath.join(' > '); } -/** - * Return the deepest cached node whose ID is a prefix of `targetID`. - */ -function findDeepestCachedAncestor(targetID) { - deepestNodeSoFar = null; - ReactInstanceHandles.traverseAncestors( - targetID, - findDeepestCachedAncestorImpl +function mountHierarchy(node, instance) { + if (__DEV__) { + firstInstancesForNodeByReactRootID[instance._rootNodeID] = instance; + } + + // Step through and ignore any ReactCompositeComponents + while (instance._renderedComponent) { + instance = instance._renderedComponent; + } + + var expectedTagName = instance.tagName; + if (!expectedTagName) { + // Only ReactTextComponent doesn't have a tagName at the moment + expectedTagName = 'SPAN'; + } + + invariant( + node && node.tagName, + 'Expected to see a rendered node of type "%s", but there were no more ' + + 'elements in the DOM.', + expectedTagName ); - var foundNode = deepestNodeSoFar; - deepestNodeSoFar = null; - return foundNode; + if (node.tagName !== expectedTagName) { + if (__DEV__) { + console.warn(getHumanReadablePathTo(node)); + } + + invariant( + false, + 'Expected to see a rendered node of type "%s", but the actual ' + + 'element is of type "%s". This is most commonly caused by invalid ' + + 'nesting, such as nesting or

, without a , etc.', + expectedTagName, node.tagName + ); + } + + var rootID = instance._rootNodeID; + nodesByReactRootID[rootID] = node; + node[ID_PROP_NAME] = rootID; + + var childInstances = instance._renderedChildren; + + if (!childInstances) { + return; + } + + var childNode = node.firstChild; + + for (var id in childInstances) { + if (childInstances.hasOwnProperty(id)) { + var childInstance = childInstances[id]; + mountHierarchy(childNode, childInstance); + childNode = childNode.nextSibling; + } + } + + if (childNode) { + invariant( + false, + 'Expected there to be no more rendered elements, but found a ' + + 'node of type `%s`. This is most commonly caused by invalid ' + + 'nesting, such as nesting or

.', + childNode.tagName + ); + } } /** @@ -361,10 +436,10 @@ var ReactMount = { } var reactRootElement = getReactRootElementInContainer(container); - var containerHasReactMarkup = - reactRootElement && ReactMount.isRenderedByReact(reactRootElement); - - var shouldReuseMarkup = containerHasReactMarkup && !prevComponent; + var shouldReuseMarkup = ( + reactRootElement && !prevComponent && + ReactMarkupChecksum.canReuseNode(reactRootElement) + ); var component = ReactMount._renderNewRootComponent( nextDescriptor, @@ -526,33 +601,6 @@ var ReactMount = { return container; }, - /** - * Finds an element rendered by React with the supplied ID. - * - * @param {string} id ID of a DOM node in the React component. - * @return {DOMElement} Root DOM node of the React component. - */ - findReactNodeByID: function(id) { - var reactRoot = ReactMount.findReactContainerForID(id); - return ReactMount.findComponentRoot(reactRoot, id); - }, - - /** - * True if the supplied `node` is rendered by React. - * - * @param {*} node DOM Element to check. - * @return {boolean} True if the DOM Element appears to be rendered by React. - * @internal - */ - isRenderedByReact: function(node) { - if (node.nodeType !== 1) { - // Not a DOMElement, therefore not a React component - return false; - } - var id = ReactMount.getID(node); - return id ? id.charAt(0) === SEPARATOR : false; - }, - /** * Traverses up the ancestors of the supplied node to find a node that is a * DOM representation of a React component. @@ -564,7 +612,7 @@ var ReactMount = { getFirstReactDOM: function(node) { var current = node; while (current && current.parentNode !== current) { - if (ReactMount.isRenderedByReact(current)) { + if (current[ID_PROP_NAME]) { return current; } current = current.parentNode; @@ -572,91 +620,14 @@ var ReactMount = { return null; }, - /** - * Finds a node with the supplied `targetID` inside of the supplied - * `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); - } - - } else { - // If this child had no ID, then there's a chance that it was - // injected automatically by the browser, as when a `

` - // element sprouts an extra `` child as a side effect of - // `.innerHTML` parsing. Optimistically continue down this - // branch, but not before examining the other siblings. - firstChildren.push(child.firstChild); - } - - child = child.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; - - return targetChild; - } - } - - firstChildren.length = 0; - - 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 when using tables or nesting

' + - 'or tags. Try inspecting the child nodes of the element with React ' + - 'ID `%s`.', - targetID, - ReactMount.getID(ancestorNode) - ); - }, - - /** * React ID utilities. */ getReactRootID: getReactRootID, + mountHierarchy: mountHierarchy, + getID: getID, setID: setID, diff --git a/src/browser/ui/dom/DOMChildrenOperations.js b/src/browser/ui/dom/DOMChildrenOperations.js index e2e3de2005a7..362755e821bc 100644 --- a/src/browser/ui/dom/DOMChildrenOperations.js +++ b/src/browser/ui/dom/DOMChildrenOperations.js @@ -21,6 +21,7 @@ var Danger = require('Danger'); var ReactMultiChildUpdateTypes = require('ReactMultiChildUpdateTypes'); +var ReactMount = require('ReactMount'); var getTextContentAccessor = require('getTextContentAccessor'); var invariant = require('invariant'); @@ -88,10 +89,13 @@ if (textContentAccessor === 'textContent') { */ var DOMChildrenOperations = { - dangerouslyReplaceNodeWithMarkup: Danger.dangerouslyReplaceNodeWithMarkup, - updateTextContent: updateTextContent, + dangerouslyReplaceNodeWithMarkup: function(node, markup, instance) { + var renderedNode = Danger.dangerouslyReplaceNodeWithMarkup(node, markup); + ReactMount.mountHierarchy(renderedNode, instance); + }, + /** * Updates a component's children by processing a series of updates. The * update configurations are each expected to have a `parentNode` property. @@ -146,11 +150,9 @@ var DOMChildrenOperations = { for (var k = 0; update = updates[k]; k++) { switch (update.type) { case ReactMultiChildUpdateTypes.INSERT_MARKUP: - insertChildAt( - update.parentNode, - renderedMarkup[update.markupIndex], - update.toIndex - ); + var markupNode = renderedMarkup[update.markupIndex]; + insertChildAt(update.parentNode, markupNode, update.toIndex); + ReactMount.mountHierarchy(markupNode, update.instance); break; case ReactMultiChildUpdateTypes.MOVE_EXISTING: insertChildAt( diff --git a/src/browser/ui/dom/DOMProperty.js b/src/browser/ui/dom/DOMProperty.js index 8d3d6626d0c1..2abd7466ee9e 100644 --- a/src/browser/ui/dom/DOMProperty.js +++ b/src/browser/ui/dom/DOMProperty.js @@ -159,7 +159,7 @@ var defaultValueCache = {}; */ var DOMProperty = { - ID_ATTRIBUTE_NAME: 'data-reactid', + ID_PROPERTY_NAME: '__reactID', /** * Checks whether a property name is a standard property. diff --git a/src/browser/ui/dom/DOMPropertyOperations.js b/src/browser/ui/dom/DOMPropertyOperations.js index 12a3502bb9f1..aa67c1637b3f 100644 --- a/src/browser/ui/dom/DOMPropertyOperations.js +++ b/src/browser/ui/dom/DOMPropertyOperations.js @@ -73,17 +73,6 @@ if (__DEV__) { */ var DOMPropertyOperations = { - /** - * Creates markup for the ID property. - * - * @param {string} id Unescaped ID. - * @return {string} Markup string. - */ - createMarkupForID: function(id) { - return processAttributeNameAndPrefix(DOMProperty.ID_ATTRIBUTE_NAME) + - escapeTextForBrowser(id) + '"'; - }, - /** * Creates markup for a property. * diff --git a/src/browser/ui/dom/Danger.js b/src/browser/ui/dom/Danger.js index 6e2dd071056e..12c1f3186c85 100644 --- a/src/browser/ui/dom/Danger.js +++ b/src/browser/ui/dom/Danger.js @@ -180,6 +180,7 @@ var Danger = { var newChild = createNodesFromMarkup(markup, emptyFunction)[0]; oldChild.parentNode.replaceChild(newChild, oldChild); + return newChild; } }; diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index b2659a2075f5..0a4126e53d26 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -289,7 +289,7 @@ var ReactComponent = { if (props.ref != null) { ReactOwner.removeComponentAsRefFrom(this, props.ref, this._owner); } - unmountIDFromEnvironment(this._rootNodeID); + //unmountIDFromEnvironment(this._rootNodeID); this._rootNodeID = null; this._lifeCycleState = ComponentLifeCycle.UNMOUNTED; }, @@ -410,7 +410,7 @@ var ReactComponent = { transaction, shouldReuseMarkup) { var markup = this.mountComponent(rootID, transaction, 0); - mountImageIntoNode(markup, container, shouldReuseMarkup); + mountImageIntoNode(markup, container, this, shouldReuseMarkup); }, /** diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 668074bdae7d..73c4120cc8ea 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -1137,7 +1137,8 @@ var ReactCompositeComponentMixin = { ); ReactComponent.BackendIDOperations.dangerouslyReplaceNodeWithMarkupByID( prevComponentID, - nextMarkup + nextMarkup, + this ); } } diff --git a/src/core/ReactMultiChild.js b/src/core/ReactMultiChild.js index 608fe62d8461..f4b987682f3e 100644 --- a/src/core/ReactMultiChild.js +++ b/src/core/ReactMultiChild.js @@ -61,13 +61,14 @@ var markupQueue = []; * @param {number} toIndex Destination index. * @private */ -function enqueueMarkup(parentID, markup, toIndex) { +function enqueueMarkup(parentID, markup, instance, toIndex) { // NOTE: Null values reduce hidden classes. updateQueue.push({ parentID: parentID, parentNode: null, type: ReactMultiChildUpdateTypes.INSERT_MARKUP, markupIndex: markupQueue.push(markup) - 1, + instance: instance, textContent: null, fromIndex: null, toIndex: toIndex @@ -89,6 +90,7 @@ function enqueueMove(parentID, fromIndex, toIndex) { parentNode: null, type: ReactMultiChildUpdateTypes.MOVE_EXISTING, markupIndex: null, + instance: null, textContent: null, fromIndex: fromIndex, toIndex: toIndex @@ -109,6 +111,7 @@ function enqueueRemove(parentID, fromIndex) { parentNode: null, type: ReactMultiChildUpdateTypes.REMOVE_NODE, markupIndex: null, + instance: null, textContent: null, fromIndex: fromIndex, toIndex: null @@ -129,6 +132,7 @@ function enqueueTextContent(parentID, textContent) { parentNode: null, type: ReactMultiChildUpdateTypes.TEXT_CONTENT, markupIndex: null, + instance: null, textContent: textContent, fromIndex: null, toIndex: null @@ -359,7 +363,7 @@ var ReactMultiChild = { * @protected */ createChild: function(child, mountImage) { - enqueueMarkup(this._rootNodeID, mountImage, child._mountIndex); + enqueueMarkup(this._rootNodeID, mountImage, child, child._mountIndex); }, /** diff --git a/src/core/__tests__/ReactInstanceHandles-test.js b/src/core/__tests__/ReactInstanceHandles-test.js index 4a5fe2243ee9..a2c1a66b28c7 100644 --- a/src/core/__tests__/ReactInstanceHandles-test.js +++ b/src/core/__tests__/ReactInstanceHandles-test.js @@ -75,85 +75,6 @@ describe('ReactInstanceHandles', function() { aggregatedArgs = []; }); - describe('isRenderedByReact', function() { - it('should not crash on text nodes', function() { - expect(function() { - ReactMount.isRenderedByReact(document.createTextNode('yolo')); - }).not.toThrow(); - }); - }); - - describe('findComponentRoot', function() { - it('should find the correct node with prefix sibling IDs', 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'); - ReactMount.setID(childNodeA, '.0.0'); - ReactMount.setID(childNodeB, '.0.0:1'); - - expect( - ReactMount.findComponentRoot( - parentNode, - ReactMount.getID(childNodeB) - ) - ).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'); - var childNodeB = document.createElement('tr'); - parentNode.appendChild(childNodeA); - childNodeA.appendChild(childNodeB); - - ReactMount.setID(parentNode, '.0'); - // No ID on `childNodeA`, it was "rendered by the browser". - ReactMount.setID(childNodeB, '.0.1:0'); - - expect(ReactMount.findComponentRoot( - parentNode, - ReactMount.getID(childNodeB) - )).toBe(childNodeB); - - expect(function() { - ReactMount.findComponentRoot( - parentNode, - ReactMount.getID(childNodeB) + ":junk" - ); - }).toThrow( - 'Invariant Violation: findComponentRoot(..., .0.1:0:junk): ' + - 'Unable to find element. This probably means the DOM was ' + - 'unexpectedly mutated (e.g., by the browser), usually due to ' + - 'forgetting a

when using tables or nesting

or ' + - 'tags. Try inspecting the child nodes of the element with React ' + - 'ID `.0`.' - ); - }); - }); - describe('getReactRootIDFromNodeID', function() { it('should support strings', function() { var test = '.s_0_1.0..1'; diff --git a/src/core/__tests__/ReactTextComponent-test.js b/src/core/__tests__/ReactTextComponent-test.js deleted file mode 100644 index 0eee800d5967..000000000000 --- a/src/core/__tests__/ReactTextComponent-test.js +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Copyright 2013-2014 Facebook, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * @jsx React.DOM - * @emails react-core - */ - -"use strict"; - -var React; - -describe('ReactTextComponent', function() { - beforeEach(function() { - React = require('React'); - }); - - it('should escape the rootID', function(){ - var ThisThingShouldBeEscaped = '">>> LULZ <<<"'; - var ThisThingWasBeEscaped = '">>> LULZ <<<"'; - var thing = React.DOM.div(null, React.DOM.span({key:ThisThingShouldBeEscaped}, ["LULZ"])); - var html = React.renderComponentToString(thing); - expect(html).not.toContain(ThisThingShouldBeEscaped); - expect(html).toContain(ThisThingWasBeEscaped); - }) -}); From d103b3b285213d97a0973fb75c8d27f97bbd1aab Mon Sep 17 00:00:00 2001 From: Andreas Svensson Date: Fri, 23 May 2014 23:40:07 +0200 Subject: [PATCH 2/4] Lazy traverse instead --- src/browser/ReactTextComponent.js | 5 +- src/browser/server/ReactServerRendering.js | 4 +- src/browser/ui/ReactDOMComponent.js | 4 +- src/browser/ui/ReactMount.js | 154 ++++++++++++++++++++- src/core/ReactComponent.js | 5 +- src/core/ReactCompositeComponent.js | 5 +- src/core/ReactMultiChild.js | 2 + 7 files changed, 171 insertions(+), 8 deletions(-) diff --git a/src/browser/ReactTextComponent.js b/src/browser/ReactTextComponent.js index 7411f53e22e5..1d02a5f49223 100644 --- a/src/browser/ReactTextComponent.js +++ b/src/browser/ReactTextComponent.js @@ -23,6 +23,7 @@ var DOMPropertyOperations = require('DOMPropertyOperations'); var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin'); var ReactComponent = require('ReactComponent'); var ReactDescriptor = require('ReactDescriptor'); +var ReactMount = require('ReactMount'); var escapeTextForBrowser = require('escapeTextForBrowser'); var mixInto = require('mixInto'); @@ -60,14 +61,16 @@ mixInto(ReactTextComponent, { * @return {string} Markup for this text node. * @internal */ - mountComponent: function(rootID, transaction, mountDepth) { + mountComponent: function(parentID, rootID, transaction, mountDepth) { ReactComponent.Mixin.mountComponent.call( this, + parentID, rootID, transaction, mountDepth ); + ReactMount.addDOMInstance(this); var escapedText = escapeTextForBrowser(this.props); if (transaction.renderToStaticMarkup) { diff --git a/src/browser/server/ReactServerRendering.js b/src/browser/server/ReactServerRendering.js index cf61cc4aaab5..af130caaabe7 100644 --- a/src/browser/server/ReactServerRendering.js +++ b/src/browser/server/ReactServerRendering.js @@ -50,7 +50,7 @@ function renderComponentToString(component) { return transaction.perform(function() { var componentInstance = instantiateReactComponent(component); - var markup = componentInstance.mountComponent(id, transaction, 0); + var markup = componentInstance.mountComponent(null, id, transaction, 0); return ReactMarkupChecksum.addChecksumToMarkup(markup); }, null); } finally { @@ -76,7 +76,7 @@ function renderComponentToStaticMarkup(component) { return transaction.perform(function() { var componentInstance = instantiateReactComponent(component); - return componentInstance.mountComponent(id, transaction, 0); + return componentInstance.mountComponent(null, id, transaction, 0); }, null); } finally { ReactServerRenderingTransaction.release(transaction); diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 843da20a3a9c..2205590d82b9 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -107,13 +107,15 @@ ReactDOMComponent.Mixin = { mountComponent: ReactPerf.measure( 'ReactDOMComponent', 'mountComponent', - function(rootID, transaction, mountDepth) { + function(parentID, rootID, transaction, mountDepth) { ReactComponent.Mixin.mountComponent.call( this, + parentID, rootID, transaction, mountDepth ); + ReactMount.addDOMInstance(this); assertValidProps(this.props); return ( this._createOpenTagMarkupAndPutListeners(transaction) + diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js index 4ad5a444616e..11e31804f430 100644 --- a/src/browser/ui/ReactMount.js +++ b/src/browser/ui/ReactMount.js @@ -36,6 +36,7 @@ var SEPARATOR = ReactInstanceHandles.SEPARATOR; var ID_PROP_NAME = DOMProperty.ID_PROPERTY_NAME; var nodesByReactRootID = {}; +var reactDOMInstancesByReactRootID = {}; if (__DEV__) { /** Mapping from reactRootID to first React component instance in hierarchy.*/ @@ -70,7 +71,9 @@ function getReactRootID(container) { * @return {string} ID of the supplied `domNode`. */ function getID(node) { - var id = internalGetID(node); + var id = ''; // = internalGetID(node); + var instance = getInstanceForNode(node); + if (instance) id = instance._rootNodeID; if (id) { invariant( @@ -113,6 +116,12 @@ function setID(node, id) { * @internal */ function getNode(id) { + var instance = reactDOMInstancesByReactRootID[id]; + + invariant(instance, 'awd'); + + var node = getNodeForInstance(instance); + invariant( nodesByReactRootID.hasOwnProperty(id), 'getNode(): Node does not exist in nodesByReactRootID.' @@ -162,6 +171,7 @@ function isValid(node, id) { */ function purgeID(id) { delete nodesByReactRootID[id]; + delete reactDOMInstancesByReactRootID[id]; if (__DEV__) { delete firstInstancesForNodeByReactRootID[id]; @@ -201,6 +211,8 @@ function getHumanReadablePathTo(node) { } function mountHierarchy(node, instance) { + evaluateDOMChild(node, instance); + return; if (__DEV__) { firstInstancesForNodeByReactRootID[instance._rootNodeID] = instance; } @@ -268,6 +280,139 @@ function mountHierarchy(node, instance) { } } +function getNodeForInstance(instance) { + var rootID = instance._rootNodeID; + if (nodesByReactRootID.hasOwnProperty(rootID)) { + return nodesByReactRootID[rootID]; + } + + evaluateToInstance(instance); + + if (nodesByReactRootID.hasOwnProperty(rootID)) { + return nodesByReactRootID[rootID]; + } +} + +function getInstanceForNode(node) { + var rootID = node[ID_PROP_NAME]; + if (rootID) { + return reactDOMInstancesByReactRootID[rootID]; + } + + evaluateToNode(node); + + rootID = node[ID_PROP_NAME] + if (rootID) { + return reactDOMInstancesByReactRootID[rootID]; + } +} + +function evaluateToInstance(instance) { + var rootID = instance._rootNodeID; + var isCached = nodesByReactRootID.hasOwnProperty(rootID); + + if (!isCached) { + evaluateToInstance(reactDOMInstancesByReactRootID[instance._parentNodeID]); + isCached = nodesByReactRootID.hasOwnProperty(rootID); + } + if (isCached) { + evaluateDOMChildren(nodesByReactRootID[rootID], instance); + } +} + +function evaluateToNode(node) { + if (!node || node === document) { + return; + } + + var rootID = node[ID_PROP_NAME]; + + if (!rootID) { + evaluateToNode(node.parentNode); + rootID = node[ID_PROP_NAME]; + } + if (rootID) { + evaluateDOMChildren(node, reactDOMInstancesByReactRootID[rootID]); + } +} + +function evaluateDOMChild(node, rootInstance) { + var leafInstance = rootInstance; + // Step through and ignore any ReactCompositeComponents + while (leafInstance._renderedComponent) { + leafInstance = leafInstance._renderedComponent; + } + + var expectedTagName = leafInstance.tagName; + if (!expectedTagName) { + // Only ReactTextComponent doesn't have a tagName at the moment + expectedTagName = 'SPAN'; + } + + invariant( + node && node.tagName, + 'Expected to see a rendered node of type "%s", but there were no more ' + + 'elements in the DOM.', + expectedTagName + ); + + if (node.tagName !== expectedTagName) { + if (__DEV__) { + console.warn(getHumanReadablePathTo(node)); + } + + invariant( + false, + 'Expected to see a rendered node of type "%s", but the actual ' + + 'element is of type "%s". This is most commonly caused by invalid ' + + 'nesting, such as nesting or

,

without a , etc.', + expectedTagName, node.tagName + ); + } + + var rootID = rootInstance._rootNodeID; + nodesByReactRootID[rootID] = node; + node[ID_PROP_NAME] = rootID; + + if (__DEV__) { + firstInstancesForNodeByReactRootID[rootID] = rootInstance; + } + + //evaluateDOMChildren(node, leafInstance); + + return node.nextSibling; +} + +function evaluateDOMChildren(parentNode, parentInstance) { + var childInstances = parentInstance._renderedChildren; + + if (!childInstances) { + return; + } + + var childNode = parentNode.firstChild; + + for (var id in childInstances) { + if (childInstances.hasOwnProperty(id)) { + childNode = evaluateDOMChild(childNode, childInstances[id]); + } + } + + if (childNode) { + if (__DEV__) { + console.warn(getHumanReadablePathTo(childNode)); + } + + invariant( + false, + 'Expected there to be no more rendered elements, but found a ' + + 'node of type `%s`. This is most commonly caused by invalid ' + + 'nesting, such as nesting or

.', + childNode.tagName + ); + } +} + /** * Mounting is the process of initializing a React component by creatings its * representative DOM elements and inserting them into a supplied `container`. @@ -624,6 +769,9 @@ var ReactMount = { * React ID utilities. */ + getNodeForInstance: getNodeForInstance, + getInstanceForNode: getInstanceForNode, + getReactRootID: getReactRootID, mountHierarchy: mountHierarchy, @@ -634,6 +782,10 @@ var ReactMount = { getNode: getNode, + addDOMInstance: function(instance) { + reactDOMInstancesByReactRootID[instance._rootNodeID] = instance; + }, + purgeID: purgeID }; diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 0a4126e53d26..93efb537f423 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -251,7 +251,7 @@ var ReactComponent = { * @return {?string} Rendered markup to be inserted into the DOM. * @internal */ - mountComponent: function(rootID, transaction, mountDepth) { + mountComponent: function(parentID, rootID, transaction, mountDepth) { invariant( !this.isMounted(), 'mountComponent(%s, ...): Can only mount an unmounted component. ' + @@ -264,6 +264,7 @@ var ReactComponent = { var owner = this._descriptor._owner; ReactOwner.addComponentAsRefTo(this, props.ref, owner); } + this._parentNodeID = parentID; this._rootNodeID = rootID; this._lifeCycleState = ComponentLifeCycle.MOUNTED; this._mountDepth = mountDepth; @@ -409,7 +410,7 @@ var ReactComponent = { container, transaction, shouldReuseMarkup) { - var markup = this.mountComponent(rootID, transaction, 0); + var markup = this.mountComponent(null, rootID, transaction, 0); mountImageIntoNode(markup, container, this, shouldReuseMarkup); }, diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 73c4120cc8ea..ac7e3904ae78 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -713,9 +713,10 @@ var ReactCompositeComponentMixin = { mountComponent: ReactPerf.measure( 'ReactCompositeComponent', 'mountComponent', - function(rootID, transaction, mountDepth) { + function(parentID, rootID, transaction, mountDepth) { ReactComponent.Mixin.mountComponent.call( this, + parentID, rootID, transaction, mountDepth @@ -757,6 +758,7 @@ var ReactCompositeComponentMixin = { // Done with mounting, `setState` will now trigger UI changes. this._compositeLifeCycleState = null; var markup = this._renderedComponent.mountComponent( + parentID, rootID, transaction, mountDepth + 1 @@ -1131,6 +1133,7 @@ var ReactCompositeComponentMixin = { prevComponentInstance.unmountComponent(); this._renderedComponent = instantiateReactComponent(nextDescriptor); var nextMarkup = this._renderedComponent.mountComponent( + this._parentNodeID, thisID, transaction, this._mountDepth + 1 diff --git a/src/core/ReactMultiChild.js b/src/core/ReactMultiChild.js index f4b987682f3e..0572d98d93b8 100644 --- a/src/core/ReactMultiChild.js +++ b/src/core/ReactMultiChild.js @@ -204,6 +204,7 @@ var ReactMultiChild = { // Inlined for performance, see `ReactInstanceHandles.createReactID`. var rootID = this._rootNodeID + name; var mountImage = childInstance.mountComponent( + this._rootNodeID, rootID, transaction, this._mountDepth + 1 @@ -401,6 +402,7 @@ var ReactMultiChild = { // Inlined for performance, see `ReactInstanceHandles.createReactID`. var rootID = this._rootNodeID + name; var mountImage = child.mountComponent( + this.rootNodeID, rootID, transaction, this._mountDepth + 1 From 260e4443544142856130471e3499edd656d0e166 Mon Sep 17 00:00:00 2001 From: Andreas Svensson Date: Sat, 24 May 2014 13:55:08 +0200 Subject: [PATCH 3/4] Improvements to lazy eval and documentation --- src/browser/ReactReconcileTransaction.js | 1 + src/browser/ReactTextComponent.js | 4 +- .../__tests__/ReactEventEmitter-test.js | 1 + src/browser/server/ReactMarkupChecksum.js | 4 +- .../server/ReactServerRenderingTransaction.js | 1 + .../__tests__/ReactServerRendering-test.js | 4 +- .../ui/ReactComponentBrowserEnvironment.js | 4 +- src/browser/ui/ReactDOMComponent.js | 4 +- src/browser/ui/ReactDOMIDOperations.js | 4 +- src/browser/ui/ReactMount.js | 369 +++++++++--------- src/browser/ui/dom/DOMChildrenOperations.js | 4 +- src/core/ReactCompositeComponent.js | 4 +- .../__tests__/ReactCompositeComponent-test.js | 2 +- 13 files changed, 202 insertions(+), 204 deletions(-) diff --git a/src/browser/ReactReconcileTransaction.js b/src/browser/ReactReconcileTransaction.js index 87f726884a79..be35c646a887 100644 --- a/src/browser/ReactReconcileTransaction.js +++ b/src/browser/ReactReconcileTransaction.js @@ -131,6 +131,7 @@ function ReactReconcileTransaction() { // `ReactServerRenderingTransaction` instead. This option is here so that it's // accessible and defaults to false when `ReactDOMComponent` and // `ReactTextComponent` checks it in `mountComponent`.` + this.renderToString = false; this.renderToStaticMarkup = false; this.reactMountReady = CallbackQueue.getPooled(null); this.putListenerQueue = ReactPutListenerQueue.getPooled(); diff --git a/src/browser/ReactTextComponent.js b/src/browser/ReactTextComponent.js index 1d02a5f49223..5961349d1b6f 100644 --- a/src/browser/ReactTextComponent.js +++ b/src/browser/ReactTextComponent.js @@ -70,7 +70,9 @@ mixInto(ReactTextComponent, { mountDepth ); - ReactMount.addDOMInstance(this); + if (!transaction.renderToString) { + ReactMount.registerDOMInstance(this); + } var escapedText = escapeTextForBrowser(this.props); if (transaction.renderToStaticMarkup) { diff --git a/src/browser/__tests__/ReactEventEmitter-test.js b/src/browser/__tests__/ReactEventEmitter-test.js index 2aeaecbedfb1..82f075b25dce 100644 --- a/src/browser/__tests__/ReactEventEmitter-test.js +++ b/src/browser/__tests__/ReactEventEmitter-test.js @@ -100,6 +100,7 @@ describe('ReactEventEmitter', function() { EventListener = require('EventListener'); ReactEventEmitter = require('ReactEventEmitter'); ReactTestUtils = require('ReactTestUtils'); + ReactMount.getID = getID; ReactMount.getNode = function(id) { return idToNode[id]; }; diff --git a/src/browser/server/ReactMarkupChecksum.js b/src/browser/server/ReactMarkupChecksum.js index 0852c614c68c..18ac10428e75 100644 --- a/src/browser/server/ReactMarkupChecksum.js +++ b/src/browser/server/ReactMarkupChecksum.js @@ -37,9 +37,9 @@ var ReactMarkupChecksum = { /** * @param {DOMElement} element root React element - * @returns {boolean} whether or not the element can be reused + * @returns {boolean} whether or not the root element can be reused */ - canReuseNode: function(element) { + canReuseRoot: function(element) { return ( element && element.hasAttribute && element.hasAttribute(ReactMarkupChecksum.CHECKSUM_ATTR_NAME) diff --git a/src/browser/server/ReactServerRenderingTransaction.js b/src/browser/server/ReactServerRenderingTransaction.js index 2251f82f96bd..e78fc1886db5 100644 --- a/src/browser/server/ReactServerRenderingTransaction.js +++ b/src/browser/server/ReactServerRenderingTransaction.js @@ -66,6 +66,7 @@ var TRANSACTION_WRAPPERS = [ */ function ReactServerRenderingTransaction(renderToStaticMarkup) { this.reinitializeTransaction(); + this.renderToString = true; this.renderToStaticMarkup = renderToStaticMarkup; this.reactMountReady = CallbackQueue.getPooled(null); this.putListenerQueue = ReactPutListenerQueue.getPooled(); diff --git a/src/browser/server/__tests__/ReactServerRendering-test.js b/src/browser/server/__tests__/ReactServerRendering-test.js index a584c995d8d9..9dc2f249375a 100644 --- a/src/browser/server/__tests__/ReactServerRendering-test.js +++ b/src/browser/server/__tests__/ReactServerRendering-test.js @@ -50,10 +50,10 @@ describe('ReactServerRendering', function() { ReactMarkupChecksum = require('ReactMarkupChecksum'); }); - describe('canReuseNode', function() { + describe('canReuseRoot', function() { it('should not crash on text nodes', function() { expect(function() { - ReactMarkupChecksum.canReuseNode(document.createTextNode('yolo')); + ReactMarkupChecksum.canReuseRoot(document.createTextNode('yolo')); }).not.toThrow(); }); }); diff --git a/src/browser/ui/ReactComponentBrowserEnvironment.js b/src/browser/ui/ReactComponentBrowserEnvironment.js index c97d753f2aa3..8c7ec25a58a7 100644 --- a/src/browser/ui/ReactComponentBrowserEnvironment.js +++ b/src/browser/ui/ReactComponentBrowserEnvironment.js @@ -76,7 +76,7 @@ var ReactComponentBrowserEnvironment = { if (shouldReuseMarkup) { var rootElement = getReactRootElementInContainer(container); if (ReactMarkupChecksum.canReuseMarkup(markup, rootElement)) { - ReactMount.mountHierarchy(rootElement, instance); + ReactMount.evaluateRoot(rootElement, instance); return; } else { invariant( @@ -115,7 +115,7 @@ var ReactComponentBrowserEnvironment = { ); setInnerHTML(container, markup); - ReactMount.mountHierarchy(container.firstChild, instance); + ReactMount.evaluateRoot(container.firstChild, instance); } ) }; diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 2205590d82b9..1b8e120c9a8f 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -115,7 +115,9 @@ ReactDOMComponent.Mixin = { transaction, mountDepth ); - ReactMount.addDOMInstance(this); + if (!transaction.renderToString) { + ReactMount.registerDOMInstance(this); + } assertValidProps(this.props); return ( this._createOpenTagMarkupAndPutListeners(transaction) + diff --git a/src/browser/ui/ReactDOMIDOperations.js b/src/browser/ui/ReactDOMIDOperations.js index 1752f4d9755b..115f840ce022 100644 --- a/src/browser/ui/ReactDOMIDOperations.js +++ b/src/browser/ui/ReactDOMIDOperations.js @@ -161,9 +161,9 @@ var ReactDOMIDOperations = { dangerouslyReplaceNodeWithMarkupByID: ReactPerf.measure( 'ReactDOMIDOperations', 'dangerouslyReplaceNodeWithMarkupByID', - function(id, markup, instance) { + function(node, markup, instance) { DOMChildrenOperations.dangerouslyReplaceNodeWithMarkup( - ReactMount.getNode(id), + node, markup, instance ); diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js index 11e31804f430..777f594f0743 100644 --- a/src/browser/ui/ReactMount.js +++ b/src/browser/ui/ReactMount.js @@ -39,8 +39,9 @@ var nodesByReactRootID = {}; var reactDOMInstancesByReactRootID = {}; if (__DEV__) { - /** Mapping from reactRootID to first React component instance in hierarchy.*/ - var firstInstancesForNodeByReactRootID = {}; + /** Mapping from reactRootID to the owning React component instance of a + React node instance */ + var owningInstancesForNodeByReactRootID = {}; } var ELEMENT_NODE_TYPE = 1; @@ -71,21 +72,30 @@ function getReactRootID(container) { * @return {string} ID of the supplied `domNode`. */ function getID(node) { - var id = ''; // = internalGetID(node); - var instance = getInstanceForNode(node); - if (instance) id = instance._rootNodeID; + if (!node || node.nodeType !== ELEMENT_NODE_TYPE) { + return ''; + } - if (id) { - invariant( - nodesByReactRootID.hasOwnProperty(id), - 'getID(): Element not in node lookup.' - ); - invariant( - nodesByReactRootID[id] === node, - 'getID(): Node is different from node in nodesByReactRootID.' - ); + var id = node[ID_PROP_NAME]; + + if (!id) { + evaluateNodeParents(node); + id = node[ID_PROP_NAME]; + + if (!id) { + return ''; + } } + invariant( + nodesByReactRootID.hasOwnProperty(id), + 'getID(): Element not in node lookup.' + ); + invariant( + nodesByReactRootID[id] === node, + 'getID(): Node is different from node in nodesByReactRootID.' + ); + return id; } @@ -118,15 +128,20 @@ function setID(node, id) { function getNode(id) { var instance = reactDOMInstancesByReactRootID[id]; - invariant(instance, 'awd'); - - var node = getNodeForInstance(instance); - invariant( - nodesByReactRootID.hasOwnProperty(id), - 'getNode(): Node does not exist in nodesByReactRootID.' + instance, + 'getNode(): Instance does not exist in reactDOMInstancesByReactRootID.' ); + if (!nodesByReactRootID.hasOwnProperty(id)) { + evaluateInstanceParents(instance); + + invariant( + nodesByReactRootID.hasOwnProperty(id), + 'getNode(): Node for instance is apparently missing.' + ) + } + var node = nodesByReactRootID[id]; invariant( @@ -174,57 +189,53 @@ function purgeID(id) { delete reactDOMInstancesByReactRootID[id]; if (__DEV__) { - delete firstInstancesForNodeByReactRootID[id]; + delete owningInstancesForNodeByReactRootID[id]; } } -function getHumanReadablePathTo(node) { - var nodes = []; - - while (node && node !== document) { - nodes.push(node); - node = node.parentNode; - } - - var humanReadablePath = []; - - for (var i = 0; node = nodes[i]; i++) { - var parentID = internalGetID(node); - - if (parentID) { - var parentInstance = firstInstancesForNodeByReactRootID[parentID]; - - while (parentInstance._renderedComponent) { - humanReadablePath.push( - parentInstance.displayName || 'UnnamedComponent' - ); - parentInstance = parentInstance._renderedComponent; - } - } - - humanReadablePath.push(node.tagName); - } - - humanReadablePath.reverse(); +/** + * Store reference to a React node component instance by React ID. + * + * @param {ReactComponent} instance The React node component instance. + */ +function registerDOMInstance(instance) { + reactDOMInstancesByReactRootID[instance._rootNodeID] = instance; +} - return humanReadablePath.join(' > '); +/** + * Evaluate an inserted/replaced node/instance pair. This MUST be invoked for + * the React root container instance. Invoking on inserted/replaced + * instances/nodes prevents already evaluated siblings from being potentially + * re-evaluated as a side-effect of the removal of an evaluated sibling. + * + * @param {DOMElement} node The node to store. + * @param {ReactComponent} instance The root component instance. + */ +function evaluateRoot(node, instance) { + evaluateChild(node, instance); } -function mountHierarchy(node, instance) { - evaluateDOMChild(node, instance); - return; - if (__DEV__) { - firstInstancesForNodeByReactRootID[instance._rootNodeID] = instance; - } +/** + * Validate nesting and update the data structures with the resulting + * node/instance pair. + * + * @param {DOMElement} node The node associated with the instance. + * @param {ReactComponent} instance The root component instance for the node. + */ +function evaluateChild(node, instance) { + var rootID = instance._rootNodeID; + var reactDOMInstance = instance; - // Step through and ignore any ReactCompositeComponents - while (instance._renderedComponent) { - instance = instance._renderedComponent; + // Step through ReactCompositeComponents until we encounter + // a ReactDOMComponent or ReactTextComponent, only ReactCompositeComponents + // have _renderedComponent + while (reactDOMInstance._renderedComponent) { + reactDOMInstance = reactDOMInstance._renderedComponent; } - var expectedTagName = instance.tagName; + var expectedTagName = reactDOMInstance.tagName; if (!expectedTagName) { - // Only ReactTextComponent doesn't have a tagName at the moment + // ReactTextComponent doesn't have a tagName at the moment expectedTagName = 'SPAN'; } @@ -244,173 +255,156 @@ function mountHierarchy(node, instance) { false, 'Expected to see a rendered node of type "%s", but the actual ' + 'element is of type "%s". This is most commonly caused by invalid ' + - 'nesting, such as nesting or

,

without a , etc.', - expectedTagName, node.tagName + 'nesting, such as nesting or

,

without a , etc. %s', + expectedTagName, node.tagName, getHumanReadablePathTo(node) ); } - var rootID = instance._rootNodeID; + nodesByReactRootID[rootID] = node; node[ID_PROP_NAME] = rootID; + if (__DEV__) { + owningInstancesForNodeByReactRootID[rootID] = instance; + + // Immediately evaluate all children in DEV to discover any invalid nesting + //evaluateChildren(node, reactDOMInstance); + } + + //return node.nextSibling; +} + +/** + * Evaluate all children node/instance pairs for a parent node/instance pair. + * Does not recursively evaluate children. + * + * @param {DOMElement} node The parent node to evaluate children for. + * @param {ReactComponent} instance The parent root component instance. + */ +function evaluateChildren(node, instance) { var childInstances = instance._renderedChildren; if (!childInstances) { return; } - var childNode = node.firstChild; + //var childNode = node.firstChild; + var childNodes = node.childNodes; for (var id in childInstances) { if (childInstances.hasOwnProperty(id)) { var childInstance = childInstances[id]; - mountHierarchy(childNode, childInstance); - childNode = childNode.nextSibling; + //childNode = + evaluateChild(childNodes[childInstance._mountIndex], childInstance); } } - if (childNode) { - invariant( - false, - 'Expected there to be no more rendered elements, but found a ' + - 'node of type `%s`. This is most commonly caused by invalid ' + - 'nesting, such as nesting or

.', - childNode.tagName - ); - } + //if (childNode) { + // if (__DEV__) { + // console.warn(getHumanReadablePathTo(childNode)); + // } + // + // invariant( + // false, + // 'Expected there to be no more rendered elements, but found a ' + + // 'node of type `%s`. This is most commonly caused by invalid ' + + // 'nesting, such as nesting or

. %s', + // childNode.tagName, getHumanReadablePathTo(childNode) + // ); + //} } -function getNodeForInstance(instance) { - var rootID = instance._rootNodeID; - if (nodesByReactRootID.hasOwnProperty(rootID)) { - return nodesByReactRootID[rootID]; - } - - evaluateToInstance(instance); - - if (nodesByReactRootID.hasOwnProperty(rootID)) { - return nodesByReactRootID[rootID]; - } -} +/** + * Recursively traverse the parents of a node until an evaluated node is + * encountered, if there is one. Then iteratively evaluate all children for each + * parent. + * + * @param {DOMElement} node The node to evaluate parents for. + */ +function evaluateNodeParents(node) { + var parentNode = node.parentNode; -function getInstanceForNode(node) { - var rootID = node[ID_PROP_NAME]; - if (rootID) { - return reactDOMInstancesByReactRootID[rootID]; + if (!parentNode || parentNode.nodeType !== ELEMENT_NODE_TYPE) { + return; } - evaluateToNode(node); + var id = parentNode[ID_PROP_NAME]; - rootID = node[ID_PROP_NAME] - if (rootID) { - return reactDOMInstancesByReactRootID[rootID]; - } -} + if (!id) { + // If evaluation of the parent node did succeed, then it is not owned by + // React and thus no earlier parents can be either, early out all the way. + if (!evaluateNodeParents(parentNode)) { + return false; + } -function evaluateToInstance(instance) { - var rootID = instance._rootNodeID; - var isCached = nodesByReactRootID.hasOwnProperty(rootID); + // As all children have been evaluated, simply checking if our node is now + // evaluated is enough. + id = parentNode[ID_PROP_NAME]; - if (!isCached) { - evaluateToInstance(reactDOMInstancesByReactRootID[instance._parentNodeID]); - isCached = nodesByReactRootID.hasOwnProperty(rootID); - } - if (isCached) { - evaluateDOMChildren(nodesByReactRootID[rootID], instance); - } -} - -function evaluateToNode(node) { - if (!node || node === document) { - return; + if (!id) { + return false; + } } - var rootID = node[ID_PROP_NAME]; - - if (!rootID) { - evaluateToNode(node.parentNode); - rootID = node[ID_PROP_NAME]; - } - if (rootID) { - evaluateDOMChildren(node, reactDOMInstancesByReactRootID[rootID]); - } + evaluateChildren(parentNode, reactDOMInstancesByReactRootID[id]); + return true; } -function evaluateDOMChild(node, rootInstance) { - var leafInstance = rootInstance; - // Step through and ignore any ReactCompositeComponents - while (leafInstance._renderedComponent) { - leafInstance = leafInstance._renderedComponent; - } - - var expectedTagName = leafInstance.tagName; - if (!expectedTagName) { - // Only ReactTextComponent doesn't have a tagName at the moment - expectedTagName = 'SPAN'; - } - - invariant( - node && node.tagName, - 'Expected to see a rendered node of type "%s", but there were no more ' + - 'elements in the DOM.', - expectedTagName - ); +/** + * Recursively traverse the parents of an instance until an evaluated instance + * is encountered, if there is one. Then iteratively evaluate all children for + * for each parent. + * + * @param {ReactComponent} instance The instance to evaluate parents for. + */ +function evaluateInstanceParents(instance) { + var parentInstance = reactDOMInstancesByReactRootID[instance._parentNodeID]; + var id = parentInstance._rootNodeID; - if (node.tagName !== expectedTagName) { - if (__DEV__) { - console.warn(getHumanReadablePathTo(node)); - } + if (!nodesByReactRootID.hasOwnProperty(id)) { + evaluateInstanceParents(parentInstance); + // As all children have been evaluated, if our instance is not evaluated + // now then something is terribly wrong. invariant( - false, - 'Expected to see a rendered node of type "%s", but the actual ' + - 'element is of type "%s". This is most commonly caused by invalid ' + - 'nesting, such as nesting or

,

without a , etc.', - expectedTagName, node.tagName + nodesByReactRootID.hasOwnProperty(id), + 'getNode(): Instance cannot find its node.' ); } - var rootID = rootInstance._rootNodeID; - nodesByReactRootID[rootID] = node; - node[ID_PROP_NAME] = rootID; - - if (__DEV__) { - firstInstancesForNodeByReactRootID[rootID] = rootInstance; - } - - //evaluateDOMChildren(node, leafInstance); - - return node.nextSibling; + evaluateChildren(nodesByReactRootID[id], parentInstance); } -function evaluateDOMChildren(parentNode, parentInstance) { - var childInstances = parentInstance._renderedChildren; +function getHumanReadablePathTo(node) { + var nodes = []; - if (!childInstances) { - return; + while (node && node !== document) { + nodes.push(node); + node = node.parentNode; } - var childNode = parentNode.firstChild; + var humanReadablePath = []; - for (var id in childInstances) { - if (childInstances.hasOwnProperty(id)) { - childNode = evaluateDOMChild(childNode, childInstances[id]); - } - } + for (var i = 0; node = nodes[i]; i++) { + var parentID = internalGetID(node); - if (childNode) { - if (__DEV__) { - console.warn(getHumanReadablePathTo(childNode)); + if (parentID) { + var parentInstance = owningInstancesForNodeByReactRootID[parentID]; + + while (parentInstance._renderedComponent) { + humanReadablePath.push( + parentInstance.constructor.displayName || 'UnnamedComponent' + ); + parentInstance = parentInstance._renderedComponent; + } } - invariant( - false, - 'Expected there to be no more rendered elements, but found a ' + - 'node of type `%s`. This is most commonly caused by invalid ' + - 'nesting, such as nesting or

.', - childNode.tagName - ); + humanReadablePath.push(node.tagName); } + + humanReadablePath.reverse(); + + return humanReadablePath.join(' > '); } /** @@ -583,7 +577,7 @@ var ReactMount = { var reactRootElement = getReactRootElementInContainer(container); var shouldReuseMarkup = ( reactRootElement && !prevComponent && - ReactMarkupChecksum.canReuseNode(reactRootElement) + ReactMarkupChecksum.canReuseRoot(reactRootElement) ); var component = ReactMount._renderNewRootComponent( @@ -769,12 +763,11 @@ var ReactMount = { * React ID utilities. */ - getNodeForInstance: getNodeForInstance, - getInstanceForNode: getInstanceForNode, - getReactRootID: getReactRootID, - mountHierarchy: mountHierarchy, + registerDOMInstance: registerDOMInstance, + + evaluateRoot: evaluateRoot, getID: getID, @@ -782,10 +775,6 @@ var ReactMount = { getNode: getNode, - addDOMInstance: function(instance) { - reactDOMInstancesByReactRootID[instance._rootNodeID] = instance; - }, - purgeID: purgeID }; diff --git a/src/browser/ui/dom/DOMChildrenOperations.js b/src/browser/ui/dom/DOMChildrenOperations.js index 362755e821bc..11a3b3b3795c 100644 --- a/src/browser/ui/dom/DOMChildrenOperations.js +++ b/src/browser/ui/dom/DOMChildrenOperations.js @@ -93,7 +93,7 @@ var DOMChildrenOperations = { dangerouslyReplaceNodeWithMarkup: function(node, markup, instance) { var renderedNode = Danger.dangerouslyReplaceNodeWithMarkup(node, markup); - ReactMount.mountHierarchy(renderedNode, instance); + ReactMount.evaluateRoot(renderedNode, instance); }, /** @@ -152,7 +152,7 @@ var DOMChildrenOperations = { case ReactMultiChildUpdateTypes.INSERT_MARKUP: var markupNode = renderedMarkup[update.markupIndex]; insertChildAt(update.parentNode, markupNode, update.toIndex); - ReactMount.mountHierarchy(markupNode, update.instance); + ReactMount.evaluateRoot(markupNode, update.instance); break; case ReactMultiChildUpdateTypes.MOVE_EXISTING: insertChildAt( diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index ac7e3904ae78..fef6bcd2d6fa 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -30,6 +30,7 @@ var ReactPropTransferer = require('ReactPropTransferer'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactUpdates = require('ReactUpdates'); +var ReactMount = require('ReactMount'); var instantiateReactComponent = require('instantiateReactComponent'); var invariant = require('invariant'); @@ -1130,6 +1131,7 @@ var ReactCompositeComponentMixin = { // These two IDs are actually the same! But nothing should rely on that. var thisID = this._rootNodeID; var prevComponentID = prevComponentInstance._rootNodeID; + var prevComponentNode = ReactMount.getNode(prevComponentInstance._rootNodeID); prevComponentInstance.unmountComponent(); this._renderedComponent = instantiateReactComponent(nextDescriptor); var nextMarkup = this._renderedComponent.mountComponent( @@ -1139,7 +1141,7 @@ var ReactCompositeComponentMixin = { this._mountDepth + 1 ); ReactComponent.BackendIDOperations.dangerouslyReplaceNodeWithMarkupByID( - prevComponentID, + prevComponentNode, nextMarkup, this ); diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index e3a9d2af521a..db99e9dde468 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -841,7 +841,7 @@ describe('ReactCompositeComponent', function() { // , , and both

elements each call // unmountIDFromEnvironment which calls purgeID, for a total of 4. - expect(ReactMount.purgeID.callCount).toBe(4); + expect(ReactMount.purgeID.callCount).toBe(2); }); it('should detect valid CompositeComponent classes', function() { From 815165cc0da831324ed8d735df6f781f6b4ec663 Mon Sep 17 00:00:00 2001 From: Andreas Svensson Date: Thu, 10 Jul 2014 20:34:38 +0200 Subject: [PATCH 4/4] Use Map when available --- src/browser/ui/ReactMount.js | 39 +++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js index 777f594f0743..a63ea55c9e6e 100644 --- a/src/browser/ui/ReactMount.js +++ b/src/browser/ui/ReactMount.js @@ -47,6 +47,9 @@ if (__DEV__) { var ELEMENT_NODE_TYPE = 1; var DOC_NODE_TYPE = 9; +var isNativeMap = + typeof Map === 'function' && Map.toString().indexOf('[native code]'); + /** Mapping from reactRootID to React component instance. */ var instancesByReactRootID = {}; @@ -58,6 +61,26 @@ if (__DEV__) { var rootElementsByReactRootID = {}; } +var internalGetID, internalSetID; + +if (isNativeMap) { + var reactRootIDsByElement = new Map; + + internalGetID = function(node) { + return node && reactRootIDsByElement.get(node) || ''; + }; + internalSetID = function(node, id) { + reactRootIDsByElement.set(node, id); + }; +} else { + internalGetID = function(node) { + return node && node[ID_PROP_NAME] || ''; + }; + internalSetID = function(node, id) { + node[ID_PROP_NAME] = id; + }; +} + /** * @param {DOMElement} container DOM element that may contain a React component. * @return {?string} A "reactRoot" ID, if a React component is rendered. @@ -76,11 +99,11 @@ function getID(node) { return ''; } - var id = node[ID_PROP_NAME]; + var id = internalGetID(node); if (!id) { evaluateNodeParents(node); - id = node[ID_PROP_NAME]; + id = internalGetID(node); if (!id) { return ''; @@ -99,10 +122,6 @@ function getID(node) { return id; } -function internalGetID(node) { - return node && node[ID_PROP_NAME] || ''; -} - /** * Sets the React-specific ID of the given node. * @@ -114,7 +133,7 @@ function setID(node, id) { if (oldID !== id) { delete nodesByReactRootID[oldID]; } - node[ID_PROP_NAME] = id; + internalSetID(node, id); nodesByReactRootID[id] = node; } @@ -262,7 +281,7 @@ function evaluateChild(node, instance) { nodesByReactRootID[rootID] = node; - node[ID_PROP_NAME] = rootID; + internalSetID(node, rootID); if (__DEV__) { owningInstancesForNodeByReactRootID[rootID] = instance; @@ -328,7 +347,7 @@ function evaluateNodeParents(node) { return; } - var id = parentNode[ID_PROP_NAME]; + var id = internalGetID(parentNode); if (!id) { // If evaluation of the parent node did succeed, then it is not owned by @@ -339,7 +358,7 @@ function evaluateNodeParents(node) { // As all children have been evaluated, simply checking if our node is now // evaluated is enough. - id = parentNode[ID_PROP_NAME]; + id = internalGetID(parentNode); if (!id) { return false;