Fix bug with toTree on rendered array #10616 (#10652)

* Fix bug with toTree on rendered array #10616 * Preserve previous behaviour of rendered being a node unless it's for rendering an array
facebook · Sep 14, 2017 · 7caa035 · 7caa035
1 parent c043c1e
commit 7caa035
Show file tree

Hide file tree

Showing 2 changed files with 142 additions and 11 deletions.
diff --git a/src/renderers/testing/ReactTestRendererFiberEntry.js b/src/renderers/testing/ReactTestRendererFiberEntry.js
@@ -285,14 +285,19 @@ function toJSON(inst: Instance | TextInstance): ReactTestRendererNode {
   }
 }
 
-function nodeAndSiblingsArray(nodeWithSibling: ?Fiber) {
+function nodeAndSiblingsTrees(nodeWithSibling: ?Fiber) {
   var array = [];
   var node = nodeWithSibling;
   while (node != null) {
     array.push(node);
     node = node.sibling;
   }
-  return array;
+  const trees = array.map(toTree);
+  return trees.length ? trees : null;
+}
+
+function hasSiblings(node: ?Fiber) {
+  return node && node.sibling;
 }
 
 function toTree(node: ?Fiber) {
@@ -308,23 +313,27 @@ function toTree(node: ?Fiber) {
         type: node.type,
         props: {...node.memoizedProps},
         instance: node.stateNode,
-        rendered: toTree(node.child),
+        rendered: hasSiblings(node.child)
+          ? nodeAndSiblingsTrees(node.child)
+          : toTree(node.child),
       };
     case FunctionalComponent: // 1
       return {
         nodeType: 'component',
         type: node.type,
         props: {...node.memoizedProps},
         instance: null,
-        rendered: toTree(node.child),
+        rendered: hasSiblings(node.child)
+          ? nodeAndSiblingsTrees(node.child)
+          : toTree(node.child),
       };
     case HostComponent: // 5
       return {
         nodeType: 'host',
         type: node.type,
         props: {...node.memoizedProps},
         instance: null, // TODO: use createNodeMock here somehow?
-        rendered: nodeAndSiblingsArray(node.child).map(toTree),
+        rendered: nodeAndSiblingsTrees(node.child),
       };
     case HostText: // 6
       return node.stateNode.text;

diff --git a/src/renderers/testing/__tests__/ReactTestRenderer-test.js b/src/renderers/testing/__tests__/ReactTestRenderer-test.js
@@ -19,10 +19,14 @@ var prettyFormat = require('pretty-format');
 // with jasmine's deep equality function, and test the instances separate. We
 // also delete children props because testing them is more annoying and not
 // really important to verify.
-function cleanNode(node) {
+function cleanNodeOrArray(node) {
   if (!node) {
     return;
   }
+  if (Array.isArray(node)) {
+    node.forEach(cleanNodeOrArray);
+    return;
+  }
   if (node && node.instance) {
     node.instance = null;
   }
@@ -32,9 +36,9 @@ function cleanNode(node) {
     node.props = props;
   }
   if (Array.isArray(node.rendered)) {
-    node.rendered.forEach(cleanNode);
+    node.rendered.forEach(cleanNodeOrArray);
   } else if (typeof node.rendered === 'object') {
-    cleanNode(node.rendered);
+    cleanNodeOrArray(node.rendered);
   }
 }
 
@@ -507,7 +511,7 @@ describe('ReactTestRenderer', () => {
     var renderer = ReactTestRenderer.create(<Qoo />);
     var tree = renderer.toTree();
 
-    cleanNode(tree);
+    cleanNodeOrArray(tree);
 
     expect(prettyFormat(tree)).toEqual(
       prettyFormat({
@@ -538,7 +542,7 @@ describe('ReactTestRenderer', () => {
 
     expect(tree.instance).toBeInstanceOf(Foo);
 
-    cleanNode(tree);
+    cleanNodeOrArray(tree);
 
     expect(tree).toEqual({
       type: Foo,
@@ -549,6 +553,124 @@ describe('ReactTestRenderer', () => {
     });
   });
 
+  it('toTree() handles simple components that return arrays', () => {
+    const Foo = ({children}) => children;
+
+    const renderer = ReactTestRenderer.create(
+      <Foo>
+        <div>One</div>
+        <div>Two</div>
+      </Foo>,
+    );
+
+    var tree = renderer.toTree();
+
+    cleanNodeOrArray(tree);
+
+    expect(prettyFormat(tree)).toEqual(
+      prettyFormat({
+        type: Foo,
+        nodeType: 'component',
+        props: {},
+        instance: null,
+        rendered: [
+          {
+            instance: null,
+            nodeType: 'host',
+            props: {},
+            rendered: ['One'],
+            type: 'div',
+          },
+          {
+            instance: null,
+            nodeType: 'host',
+            props: {},
+            rendered: ['Two'],
+            type: 'div',
+          },
+        ],
+      }),
+    );
+  });
+
+  it('toTree() handles complicated tree of fragments', () => {
+    class Foo extends React.Component {
+      render() {
+        return this.props.children;
+      }
+    }
+
+    const renderer = ReactTestRenderer.create(
+      <div>
+        <Foo>
+          <div>One</div>
+          <div>Two</div>
+          <Foo>
+            <div>Three</div>
+          </Foo>
+        </Foo>
+        <div>Four</div>
+      </div>,
+    );
+
+    var tree = renderer.toTree();
+
+    cleanNodeOrArray(tree);
+
+    expect(prettyFormat(tree)).toEqual(
+      prettyFormat({
+        type: 'div',
+        instance: null,
+        nodeType: 'host',
+        props: {},
+        rendered: [
+          {
+            type: Foo,
+            nodeType: 'component',
+            props: {},
+            instance: null,
+            rendered: [
+              {
+                type: 'div',
+                nodeType: 'host',
+                props: {},
+                instance: null,
+                rendered: ['One'],
+              },
+              {
+                type: 'div',
+                nodeType: 'host',
+                props: {},
+                instance: null,
+                rendered: ['Two'],
+              },
+              {
+                type: Foo,
+                nodeType: 'component',
+                props: {},
+                instance: null,
+                rendered: {
+                  type: 'div',
+                  nodeType: 'host',
+                  props: {},
+                  instance: null,
+                  rendered: ['Three'],
+                },
+              },
+            ],
+          },
+          {
+            type: 'div',
+            nodeType: 'host',
+            props: {},
+            instance: null,
+            rendered: ['Four'],
+          },
+        ],
+      }),
+    );
+  });
+
   it('root instance and createNodeMock ref return the same value', () => {
     var createNodeMock = ref => ({node: ref});
     var refInst = null;
@@ -602,7 +724,7 @@ describe('ReactTestRenderer', () => {
     expect(tree.instance).toBeInstanceOf(Bam);
     expect(tree.rendered.instance).toBeInstanceOf(Bar);
 
-    cleanNode(tree);
+    cleanNodeOrArray(tree);
 
     expect(prettyFormat(tree)).toEqual(
       prettyFormat({