Merge pull request facebook#5782 from spicyj/img

Move nodes around by reference instead of by index

Author: sophiebits

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 <tbody> when using tables, ' +
-          'nesting tags like <form>, <p>, or <a>, or using non-SVG elements ' +
-          'in an <svg> 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;
       }
     }

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,15 +1057,13 @@ ReactDOMComponent.Mixin = {
         break;
     }
 
-    var nativeNode = getNode(this);
     this.unmountChildren();
     ReactDOMComponentTree.uncacheNode(this);
     EventPluginHub.deleteAllListeners(this);
     ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID);
     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;

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;
   },
 });
 

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;
   },
 
 });

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;
   },
 
   /**

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