diff --git a/src/renderers/dom/client/ReactDOMIDOperations.js b/src/renderers/dom/client/ReactDOMIDOperations.js index d8630ce0a51b..7b419e2bf966 100644 --- a/src/renderers/dom/client/ReactDOMIDOperations.js +++ b/src/renderers/dom/client/ReactDOMIDOperations.js @@ -63,19 +63,6 @@ var ReactDOMIDOperations = { } }, - /** - * Replaces a DOM node that exists in the document with markup. - * - * @param {string} id ID of child to be replaced. - * @param {string} markup Dangerous markup to inject in place of child. - * @internal - * @see {Danger.dangerouslyReplaceNodeWithMarkup} - */ - dangerouslyReplaceNodeWithMarkupByID: function(id, markup) { - var node = ReactMount.getNode(id); - DOMChildrenOperations.dangerouslyReplaceNodeWithMarkup(node, markup); - }, - /** * Updates a component's children by processing a series of updates. * @@ -92,7 +79,6 @@ var ReactDOMIDOperations = { }; ReactPerf.measureMethods(ReactDOMIDOperations, 'ReactDOMIDOperations', { - dangerouslyReplaceNodeWithMarkupByID: 'dangerouslyReplaceNodeWithMarkupByID', dangerouslyProcessChildrenUpdates: 'dangerouslyProcessChildrenUpdates', }); diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 2a231a420b0b..8ab20475fe8c 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -100,7 +100,7 @@ function getReactRootElementInContainer(container) { */ function getReactRootID(container) { var rootElement = getReactRootElementInContainer(container); - return rootElement && ReactMount.getID(rootElement); + return rootElement && internalGetID(rootElement); } /** @@ -116,20 +116,12 @@ function getReactRootID(container) { 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; - } + invariant( + !nodeCache.hasOwnProperty(id) || nodeCache[id] === node, + 'ReactMount: Two unequal nodes with the same `%s`: %s', + ATTR_NAME, id + ); + nodeCache[id] = node; } return id; @@ -157,6 +149,25 @@ function setID(node, id) { nodeCache[id] = node; } +/** + * Finds the node with the supplied ID if present in the cache. + */ +function getNodeIfCached(id) { + var node = nodeCache[id]; + // TODO: Since this "isValid" business is now just a sanity check, we can + // probably drop it with no consequences. + invariant( + !node || isValid(node, id), + 'ReactMount: Cached node with `%s`: %s is missing from the document. ' + + 'This probably means the DOM was unexpectedly mutated -- when removing ' + + 'React-rendered children from the DOM, rerender without those children ' + + 'or call ReactDOM.unmountComponentAtNode on the container to unmount an ' + + 'entire subtree.', + ATTR_NAME, id + ); + return node; +} + /** * Finds the node with the supplied React-generated DOM ID. * @@ -165,10 +176,12 @@ function setID(node, id) { * @internal */ function getNode(id) { - if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { - nodeCache[id] = ReactMount.findReactNodeByID(id); + var node = getNodeIfCached(id); + if (node) { + return node; + } else { + return nodeCache[id] = ReactMount.findReactNodeByID(id); } - return nodeCache[id]; } /** @@ -183,10 +196,7 @@ function getNodeFromInstance(instance) { if (ReactEmptyComponentRegistry.isNullComponentID(id)) { return null; } - if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { - nodeCache[id] = ReactMount.findReactNodeByID(id); - } - return nodeCache[id]; + return getNode(id); } /** @@ -227,8 +237,8 @@ function purgeID(id) { var deepestNodeSoFar = null; function findDeepestCachedAncestorImpl(ancestorID) { - var ancestor = nodeCache[ancestorID]; - if (ancestor && isValid(ancestor, ancestorID)) { + var ancestor = getNodeIfCached(ancestorID); + if (ancestor) { deepestNodeSoFar = ancestor; } else { // This node isn't populated in the cache, so presumably none of its @@ -724,19 +734,16 @@ var ReactMount = { * @return {string} The "reactRoot" ID of elements rendered within. */ registerContainer: function(container) { - var reactRootID = getReactRootID(container); - if (reactRootID) { - // If one exists, make sure it is a valid "reactRoot" ID. - reactRootID = ReactInstanceHandles.getReactRootIDFromNodeID(reactRootID); - } - if (!reactRootID) { + var id = getReactRootID(container); + // If one exists, make sure it is a valid "reactRoot" ID. + if (!id || id !== ReactInstanceHandles.getReactRootIDFromNodeID(id)) { // No valid "reactRoot" ID found, create one. - reactRootID = ReactInstanceHandles.createReactRootID( + id = ReactInstanceHandles.createReactRootID( ClientReactRootIndex.createReactRootIndex() ); } - containersByReactRootID[reactRootID] = container; - return reactRootID; + containersByReactRootID[id] = container; + return id; }, /** diff --git a/src/renderers/dom/shared/ReactComponentBrowserEnvironment.js b/src/renderers/dom/shared/ReactComponentBrowserEnvironment.js index e1ee6fc66b4c..3f31a52a2174 100644 --- a/src/renderers/dom/shared/ReactComponentBrowserEnvironment.js +++ b/src/renderers/dom/shared/ReactComponentBrowserEnvironment.js @@ -11,8 +11,10 @@ 'use strict'; +var DOMChildrenOperations = require('DOMChildrenOperations'); var ReactDOMIDOperations = require('ReactDOMIDOperations'); var ReactMount = require('ReactMount'); +var ReactPerf = require('ReactPerf'); /** * Abstracts away all functionality of the reconciler that requires knowledge of @@ -24,8 +26,8 @@ var ReactComponentBrowserEnvironment = { processChildrenUpdates: ReactDOMIDOperations.dangerouslyProcessChildrenUpdates, - replaceNodeWithMarkupByID: - ReactDOMIDOperations.dangerouslyReplaceNodeWithMarkupByID, + replaceNodeWithMarkup: + DOMChildrenOperations.dangerouslyReplaceNodeWithMarkup, /** * If a particular environment requires that some resources be cleaned up, @@ -40,4 +42,12 @@ var ReactComponentBrowserEnvironment = { }; +ReactPerf.measureMethods( + ReactComponentBrowserEnvironment, + 'ReactComponentBrowserEnvironment', + { + replaceNodeWithMarkup: 'replaceNodeWithMarkup', + } +); + module.exports = ReactComponentBrowserEnvironment; diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index 4e30c5f69994..827f06d945c9 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -357,7 +357,7 @@ function trapBubbledEventsLocal() { // If a component renders to null or if another component fatals and causes // the state of the tree to be corrupted, `node` here can be null. invariant(inst._rootNodeID, 'Must be mounted to trap events'); - var node = ReactMount.getNode(inst._rootNodeID); + var node = getNode(inst); invariant( node, 'trapBubbledEvent(...): Requires node to be rendered.' @@ -493,6 +493,14 @@ function isCustomComponent(tagName, props) { return tagName.indexOf('-') >= 0 || props.is != null; } +function getNode(inst) { + if (inst._nativeNode) { + return inst._nativeNode; + } else { + return inst._nativeNode = ReactMount.getNode(inst._rootNodeID); + } +} + /** * Creates a new React class that is idempotent and capable of containing other * React components. It accepts event listeners and DOM properties that are @@ -513,10 +521,11 @@ function ReactDOMComponent(tag) { this._renderedChildren = null; this._previousStyle = null; this._previousStyleCopy = null; + this._nativeNode = null; this._rootNodeID = null; this._wrapperState = null; this._topLevelWrapper = null; - this._nodeWithLegacyProperties = null; + this._nodeHasLegacyProperties = false; if (__DEV__) { this._unprocessedContextDev = null; this._processedContextDev = null; @@ -600,10 +609,11 @@ ReactDOMComponent.Mixin = { if (transaction.useCreateElement) { var ownerDocument = context[ReactMount.ownerDocumentContextKey]; var el = ownerDocument.createElement(this._currentElement.type); + this._nativeNode = el; DOMPropertyOperations.setAttributeForID(el, this._rootNodeID); // Populate node cache ReactMount.getID(el); - this._updateDOMProperties({}, props, transaction, el); + this._updateDOMProperties({}, props, transaction); this._createInitialChildren(transaction, props, context, el); mountImage = el; } else { @@ -844,7 +854,7 @@ ReactDOMComponent.Mixin = { assertValidProps(this, nextProps); - this._updateDOMProperties(lastProps, nextProps, transaction, null); + this._updateDOMProperties(lastProps, nextProps, transaction); this._updateDOMChildren( lastProps, nextProps, @@ -852,8 +862,8 @@ ReactDOMComponent.Mixin = { context ); - if (!canDefineProperty && this._nodeWithLegacyProperties) { - this._nodeWithLegacyProperties.props = nextProps; + if (!canDefineProperty && this._nodeHasLegacyProperties) { + this._nativeNode.props = nextProps; } if (this._tag === 'select') { @@ -877,10 +887,9 @@ ReactDOMComponent.Mixin = { * @private * @param {object} lastProps * @param {object} nextProps - * @param {ReactReconcileTransaction} transaction * @param {?DOMElement} node */ - _updateDOMProperties: function(lastProps, nextProps, transaction, node) { + _updateDOMProperties: function(lastProps, nextProps, transaction) { var propKey; var styleName; var styleUpdates; @@ -908,10 +917,7 @@ ReactDOMComponent.Mixin = { } else if ( DOMProperty.properties[propKey] || DOMProperty.isCustomAttribute(propKey)) { - if (!node) { - node = ReactMount.getNode(this._rootNodeID); - } - DOMPropertyOperations.deleteValueForProperty(node, propKey); + DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); } } for (propKey in nextProps) { @@ -964,20 +970,15 @@ ReactDOMComponent.Mixin = { deleteListener(this._rootNodeID, propKey); } } else if (isCustomComponent(this._tag, nextProps)) { - if (!node) { - node = ReactMount.getNode(this._rootNodeID); - } DOMPropertyOperations.setValueForAttribute( - node, + getNode(this), propKey, nextProp ); } else if ( DOMProperty.properties[propKey] || DOMProperty.isCustomAttribute(propKey)) { - if (!node) { - node = ReactMount.getNode(this._rootNodeID); - } + var node = getNode(this); // If we're updating to null or undefined, we should remove the property // from the DOM node instead of inadvertantly setting to a string. This // brings us in line with the same behavior we have on initial render. @@ -989,10 +990,7 @@ ReactDOMComponent.Mixin = { } } if (styleUpdates) { - if (!node) { - node = ReactMount.getNode(this._rootNodeID); - } - CSSPropertyOperations.setValueForStyles(node, styleUpdates); + CSSPropertyOperations.setValueForStyles(getNode(this), styleUpdates); } }, @@ -1089,21 +1087,26 @@ ReactDOMComponent.Mixin = { break; } + var nativeNode = getNode(this); + this._nativeNode = null; + if (this._nodeHasLegacyProperties) { + nativeNode._reactInternalComponent = null; + } + this.unmountChildren(); ReactBrowserEventEmitter.deleteAllListeners(this._rootNodeID); ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID); this._rootNodeID = null; this._wrapperState = null; - if (this._nodeWithLegacyProperties) { - var node = this._nodeWithLegacyProperties; - node._reactInternalComponent = null; - this._nodeWithLegacyProperties = null; - } + + return nativeNode; }, getPublicInstance: function() { - if (!this._nodeWithLegacyProperties) { - var node = ReactMount.getNode(this._rootNodeID); + if (this._nodeHasLegacyProperties) { + return this._nativeNode; + } else { + var node = getNode(this); node._reactInternalComponent = this; node.getDOMNode = legacyGetDOMNode; @@ -1126,9 +1129,9 @@ ReactDOMComponent.Mixin = { node.props = this._currentElement.props; } - this._nodeWithLegacyProperties = node; + this._nodeHasLegacyProperties = true; + return node; } - return this._nodeWithLegacyProperties; }, }; diff --git a/src/renderers/dom/shared/ReactDOMTextComponent.js b/src/renderers/dom/shared/ReactDOMTextComponent.js index 7212058905ed..9aab346986d1 100644 --- a/src/renderers/dom/shared/ReactDOMTextComponent.js +++ b/src/renderers/dom/shared/ReactDOMTextComponent.js @@ -52,6 +52,7 @@ assign(ReactDOMTextComponent.prototype, { // TODO: This is really a ReactText (ReactNode), not a ReactElement this._currentElement = text; this._stringText = '' + text; + this._nativeNode = null; // Properties this._rootNodeID = null; @@ -82,6 +83,7 @@ assign(ReactDOMTextComponent.prototype, { if (transaction.useCreateElement) { var ownerDocument = context[ReactMount.ownerDocumentContextKey]; var el = ownerDocument.createElement('span'); + this._nativeNode = el; DOMPropertyOperations.setAttributeForID(el, rootID); // Populate node cache ReactMount.getID(el); @@ -121,14 +123,20 @@ assign(ReactDOMTextComponent.prototype, { // and/or updateComponent to do the actual update for consistency with // other component types? this._stringText = nextStringText; - var node = ReactMount.getNode(this._rootNodeID); + var node = this._nativeNode; + if (!node) { + node = this._nativeNode = ReactMount.getNode(this._rootNodeID); + } DOMChildrenOperations.updateTextContent(node, nextStringText); } } }, unmountComponent: function() { + var node = this._nativeNode || ReactMount.getNode(this._rootNodeID); + this._nativeNode = null; ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID); + return node; }, }); diff --git a/src/renderers/shared/reconciler/ReactComponentEnvironment.js b/src/renderers/shared/reconciler/ReactComponentEnvironment.js index 91359f2bbe64..274f53f3b469 100644 --- a/src/renderers/shared/reconciler/ReactComponentEnvironment.js +++ b/src/renderers/shared/reconciler/ReactComponentEnvironment.js @@ -28,7 +28,7 @@ var ReactComponentEnvironment = { * Optionally injectable hook for swapping out mount images in the middle of * the tree. */ - replaceNodeWithMarkupByID: null, + replaceNodeWithMarkup: null, /** * Optionally injectable hook for processing a queue of child updates. Will @@ -44,8 +44,8 @@ var ReactComponentEnvironment = { ); ReactComponentEnvironment.unmountIDFromEnvironment = environment.unmountIDFromEnvironment; - ReactComponentEnvironment.replaceNodeWithMarkupByID = - environment.replaceNodeWithMarkupByID; + ReactComponentEnvironment.replaceNodeWithMarkup = + environment.replaceNodeWithMarkup; ReactComponentEnvironment.processChildrenUpdates = environment.processChildrenUpdates; injected = true; diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js index fb570f7fe3d6..c7c9a8c3542a 100644 --- a/src/renderers/shared/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/reconciler/ReactCompositeComponent.js @@ -310,7 +310,8 @@ var ReactCompositeComponentMixin = { inst.componentWillUnmount(); } - ReactReconciler.unmountComponent(this._renderedComponent); + var unmountedNativeNode = + ReactReconciler.unmountComponent(this._renderedComponent); this._renderedComponent = null; this._instance = null; @@ -339,6 +340,7 @@ var ReactCompositeComponentMixin = { // TODO: inst.props = null; // TODO: inst.state = null; // TODO: inst.context = null; + return unmountedNativeNode; }, /** @@ -727,30 +729,30 @@ var ReactCompositeComponentMixin = { this._processChildContext(context) ); } else { - // These two IDs are actually the same! But nothing should rely on that. - var thisID = this._rootNodeID; - var prevComponentID = prevComponentInstance._rootNodeID; - ReactReconciler.unmountComponent(prevComponentInstance); + var oldNativeNode = + ReactReconciler.unmountComponent(prevComponentInstance); this._renderedComponent = this._instantiateReactComponent( nextRenderedElement ); var nextMarkup = ReactReconciler.mountComponent( this._renderedComponent, - thisID, + this._rootNodeID, transaction, this._processChildContext(context) ); - this._replaceNodeWithMarkupByID(prevComponentID, nextMarkup); + this._replaceNodeWithMarkup(oldNativeNode, nextMarkup); } }, /** + * Overridden in shallow rendering. + * * @protected */ - _replaceNodeWithMarkupByID: function(prevComponentID, nextMarkup) { - ReactComponentEnvironment.replaceNodeWithMarkupByID( - prevComponentID, + _replaceNodeWithMarkup: function(oldNativeNode, nextMarkup) { + ReactComponentEnvironment.replaceNodeWithMarkup( + oldNativeNode, nextMarkup ); }, diff --git a/src/renderers/shared/reconciler/ReactEmptyComponent.js b/src/renderers/shared/reconciler/ReactEmptyComponent.js index 715dece5f1b5..1829265b9c03 100644 --- a/src/renderers/shared/reconciler/ReactEmptyComponent.js +++ b/src/renderers/shared/reconciler/ReactEmptyComponent.js @@ -46,10 +46,11 @@ assign(ReactEmptyComponent.prototype, { receiveComponent: function() { }, unmountComponent: function(rootID, transaction, context) { - ReactReconciler.unmountComponent(this._renderedComponent); + var nativeNode = ReactReconciler.unmountComponent(this._renderedComponent); ReactEmptyComponentRegistry.deregisterNullComponentID(this._rootNodeID); this._rootNodeID = null; this._renderedComponent = null; + return nativeNode; }, }); diff --git a/src/renderers/shared/reconciler/ReactReconciler.js b/src/renderers/shared/reconciler/ReactReconciler.js index ab3fbb0f6c14..4e50d387b3b9 100644 --- a/src/renderers/shared/reconciler/ReactReconciler.js +++ b/src/renderers/shared/reconciler/ReactReconciler.js @@ -50,7 +50,7 @@ var ReactReconciler = { */ unmountComponent: function(internalInstance) { ReactRef.detachRefs(internalInstance, internalInstance._currentElement); - internalInstance.unmountComponent(); + return internalInstance.unmountComponent(); }, /** diff --git a/src/renderers/shared/reconciler/__tests__/ReactInstanceHandles-test.js b/src/renderers/shared/reconciler/__tests__/ReactInstanceHandles-test.js index f5dbc051ffc8..7cb1ba9d95fc 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactInstanceHandles-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactInstanceHandles-test.js @@ -76,72 +76,84 @@ describe('ReactInstanceHandles', function() { 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'); + var parentNode = ReactTestUtils.renderIntoDocument( +
+
+ {[
]} +
+ ); + var childNodeB = parentNode.childNodes[1]; expect( - ReactMount.findComponentRoot( - parentNode, - ReactMount.getID(childNodeB) + ReactMount.getNode( + getNodeID(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); + var parentNode = ReactTestUtils.renderIntoDocument( +
+ {[
]} +
+ ); + var childNodeB = parentNode.childNodes[0]; - ReactMount.setID(parentNode, '.0'); // No ID on `childNodeA`. - ReactMount.setID(childNodeB, '.0.0:1'); + var childNodeA = document.createElement('div'); + parentNode.insertBefore(childNodeA, childNodeB); expect( - ReactMount.findComponentRoot( - parentNode, - ReactMount.getID(childNodeB) + ReactMount.getNode( + getNodeID(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'); + spyOn(console, 'error'); + var parentNode = ReactTestUtils.renderIntoDocument( + + +
+ ); + var childNodeA = parentNode.childNodes[0]; + var childNodeB; + if (childNodeA.tagName === 'TR') { + childNodeB = childNodeA; + // No ID on `childNodeA`, it was "rendered by the browser". + childNodeA = document.createElement('tbody'); + childNodeA.appendChild(childNodeB); + parentNode.appendChild(childNodeA); + } else { + childNodeB = childNodeA.childNodes[0]; + } + expect(childNodeA.tagName).toBe('TBODY'); - expect(ReactMount.findComponentRoot( - parentNode, - ReactMount.getID(childNodeB) - )).toBe(childNodeB); + expect( + ReactMount.getNode( + getNodeID(childNodeB) + ) + ).toBe(childNodeB); + var junkID = getNodeID(childNodeB) + ':junk'; expect(function() { - ReactMount.findComponentRoot( - parentNode, - ReactMount.getID(childNodeB) + ':junk' + ReactMount.getNode( + junkID ); }).toThrow( - 'Invariant Violation: findComponentRoot(..., .0.1:0:junk): ' + + 'Invariant Violation: findComponentRoot(..., ' + junkID + '): ' + '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, nesting tags ' + 'like
,

, or , or using non-SVG elements in an ' + 'parent. ' + - 'Try inspecting the child nodes of the element with React ID `.0`.' + 'Try inspecting the child nodes of the element with React ID ``.' + ); + + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toContain( + '
cannot appear as a child of
' ); }); }); diff --git a/src/test/ReactDefaultPerf.js b/src/test/ReactDefaultPerf.js index 8930573461b1..22b613a11d58 100644 --- a/src/test/ReactDefaultPerf.js +++ b/src/test/ReactDefaultPerf.js @@ -169,7 +169,8 @@ var ReactDefaultPerf = { moduleName === 'ReactDOMIDOperations' || moduleName === 'CSSPropertyOperations' || moduleName === 'DOMChildrenOperations' || - moduleName === 'DOMPropertyOperations') { + moduleName === 'DOMPropertyOperations' || + moduleName === 'ReactComponentBrowserEnvironment') { start = performanceNow(); rv = func.apply(this, args); totalTime = performanceNow() - start; diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index b28de910029d..2782df32da10 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -395,7 +395,7 @@ assign( _instantiateReactComponent: function(element) { return new NoopInternalComponent(element); }, - _replaceNodeWithMarkupByID: function() {}, + _replaceNodeWithMarkup: function() {}, _renderValidatedComponent: ReactCompositeComponent.Mixin ._renderValidatedComponentWithoutOwnerOrContext,