Resolve refs in the order of the children (#7101)

* Resolve refs in the order of the children

React makes no guarantees about ref resolution order. Unfortunately, some of the internal Facebook component APIs (specifically, layer dialogs) currently depend on the ref resolution order. Specifically, the assumption is that if the layer dialog is placed as a last child, by the time it mounts or updates, the refs to any previously declared elements have been resolved.

With the current `ReactMultiChild`, this is *usually* the case but not always. Both initial mount and an update of all components satisfy this assumption: by the time a child mounts or updates, the previous children’s refs have been resolved. The one scenario where it isn’t true is when **a new child is mounted during an update**.

In this case, the `mountComponent()` call used to be delayed until `ReactMultiChild` processes the queue. However, this is inconsistent with how updates normally work: unlike mounting, updating and unmounting happens inside `ReactChildReconciler.updateChildren()` loop.

This PR changes the `mountComponent()` to be performed inside `ReactChildReconciler`, just like `receiveComponent()` and `unmountComponent()`, and thus ensures that `attachRef()` calls are enqueued in the order the children were processed, so by the time the next child flushes, the refs of the previous children have been resolved.

This is not ideal and will probably be broken by incremental reconciler in the future. However, since we are trying to get rid of mixins in the internal codebase, and layered components are one of the biggest blockers to that, it’s lesser evil to temporarily make ref resolution order more strict until we have time to fix up the layer APIs to not rely on it, and are able to relax it again (which would be a breaking change).

* Use array instead of object to avoid lookups

(cherry picked from commit 83cbc3e)

src/renderers/shared/stack/reconciler/ReactChildReconciler.js

@@ -93,8 +93,11 @@ var ReactChildReconciler = {
   updateChildren: function(
     prevChildren,
     nextChildren,
+    mountImages,
     removedNodes,
     transaction,
+    hostParent,
+    hostContainerInfo,
     context) {
     // We currently don't have a way to track moves here but if we use iterators
     // instead of for..in we can zip the iterators and check if an item has
@@ -127,6 +130,16 @@ var ReactChildReconciler = {
         // The child must be instantiated before it's mounted.
         var nextChildInstance = instantiateReactComponent(nextElement, true);
         nextChildren[name] = nextChildInstance;
+        // Creating mount image now ensures refs are resolved in right order
+        // (see https://github.com/facebook/react/pull/7101 for explanation).
+        var nextChildMountImage = ReactReconciler.mountComponent(
+          nextChildInstance,
+          transaction,
+          hostParent,
+          hostContainerInfo,
+          context
+        );
+        mountImages.push(nextChildMountImage);
       }
     }
     // Unmount children that are no longer present.

src/renderers/shared/stack/reconciler/ReactMultiChild.js

@@ -212,6 +212,7 @@ var ReactMultiChild = {
     _reconcilerUpdateChildren: function(
       prevChildren,
       nextNestedChildrenElements,
+      mountImages,
       removedNodes,
       transaction,
       context
@@ -226,14 +227,28 @@ var ReactMultiChild = {
             ReactCurrentOwner.current = null;
           }
           ReactChildReconciler.updateChildren(
-            prevChildren, nextChildren, removedNodes, transaction, context
+            prevChildren,
+            nextChildren,
+            mountImages,
+            removedNodes,
+            transaction,
+            this,
+            this._hostContainerInfo,
+            context
           );
           return nextChildren;
         }
       }
       nextChildren = flattenChildren(nextNestedChildrenElements);
       ReactChildReconciler.updateChildren(
-        prevChildren, nextChildren, removedNodes, transaction, context
+        prevChildren,
+        nextChildren,
+        mountImages,
+        removedNodes,
+        transaction,
+        this,
+        this._hostContainerInfo,
+        context
       );
       return nextChildren;
     },
@@ -339,9 +354,11 @@ var ReactMultiChild = {
     _updateChildren: function(nextNestedChildrenElements, transaction, context) {
       var prevChildren = this._renderedChildren;
       var removedNodes = {};
+      var mountImages = [];
       var nextChildren = this._reconcilerUpdateChildren(
         prevChildren,
         nextNestedChildrenElements,
+        mountImages,
         removedNodes,
         transaction,
         context
@@ -353,8 +370,10 @@ var ReactMultiChild = {
       var name;
       // `nextIndex` will increment for each child in `nextChildren`, but
       // `lastIndex` will be the last index visited in `prevChildren`.
-      var lastIndex = 0;
       var nextIndex = 0;
+      var lastIndex = 0;
+      // `nextMountIndex` will increment for each newly mounted child.
+      var nextMountIndex = 0;
       var lastPlacedNode = null;
       for (name in nextChildren) {
         if (!nextChildren.hasOwnProperty(name)) {
@@ -380,12 +399,14 @@ var ReactMultiChild = {
             updates,
             this._mountChildAtIndex(
               nextChild,
+              mountImages[nextMountIndex],
               lastPlacedNode,
               nextIndex,
               transaction,
               context
             )
           );
+          nextMountIndex++;
         }
         nextIndex++;
         lastPlacedNode = ReactReconciler.getHostNode(nextChild);
@@ -473,17 +494,11 @@ var ReactMultiChild = {
      */
     _mountChildAtIndex: function(
       child,
+      mountImage,
       afterNode,
       index,
       transaction,
       context) {
-      var mountImage = ReactReconciler.mountComponent(
-        child,
-        transaction,
-        this,
-        this._hostContainerInfo,
-        context
-      );
       child._mountIndex = index;
       return this.createChild(child, afterNode, mountImage);
     },

src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildReconcile-test.js

@@ -60,6 +60,14 @@ var StatusDisplay = React.createClass({
     return this.state.internalState;
   },
 
+  componentDidMount: function() {
+    this.props.onFlush();
+  },
+
+  componentDidUpdate: function() {
+    this.props.onFlush();
+  },
+
   render: function() {
     return (
       <div>
@@ -74,43 +82,74 @@ var StatusDisplay = React.createClass({
  */
 var FriendsStatusDisplay = React.createClass({
   /**
-   * Retrieves the rendered children in a nice format for comparing to the input
-   * `this.props.usernameToStatus`. Gets the order directly from each rendered
-   * child's `index` field. Refs are not maintained in the rendered order, and
-   * neither is `this._renderedChildren` (surprisingly).
-   */
-  getStatusDisplays: function() {
-    var name;
-    var orderOfUsernames = [];
+  * Gets the order directly from each rendered child's `index` field.
+  * Refs are not maintained in the rendered order, and neither is
+  * `this._renderedChildren` (surprisingly).
+  */
+  getOriginalKeys: function() {
+    var originalKeys = [];
     // TODO: Update this to a better test that doesn't rely so much on internal
     // implementation details.
     var statusDisplays =
       ReactInstanceMap.get(this)
       ._renderedComponent
       ._renderedChildren;
+    var name;
     for (name in statusDisplays) {
       var child = statusDisplays[name];
       var isPresent = !!child;
       if (isPresent) {
-        orderOfUsernames[child._mountIndex] = getOriginalKey(name);
+        originalKeys[child._mountIndex] = getOriginalKey(name);
       }
     }
+    return originalKeys;
+  },
+  /**
+   * Retrieves the rendered children in a nice format for comparing to the input
+   * `this.props.usernameToStatus`.
+   */
+  getStatusDisplays: function() {
     var res = {};
     var i;
-    for (i = 0; i < orderOfUsernames.length; i++) {
-      var key = orderOfUsernames[i];
+    var originalKeys = this.getOriginalKeys();
+    for (i = 0; i < originalKeys.length; i++) {
+      var key = originalKeys[i];
       res[key] = this.refs[key];
     }
     return res;
   },
+  /**
+   * Verifies that by the time a child is flushed, the refs that appeared
+   * earlier have already been resolved.
+   * TODO: This assumption will likely break with incremental reconciler
+   * but our internal layer API depends on this assumption. We need to change
+   * it to be more declarative before making ref resolution indeterministic.
+   */
+  verifyPreviousRefsResolved: function(flushedKey) {
+    var i;
+    var originalKeys = this.getOriginalKeys();
+    for (i = 0; i < originalKeys.length; i++) {
+      var key = originalKeys[i];
+      if (key === flushedKey) {
+        // We are only interested in children up to the current key.
+        return;
+      }
+      expect(this.refs[key]).toBeTruthy();
+    }
+  },
   render: function() {
     var children = [];
     var key;
     for (key in this.props.usernameToStatus) {
       var status = this.props.usernameToStatus[key];
       children.push(
         !status ? null :
-        <StatusDisplay key={key} ref={key} status={status} />
+        <StatusDisplay
+          key={key}
+          ref={key}
+          onFlush={this.verifyPreviousRefsResolved.bind(this, key)}
+          status={status}
+        />
       );
     }
     return (