diff --git a/src/renderers/dom/client/utils/DOMChildrenOperations.js b/src/renderers/dom/client/utils/DOMChildrenOperations.js index 00143a9c3f38..c5c796325d08 100644 --- a/src/renderers/dom/client/utils/DOMChildrenOperations.js +++ b/src/renderers/dom/client/utils/DOMChildrenOperations.js @@ -18,7 +18,10 @@ var ReactPerf = require('ReactPerf'); var setInnerHTML = require('setInnerHTML'); var setTextContent = require('setTextContent'); -var invariant = require('invariant'); + +function getNodeAfter(parentNode, node) { + return node ? node.nextSibling : parentNode.firstChild; +} /** * Inserts `childNode` as a child of `parentNode` at the `index`. @@ -28,27 +31,14 @@ var invariant = require('invariant'); * @param {number} index Index at which to insert the child. * @internal */ -function insertChildAt(parentNode, childNode, index) { - // We can rely exclusively on `insertBefore(node, null)` instead of also using +function insertChildAt(parentNode, childNode, referenceNode) { + // We rely exclusively on `insertBefore(node, null)` instead of also using // `appendChild(node)`. (Using `undefined` is not allowed by all browsers so // we are careful to use `null`.) - - // In Safari, .childNodes[index] can return a DOM node with id={index} so we - // use .item() instead which is immune to this bug. (See #3560.) In contrast - // to the spec, IE8 throws an error if index is larger than the list size. - var referenceNode = - index < parentNode.childNodes.length ? - parentNode.childNodes.item(index) : null; - parentNode.insertBefore(childNode, referenceNode); } -function insertLazyTreeChildAt(parentNode, childTree, index) { - // See above. - var referenceNode = - index < parentNode.childNodes.length ? - parentNode.childNodes.item(index) : null; - +function insertLazyTreeChildAt(parentNode, childTree, referenceNode) { DOMLazyTree.insertTreeBefore(parentNode, childTree, referenceNode); } @@ -69,81 +59,21 @@ var DOMChildrenOperations = { * @internal */ processUpdates: function(parentNode, updates) { - var update; - // Mapping from parent IDs to initial child orderings. - var initialChildren = null; - // List of children that will be moved or removed. - var updatedChildren = null; - - var markupList = null; - - for (var i = 0; i < updates.length; i++) { - update = updates[i]; - if (update.type === ReactMultiChildUpdateTypes.MOVE_EXISTING || - update.type === ReactMultiChildUpdateTypes.REMOVE_NODE) { - var updatedIndex = update.fromIndex; - var updatedChild = parentNode.childNodes[updatedIndex]; - - invariant( - updatedChild, - 'processUpdates(): Unable to find child %s of element %s. 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.', - updatedIndex, - parentNode, - ); - - initialChildren = initialChildren || {}; - initialChildren[updatedIndex] = updatedChild; - - updatedChildren = updatedChildren || []; - updatedChildren.push(updatedChild); - } else if (update.type === ReactMultiChildUpdateTypes.INSERT_MARKUP) { - // Replace each HTML string with an index into the markup list - if (typeof update.content === 'string') { - markupList = markupList || []; - update.content = markupList.push(update.markup); - } - } - } - - var renderedMarkup; - if (markupList) { - renderedMarkup = Danger.dangerouslyRenderMarkup(markupList); - } - - // Remove updated children first so that `toIndex` is consistent. - if (updatedChildren) { - for (var j = 0; j < updatedChildren.length; j++) { - parentNode.removeChild(updatedChildren[j]); - } - } - for (var k = 0; k < updates.length; k++) { - update = updates[k]; + var update = updates[k]; switch (update.type) { case ReactMultiChildUpdateTypes.INSERT_MARKUP: - if (renderedMarkup) { - insertChildAt( - parentNode, - renderedMarkup[update.content], - update.toIndex - ); - } else { - insertLazyTreeChildAt( - parentNode, - update.content, - update.toIndex - ); - } + insertLazyTreeChildAt( + parentNode, + update.content, + getNodeAfter(parentNode, update.afterNode) + ); break; case ReactMultiChildUpdateTypes.MOVE_EXISTING: insertChildAt( parentNode, - initialChildren[update.fromIndex], - update.toIndex + update.fromNode, + getNodeAfter(parentNode, update.afterNode) ); break; case ReactMultiChildUpdateTypes.SET_MARKUP: @@ -159,7 +89,7 @@ var DOMChildrenOperations = { ); break; case ReactMultiChildUpdateTypes.REMOVE_NODE: - // Already removed by the for-loop above. + parentNode.removeChild(update.fromNode); break; } } diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index fde61e9b5e0d..2d740164a6b7 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -1009,6 +1009,10 @@ ReactDOMComponent.Mixin = { } }, + getNativeNode: function() { + return getNode(this); + }, + /** * Destroys all event registrations for this instance. Does not remove from * the DOM. That must be done by the parent. @@ -1053,7 +1057,6 @@ ReactDOMComponent.Mixin = { break; } - var nativeNode = getNode(this); this.unmountChildren(); ReactDOMComponentTree.uncacheNode(this); EventPluginHub.deleteAllListeners(this); @@ -1061,7 +1064,6 @@ ReactDOMComponent.Mixin = { this._rootNodeID = null; this._domID = null; this._wrapperState = null; - return nativeNode; }, getPublicInstance: function() { @@ -1078,16 +1080,7 @@ ReactPerf.measureMethods(ReactDOMComponent.Mixin, 'ReactDOMComponent', { assign( ReactDOMComponent.prototype, ReactDOMComponent.Mixin, - ReactMultiChild.Mixin, - { - prepareToManageChildren: function() { - // Before we add, remove, or reorder the children of a node, make sure - // we have references to all of its children so we don't lose them, even - // if nefarious browser plugins add extra nodes to our tree. This could be - // called once per child so it should be fast. - ReactDOMComponentTree.precacheChildNodes(this, getNode(this)); - }, - } + ReactMultiChild.Mixin ); module.exports = ReactDOMComponent; diff --git a/src/renderers/dom/shared/ReactDOMEmptyComponent.js b/src/renderers/dom/shared/ReactDOMEmptyComponent.js index f87015b029c1..e07c44781e88 100644 --- a/src/renderers/dom/shared/ReactDOMEmptyComponent.js +++ b/src/renderers/dom/shared/ReactDOMEmptyComponent.js @@ -57,10 +57,11 @@ assign(ReactDOMEmptyComponent.prototype, { }, receiveComponent: function() { }, + getNativeNode: function() { + return ReactDOMComponentTree.getNodeFromInstance(this); + }, unmountComponent: function() { - var node = ReactDOMComponentTree.getNodeFromInstance(this); ReactDOMComponentTree.uncacheNode(this); - return node; }, }); diff --git a/src/renderers/dom/shared/ReactDOMTextComponent.js b/src/renderers/dom/shared/ReactDOMTextComponent.js index 771792b33dac..63d022bf3351 100644 --- a/src/renderers/dom/shared/ReactDOMTextComponent.js +++ b/src/renderers/dom/shared/ReactDOMTextComponent.js @@ -137,10 +137,13 @@ assign(ReactDOMTextComponent.prototype, { } } }, + + getNativeNode: function() { + return getNode(this); + }, + unmountComponent: function() { - var node = getNode(this); ReactDOMComponentTree.uncacheNode(this); - return node; }, }); diff --git a/src/renderers/shared/reconciler/ReactChildReconciler.js b/src/renderers/shared/reconciler/ReactChildReconciler.js index 77b4dd928d54..8d277f7c04eb 100644 --- a/src/renderers/shared/reconciler/ReactChildReconciler.js +++ b/src/renderers/shared/reconciler/ReactChildReconciler.js @@ -71,6 +71,7 @@ var ReactChildReconciler = { updateChildren: function( prevChildren, nextChildren, + removedNodes, transaction, context) { // We currently don't have a way to track moves here but if we use iterators @@ -79,14 +80,15 @@ var ReactChildReconciler = { // TODO: If nothing has changed, return the prevChildren object so that we // can quickly bailout if nothing has changed. if (!nextChildren && !prevChildren) { - return null; + return; } var name; + var prevChild; for (name in nextChildren) { if (!nextChildren.hasOwnProperty(name)) { continue; } - var prevChild = prevChildren && prevChildren[name]; + prevChild = prevChildren && prevChildren[name]; var prevElement = prevChild && prevChild._currentElement; var nextElement = nextChildren[name]; if (prevChild != null && @@ -97,7 +99,8 @@ var ReactChildReconciler = { nextChildren[name] = prevChild; } else { if (prevChild) { - ReactReconciler.unmountComponent(prevChild, name); + removedNodes[name] = ReactReconciler.getNativeNode(prevChild); + ReactReconciler.unmountComponent(prevChild); } // The child must be instantiated before it's mounted. var nextChildInstance = instantiateReactComponent(nextElement); @@ -108,10 +111,11 @@ var ReactChildReconciler = { for (name in prevChildren) { if (prevChildren.hasOwnProperty(name) && !(nextChildren && nextChildren.hasOwnProperty(name))) { - ReactReconciler.unmountComponent(prevChildren[name]); + prevChild = prevChildren[name]; + removedNodes[name] = ReactReconciler.getNativeNode(prevChild); + ReactReconciler.unmountComponent(prevChild); } } - return nextChildren; }, /** diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js index ac85a8ddf7e7..3c5da9815744 100644 --- a/src/renderers/shared/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/reconciler/ReactCompositeComponent.js @@ -370,6 +370,10 @@ var ReactCompositeComponentMixin = { return markup; }, + getNativeNode: function() { + return ReactReconciler.getNativeNode(this._renderedComponent); + }, + /** * Releases any resources allocated by `mountComponent`. * @@ -384,7 +388,7 @@ var ReactCompositeComponentMixin = { } if (this._renderedComponent) { - var unmountedNativeNode = ReactReconciler.unmountComponent(this._renderedComponent); + ReactReconciler.unmountComponent(this._renderedComponent); this._renderedNodeType = null; this._renderedComponent = null; this._instance = null; @@ -415,7 +419,6 @@ var ReactCompositeComponentMixin = { // TODO: inst.props = null; // TODO: inst.state = null; // TODO: inst.context = null; - return unmountedNativeNode; }, /** @@ -803,7 +806,15 @@ var ReactCompositeComponentMixin = { this._processChildContext(context) ); } else { - var oldNativeNode = ReactReconciler.unmountComponent(prevComponentInstance); + // TODO: This is currently necessary due to the unfortunate caching + // that ReactMount does which makes it exceedingly difficult to unmount + // a set of siblings without accidentally repopulating the node cache (see + // #5151). Once ReactMount no longer stores the nodes by ID, this method + // can go away. + var oldNativeNode = ReactReconciler.getNativeNode(prevComponentInstance); + + ReactReconciler.unmountComponent(prevComponentInstance); + this._renderedNodeType = ReactNodeTypes.getType(nextRenderedElement); this._renderedComponent = this._instantiateReactComponent( nextRenderedElement diff --git a/src/renderers/shared/reconciler/ReactMultiChild.js b/src/renderers/shared/reconciler/ReactMultiChild.js index 3e0781189be4..7a22c76cea8c 100644 --- a/src/renderers/shared/reconciler/ReactMultiChild.js +++ b/src/renderers/shared/reconciler/ReactMultiChild.js @@ -19,6 +19,7 @@ var ReactReconciler = require('ReactReconciler'); var ReactChildReconciler = require('ReactChildReconciler'); var flattenChildren = require('flattenChildren'); +var invariant = require('invariant'); /** * Make an update for markup to be rendered and inserted at a supplied index. @@ -27,13 +28,15 @@ var flattenChildren = require('flattenChildren'); * @param {number} toIndex Destination index. * @private */ -function makeInsertMarkup(markup, toIndex) { +function makeInsertMarkup(markup, afterNode, toIndex) { // NOTE: Null values reduce hidden classes. return { type: ReactMultiChildUpdateTypes.INSERT_MARKUP, content: markup, fromIndex: null, + fromNode: null, toIndex: toIndex, + afterNode: afterNode, }; } @@ -44,13 +47,15 @@ function makeInsertMarkup(markup, toIndex) { * @param {number} toIndex Destination index of the element. * @private */ -function makeMove(fromIndex, toIndex) { +function makeMove(child, afterNode, toIndex) { // NOTE: Null values reduce hidden classes. return { type: ReactMultiChildUpdateTypes.MOVE_EXISTING, content: null, - fromIndex: fromIndex, + fromIndex: child._mountIndex, + fromNode: ReactReconciler.getNativeNode(child), toIndex: toIndex, + afterNode: afterNode, }; } @@ -60,13 +65,15 @@ function makeMove(fromIndex, toIndex) { * @param {number} fromIndex Index of the element to remove. * @private */ -function makeRemove(fromIndex) { +function makeRemove(child, node) { // NOTE: Null values reduce hidden classes. return { type: ReactMultiChildUpdateTypes.REMOVE_NODE, content: null, - fromIndex: fromIndex, + fromIndex: child._mountIndex, + fromNode: node, toIndex: null, + afterNode: null, }; } @@ -82,7 +89,9 @@ function makeSetMarkup(markup) { type: ReactMultiChildUpdateTypes.SET_MARKUP, content: markup, fromIndex: null, + fromNode: null, toIndex: null, + afterNode: null, }; } @@ -98,7 +107,9 @@ function makeTextContent(textContent) { type: ReactMultiChildUpdateTypes.TEXT_CONTENT, content: textContent, fromIndex: null, + fromNode: null, toIndex: null, + afterNode: null, }; } @@ -161,7 +172,13 @@ var ReactMultiChild = { ); }, - _reconcilerUpdateChildren: function(prevChildren, nextNestedChildrenElements, transaction, context) { + _reconcilerUpdateChildren: function( + prevChildren, + nextNestedChildrenElements, + removedNodes, + transaction, + context + ) { var nextChildren; if (__DEV__) { if (this._currentElement) { @@ -171,15 +188,17 @@ var ReactMultiChild = { } finally { ReactCurrentOwner.current = null; } - return ReactChildReconciler.updateChildren( - prevChildren, nextChildren, transaction, context + ReactChildReconciler.updateChildren( + prevChildren, nextChildren, removedNodes, transaction, context ); + return nextChildren; } } nextChildren = flattenChildren(nextNestedChildrenElements); - return ReactChildReconciler.updateChildren( - prevChildren, nextChildren, transaction, context + ReactChildReconciler.updateChildren( + prevChildren, nextChildren, removedNodes, transaction, context ); + return nextChildren; }, /** @@ -224,15 +243,13 @@ var ReactMultiChild = { var prevChildren = this._renderedChildren; // Remove any rendered children. ReactChildReconciler.unmountChildren(prevChildren); - // TODO: The setTextContent operation should be enough - var updates = []; for (var name in prevChildren) { if (prevChildren.hasOwnProperty(name)) { - updates.push(this._unmountChild(prevChildren[name])); + invariant(false, 'updateTextContent called on non-empty component.'); } } // Set new text content. - updates.push(makeTextContent(nextContent)); + var updates = [makeTextContent(nextContent)]; processQueue(this, updates); }, @@ -246,13 +263,12 @@ var ReactMultiChild = { var prevChildren = this._renderedChildren; // Remove any rendered children. ReactChildReconciler.unmountChildren(prevChildren); - var updates = []; for (var name in prevChildren) { if (prevChildren.hasOwnProperty(name)) { - updates.push(this._unmountChild(prevChildren[name])); + invariant(false, 'updateTextContent called on non-empty component.'); } } - updates.push(makeSetMarkup(nextMarkup)); + var updates = [makeSetMarkup(nextMarkup)]; processQueue(this, updates); }, @@ -276,8 +292,13 @@ var ReactMultiChild = { */ _updateChildren: function(nextNestedChildrenElements, transaction, context) { var prevChildren = this._renderedChildren; + var removedNodes = {}; var nextChildren = this._reconcilerUpdateChildren( - prevChildren, nextNestedChildrenElements, transaction, context + prevChildren, + nextNestedChildrenElements, + removedNodes, + transaction, + context ); if (!nextChildren && !prevChildren) { return; @@ -288,6 +309,7 @@ var ReactMultiChild = { // `lastIndex` will be the last index visited in `prevChildren`. var lastIndex = 0; var nextIndex = 0; + var lastPlacedNode = null; for (name in nextChildren) { if (!nextChildren.hasOwnProperty(name)) { continue; @@ -297,7 +319,7 @@ var ReactMultiChild = { if (prevChild === nextChild) { updates = enqueue( updates, - this.moveChild(prevChild, nextIndex, lastIndex) + this.moveChild(prevChild, lastPlacedNode, nextIndex, lastIndex) ); lastIndex = Math.max(prevChild._mountIndex, lastIndex); prevChild._mountIndex = nextIndex; @@ -305,28 +327,33 @@ var ReactMultiChild = { if (prevChild) { // Update `lastIndex` before `_mountIndex` gets unset by unmounting. lastIndex = Math.max(prevChild._mountIndex, lastIndex); - updates = enqueue(updates, this._unmountChild(prevChild)); + // The `removedNodes` loop below will actually remove the child. } // The child must be instantiated before it's mounted. updates = enqueue( updates, - this._mountChildAtIndex(nextChild, nextIndex, transaction, context) + this._mountChildAtIndex( + nextChild, + lastPlacedNode, + nextIndex, + transaction, + context + ) ); } nextIndex++; + lastPlacedNode = ReactReconciler.getNativeNode(nextChild); } // Remove children that are no longer present. - for (name in prevChildren) { - if (prevChildren.hasOwnProperty(name) && - !(nextChildren && nextChildren.hasOwnProperty(name))) { + for (name in removedNodes) { + if (removedNodes.hasOwnProperty(name)) { updates = enqueue( updates, - this._unmountChild(prevChildren[name]) + this._unmountChild(prevChildren[name], removedNodes[name]) ); } } if (updates) { - this.prepareToManageChildren(); processQueue(this, updates); } this._renderedChildren = nextChildren; @@ -334,7 +361,8 @@ var ReactMultiChild = { /** * Unmounts all rendered children. This should be used to clean up children - * when this component is unmounted. + * when this component is unmounted. It does not actually perform any + * backend operations. * * @internal */ @@ -344,14 +372,6 @@ var ReactMultiChild = { this._renderedChildren = null; }, - /** - * Hook used by the DOM implementation to precache the nodes before we apply - * any reorders here. - */ - prepareToManageChildren: function() { - // TODO: This sucks. Figure out a better design here. - }, - /** * Moves a child component to the supplied index. * @@ -360,12 +380,12 @@ var ReactMultiChild = { * @param {number} lastIndex Last index visited of the siblings of `child`. * @protected */ - moveChild: function(child, toIndex, lastIndex) { + moveChild: function(child, afterNode, toIndex, lastIndex) { // If the index of `child` is less than `lastIndex`, then it needs to // be moved. Otherwise, we do not need to move it because a child will be // inserted or moved before `child`. if (child._mountIndex < lastIndex) { - return makeMove(child._mountIndex, toIndex); + return makeMove(child, afterNode, toIndex); } }, @@ -376,8 +396,8 @@ var ReactMultiChild = { * @param {string} mountImage Markup to insert. * @protected */ - createChild: function(child, mountImage) { - return makeInsertMarkup(mountImage, child._mountIndex); + createChild: function(child, afterNode, mountImage) { + return makeInsertMarkup(mountImage, afterNode, child._mountIndex); }, /** @@ -386,8 +406,8 @@ var ReactMultiChild = { * @param {ReactComponent} child Child to remove. * @protected */ - removeChild: function(child) { - return makeRemove(child._mountIndex); + removeChild: function(child, node) { + return makeRemove(child, node); }, /** @@ -403,6 +423,7 @@ var ReactMultiChild = { */ _mountChildAtIndex: function( child, + afterNode, index, transaction, context) { @@ -414,7 +435,7 @@ var ReactMultiChild = { context ); child._mountIndex = index; - return this.createChild(child, mountImage); + return this.createChild(child, afterNode, mountImage); }, /** @@ -425,8 +446,8 @@ var ReactMultiChild = { * @param {ReactComponent} child Component to unmount. * @private */ - _unmountChild: function(child) { - var update = this.removeChild(child); + _unmountChild: function(child, node) { + var update = this.removeChild(child, node); child._mountIndex = null; return update; }, diff --git a/src/renderers/shared/reconciler/ReactReconciler.js b/src/renderers/shared/reconciler/ReactReconciler.js index 31c167e9fca5..4e1a1d3934ab 100644 --- a/src/renderers/shared/reconciler/ReactReconciler.js +++ b/src/renderers/shared/reconciler/ReactReconciler.js @@ -54,6 +54,14 @@ var ReactReconciler = { return markup; }, + /** + * Returns a value that can be passed to + * ReactComponentEnvironment.replaceNodeWithMarkup. + */ + getNativeNode: function(internalInstance) { + return internalInstance.getNativeNode(); + }, + /** * Releases any resources allocated by `mountComponent`. * diff --git a/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js b/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js index 8fe0a209b100..2a886be3be22 100644 --- a/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js +++ b/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js @@ -38,6 +38,9 @@ assign(ReactSimpleEmptyComponent.prototype, { }, receiveComponent: function() { }, + getNativeNode: function() { + return ReactReconciler.getNativeNode(this._renderedComponent); + }, unmountComponent: function() { ReactReconciler.unmountComponent(this._renderedComponent); this._renderedComponent = null; diff --git a/src/renderers/shared/reconciler/instantiateReactComponent.js b/src/renderers/shared/reconciler/instantiateReactComponent.js index eabe39972e8e..4ddf8e861fe5 100644 --- a/src/renderers/shared/reconciler/instantiateReactComponent.js +++ b/src/renderers/shared/reconciler/instantiateReactComponent.js @@ -104,6 +104,7 @@ function instantiateReactComponent(node) { typeof instance.construct === 'function' && typeof instance.mountComponent === 'function' && typeof instance.receiveComponent === 'function' && + typeof instance.getNativeNode === 'function' && typeof instance.unmountComponent === 'function', 'Only React Components can be mounted.' ); diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index b1a525c92a66..0855f6da26e9 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -381,6 +381,10 @@ NoopInternalComponent.prototype = { this._currentElement = element; }, + getNativeNode: function() { + return undefined; + }, + unmountComponent: function() { },