Fix errors in React Native children management (#9449)

* Fix errors in React Native children management

- Support using appendChild to move an existing child (Fiber does this, but we were assuming all children here were new; Yoga throws if you insert a child that already has a parent)
- Calculate beforeChildIndex after removing old child (previously, off by one when the new position is later than the old position)

* Add better children management tests for RN

src/__mocks__/UIManager.js

@@ -11,12 +11,139 @@
 
 // Mock of the Native Hooks
 
+const ReactNativeTagHandles = require('ReactNativeTagHandles');
+const invariant = require('fbjs/lib/invariant');
+
+// Map of viewTag -> {children: [childTag], parent: ?parentTag}
+const roots = [];
+let views = new Map();
+
+function autoCreateRoot(tag) {
+  // Seriously, this is how we distinguish roots in RN.
+  if (!views.has(tag) && ReactNativeTagHandles.reactTagIsNativeTopRootID(tag)) {
+    roots.push(tag);
+    views.set(tag, {
+      children: [],
+      parent: null,
+      props: {},
+      viewName: '<native root>',
+    });
+  }
+}
+
+function insertSubviewAtIndex(parent, child, index) {
+  const parentInfo = views.get(parent);
+  const childInfo = views.get(child);
+  invariant(
+    childInfo.parent === null,
+    'Inserting view %s %s which already has parent',
+    child,
+    JSON.stringify(childInfo.props),
+  );
+  invariant(
+    0 <= index && index <= parentInfo.children.length,
+    'Invalid index %s for children %s',
+    index,
+    parentInfo.children
+  );
+  parentInfo.children.splice(index, 0, child);
+  childInfo.parent = parent;
+}
+
+function removeChild(parent, child) {
+  const parentInfo = views.get(parent);
+  const childInfo = views.get(child);
+  const index = parentInfo.children.indexOf(child);
+  invariant(index >= 0, 'Missing view %s during removal', child);
+  parentInfo.children.splice(index, 1);
+  childInfo.parent = null;
+}
+
 var RCTUIManager = {
+  __dumpHierarchyForJestTestsOnly: function() {
+    return roots.map((tag) => dumpSubtree(tag, 0)).join('\n');
+
+    function dumpSubtree(tag, indent) {
+      const info = views.get(tag);
+      let out = '';
+      out += ' '.repeat(indent) + info.viewName + ' ' + JSON.stringify(info.props);
+      for (const child of info.children) {
+        out += '\n' + dumpSubtree(child, indent + 2);
+      }
+      return out;
+    }
+  },
   clearJSResponder: jest.fn(),
-  createView: jest.fn(),
+  createView: jest.fn(function createView(reactTag, viewName, rootTag, props) {
+    invariant(
+      !views.has(reactTag),
+      'Created two native views with tag %s',
+      reactTag,
+    );
+    views.set(reactTag, {
+      children: [],
+      parent: null,
+      props: props,
+      viewName: viewName,
+    });
+  }),
   setJSResponder: jest.fn(),
-  setChildren: jest.fn(),
-  manageChildren: jest.fn(),
+  setChildren: jest.fn(function setChildren(parentTag, reactTags) {
+    autoCreateRoot(parentTag);
+    // Native doesn't actually check this but it seems like a good idea
+    invariant(
+      views.get(parentTag).children.length === 0,
+      'Calling .setChildren on nonempty view %s',
+      parentTag,
+    );
+    // This logic ported from iOS (RCTUIManager.m)
+    reactTags.forEach((tag, i) => {
+      insertSubviewAtIndex(parentTag, tag, i);
+    });
+  }),
+  manageChildren: jest.fn(function manageChildren(
+    parentTag,
+    moveFromIndices = [],
+    moveToIndices = [],
+    addChildReactTags = [],
+    addAtIndices = [],
+    removeAtIndices = [],
+  ) {
+    autoCreateRoot(parentTag);
+    // This logic ported from iOS (RCTUIManager.m)
+    invariant(
+      moveFromIndices.length === moveToIndices.length,
+      'Mismatched move indices %s and %s',
+      moveFromIndices,
+      moveToIndices,
+    );
+    invariant(
+      addChildReactTags.length === addAtIndices.length,
+      'Mismatched add indices %s and %s',
+      addChildReactTags,
+      addAtIndices,
+    );
+    const parentInfo = views.get(parentTag);
+    const permanentlyRemovedChildren = removeAtIndices.map((index) => parentInfo.children[index]);
+    const temporarilyRemovedChildren = moveFromIndices.map((index) => parentInfo.children[index]);
+    permanentlyRemovedChildren.forEach((tag) => removeChild(parentTag, tag));
+    temporarilyRemovedChildren.forEach((tag) => removeChild(parentTag, tag));
+    permanentlyRemovedChildren.forEach((tag) => {
+      views.delete(tag);
+    });
+    // List of [index, tag]
+    const indicesToInsert = [];
+    temporarilyRemovedChildren.forEach((tag, i) => {
+      indicesToInsert.push([moveToIndices[i], temporarilyRemovedChildren[i]]);
+    });
+    addChildReactTags.forEach((tag, i) => {
+      indicesToInsert.push([addAtIndices[i], addChildReactTags[i]]);
+    });
+    indicesToInsert.sort((a, b) => a[0] - b[0]);
+    for (const [i, tag] of indicesToInsert) {
+      insertSubviewAtIndex(parentTag, tag, i);
+    }
+  }),
   updateView: jest.fn(),
   removeSubviewsFromContainerWithID: jest.fn(),
   replaceExistingNonRootView: jest.fn(),

src/renderers/native/ReactNativeFiber.js

@@ -82,16 +82,32 @@ const NativeRenderer = ReactFiberReconciler({
     } else {
       const children = parentInstance._children;
 
-      children.push(child);
+      const index = children.indexOf(child);
 
-      UIManager.manageChildren(
-        parentInstance._nativeTag, // containerTag
-        [], // moveFromIndices
-        [], // moveToIndices
-        [childTag], // addChildReactTags
-        [children.length - 1], // addAtIndices
-        [], // removeAtIndices
-      );
+      if (index >= 0) {
+        children.splice(index, 1);
+        children.push(child);
+
+        UIManager.manageChildren(
+          parentInstance._nativeTag, // containerTag
+          [index], // moveFromIndices
+          [children.length - 1], // moveToIndices
+          [], // addChildReactTags
+          [], // addAtIndices
+          [], // removeAtIndices
+        );
+      } else {
+        children.push(child);
+
+        UIManager.manageChildren(
+          parentInstance._nativeTag, // containerTag
+          [], // moveFromIndices
+          [], // moveToIndices
+          [childTag], // addChildReactTags
+          [children.length - 1], // addAtIndices
+          [], // removeAtIndices
+        );
+      }
     }
   },
 
@@ -264,12 +280,12 @@ const NativeRenderer = ReactFiberReconciler({
 
     const children = (parentInstance: any)._children;
 
-    const beforeChildIndex = children.indexOf(beforeChild);
     const index = children.indexOf(child);
 
     // Move existing child or add new child?
     if (index >= 0) {
       children.splice(index, 1);
+      const beforeChildIndex = children.indexOf(beforeChild);
       children.splice(beforeChildIndex, 0, child);
 
       UIManager.manageChildren(
@@ -281,6 +297,7 @@ const NativeRenderer = ReactFiberReconciler({
         [], // removeAtIndices
       );
     } else {
+      const beforeChildIndex = children.indexOf(beforeChild);
       children.splice(beforeChildIndex, 0, child);
 
       const childTag = typeof child === 'number' ? child : child._nativeTag;

src/renderers/native/__tests__/ReactNativeEvents-test.js

@@ -58,6 +58,7 @@ it('handles events', () => {
     1,
   );
 
+  expect(UIManager.__dumpHierarchyForJestTestsOnly()).toMatchSnapshot();
   expect(UIManager.createView.mock.calls.length).toBe(2);
 
   // Don't depend on the order of createView() calls.

src/renderers/native/__tests__/ReactNativeMount-test.js

@@ -80,4 +80,32 @@ describe('ReactNative', () => {
     expect(a).toBe(b);
     expect(a).toBe(c);
   });
+
+  it('renders and reorders children', () => {
+    var View = createReactNativeComponentClass({
+      validAttributes: {title: true},
+      uiViewClassName: 'View',
+    });
+
+    class Component extends React.Component {
+      render() {
+        var chars = this.props.chars.split('');
+        return (
+          <View>
+            {chars.map(text => <View key={text} title={text} />)}
+          </View>
+        );
+      }
+    }
+
+    // Mini multi-child stress test: lots of reorders, some adds, some removes.
+    var before = 'abcdefghijklmnopqrst';
+    var after = 'mxhpgwfralkeoivcstzy';
+
+    ReactNative.render(<Component chars={before} />, 11);
+    expect(UIManager.__dumpHierarchyForJestTestsOnly()).toMatchSnapshot();
+
+    ReactNative.render(<Component chars={after} />, 11);
+    expect(UIManager.__dumpHierarchyForJestTestsOnly()).toMatchSnapshot();
+  });
 });

src/renderers/native/__tests__/__snapshots__/ReactNativeEvents-test.js.snap

@@ -0,0 +1,7 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`handles events 1`] = `
+"<native root> {}
+  View {\\"foo\\":\\"outer\\"}
+    View {\\"foo\\":\\"inner\\"}"
+`;

src/renderers/native/__tests__/__snapshots__/ReactNativeMount-test.js.snap

@@ -0,0 +1,51 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`ReactNative renders and reorders children 1`] = `
+"<native root> {}
+  View null
+    View {\\"title\\":\\"a\\"}
+    View {\\"title\\":\\"b\\"}
+    View {\\"title\\":\\"c\\"}
+    View {\\"title\\":\\"d\\"}
+    View {\\"title\\":\\"e\\"}
+    View {\\"title\\":\\"f\\"}
+    View {\\"title\\":\\"g\\"}
+    View {\\"title\\":\\"h\\"}
+    View {\\"title\\":\\"i\\"}
+    View {\\"title\\":\\"j\\"}
+    View {\\"title\\":\\"k\\"}
+    View {\\"title\\":\\"l\\"}
+    View {\\"title\\":\\"m\\"}
+    View {\\"title\\":\\"n\\"}
+    View {\\"title\\":\\"o\\"}
+    View {\\"title\\":\\"p\\"}
+    View {\\"title\\":\\"q\\"}
+    View {\\"title\\":\\"r\\"}
+    View {\\"title\\":\\"s\\"}
+    View {\\"title\\":\\"t\\"}"
+`;
+
+exports[`ReactNative renders and reorders children 2`] = `
+"<native root> {}
+  View null
+    View {\\"title\\":\\"m\\"}
+    View {\\"title\\":\\"x\\"}
+    View {\\"title\\":\\"h\\"}
+    View {\\"title\\":\\"p\\"}
+    View {\\"title\\":\\"g\\"}
+    View {\\"title\\":\\"w\\"}
+    View {\\"title\\":\\"f\\"}
+    View {\\"title\\":\\"r\\"}
+    View {\\"title\\":\\"a\\"}
+    View {\\"title\\":\\"l\\"}
+    View {\\"title\\":\\"k\\"}
+    View {\\"title\\":\\"e\\"}
+    View {\\"title\\":\\"o\\"}
+    View {\\"title\\":\\"i\\"}
+    View {\\"title\\":\\"v\\"}
+    View {\\"title\\":\\"c\\"}
+    View {\\"title\\":\\"s\\"}
+    View {\\"title\\":\\"t\\"}
+    View {\\"title\\":\\"z\\"}
+    View {\\"title\\":\\"y\\"}"
+`;