@samitouri / QOS-React / commits / 7caa035be5

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

Clement Hoang committed Sep 14, 2017 at 14:15 UTC 7caa035be5aef5ac9ae7541b8c199ef534729782
2 files changed +142 -11
src/renderers/testing/ReactTestRendererFiberEntry.js
+14 -5
@@ -285,14 +285,19 @@ function toJSON(inst: Instance | TextInstance): ReactTestRendererNode {
285 }
286 }
287
288 -function nodeAndSiblingsArray(nodeWithSibling: ?Fiber) {
288 +function nodeAndSiblingsTrees(nodeWithSibling: ?Fiber) {
289 var array = [];
290 var node = nodeWithSibling;
291 while (node != null) {
292 array.push(node);
293 node = node.sibling;
294 }
295 - return array;
295 + const trees = array.map(toTree);
296 + return trees.length ? trees : null;
297 +}
298 +
299 +function hasSiblings(node: ?Fiber) {
300 + return node && node.sibling;
301 }
302
303 function toTree(node: ?Fiber) {
@@ -308,7 +313,9 @@ function toTree(node: ?Fiber) {
313 type: node.type,
314 props: {...node.memoizedProps},
315 instance: node.stateNode,
311 - rendered: toTree(node.child),
316 + rendered: hasSiblings(node.child)
317 + ? nodeAndSiblingsTrees(node.child)
318 + : toTree(node.child),
319 };
320 case FunctionalComponent: // 1
321 return {
@@ -316,7 +323,9 @@ function toTree(node: ?Fiber) {
323 type: node.type,
324 props: {...node.memoizedProps},
325 instance: null,
319 - rendered: toTree(node.child),
326 + rendered: hasSiblings(node.child)
327 + ? nodeAndSiblingsTrees(node.child)
328 + : toTree(node.child),
329 };
330 case HostComponent: // 5
331 return {
@@ -324,7 +333,7 @@ function toTree(node: ?Fiber) {
333 type: node.type,
334 props: {...node.memoizedProps},
335 instance: null, // TODO: use createNodeMock here somehow?
327 - rendered: nodeAndSiblingsArray(node.child).map(toTree),
336 + rendered: nodeAndSiblingsTrees(node.child),
337 };
338 case HostText: // 6
339 return node.stateNode.text;
src/renderers/testing/__tests__/ReactTestRenderer-test.js
+128 -6
@@ -19,10 +19,14 @@ var prettyFormat = require('pretty-format');
19 // with jasmine's deep equality function, and test the instances separate. We
20 // also delete children props because testing them is more annoying and not
21 // really important to verify.
22 -function cleanNode(node) {
22 +function cleanNodeOrArray(node) {
23 if (!node) {
24 return;
25 }
26 + if (Array.isArray(node)) {
27 + node.forEach(cleanNodeOrArray);
28 + return;
29 + }
30 if (node && node.instance) {
31 node.instance = null;
32 }
@@ -32,9 +36,9 @@ function cleanNode(node) {
36 node.props = props;
37 }
38 if (Array.isArray(node.rendered)) {
35 - node.rendered.forEach(cleanNode);
39 + node.rendered.forEach(cleanNodeOrArray);
40 } else if (typeof node.rendered === 'object') {
37 - cleanNode(node.rendered);
41 + cleanNodeOrArray(node.rendered);
42 }
43 }
44
@@ -507,7 +511,7 @@ describe('ReactTestRenderer', () => {
511 var renderer = ReactTestRenderer.create(<Qoo />);
512 var tree = renderer.toTree();
513
510 - cleanNode(tree);
514 + cleanNodeOrArray(tree);
515
516 expect(prettyFormat(tree)).toEqual(
517 prettyFormat({
@@ -538,7 +542,7 @@ describe('ReactTestRenderer', () => {
542
543 expect(tree.instance).toBeInstanceOf(Foo);
544
541 - cleanNode(tree);
545 + cleanNodeOrArray(tree);
546
547 expect(tree).toEqual({
548 type: Foo,
@@ -549,6 +553,124 @@ describe('ReactTestRenderer', () => {
553 });
554 });
555
556 + it('toTree() handles simple components that return arrays', () => {
557 + const Foo = ({children}) => children;
558 +
559 + const renderer = ReactTestRenderer.create(
560 + <Foo>
561 + <div>One</div>
562 + <div>Two</div>
563 + </Foo>,
564 + );
565 +
566 + var tree = renderer.toTree();
567 +
568 + cleanNodeOrArray(tree);
569 +
570 + expect(prettyFormat(tree)).toEqual(
571 + prettyFormat({
572 + type: Foo,
573 + nodeType: 'component',
574 + props: {},
575 + instance: null,
576 + rendered: [
577 + {
578 + instance: null,
579 + nodeType: 'host',
580 + props: {},
581 + rendered: ['One'],
582 + type: 'div',
583 + },
584 + {
585 + instance: null,
586 + nodeType: 'host',
587 + props: {},
588 + rendered: ['Two'],
589 + type: 'div',
590 + },
591 + ],
592 + }),
593 + );
594 + });
595 +
596 + it('toTree() handles complicated tree of fragments', () => {
597 + class Foo extends React.Component {
598 + render() {
599 + return this.props.children;
600 + }
601 + }
602 +
603 + const renderer = ReactTestRenderer.create(
604 + <div>
605 + <Foo>
606 + <div>One</div>
607 + <div>Two</div>
608 + <Foo>
609 + <div>Three</div>
610 + </Foo>
611 + </Foo>
612 + <div>Four</div>
613 + </div>,
614 + );
615 +
616 + var tree = renderer.toTree();
617 +
618 + cleanNodeOrArray(tree);
619 +
620 + expect(prettyFormat(tree)).toEqual(
621 + prettyFormat({
622 + type: 'div',
623 + instance: null,
624 + nodeType: 'host',
625 + props: {},
626 + rendered: [
627 + {
628 + type: Foo,
629 + nodeType: 'component',
630 + props: {},
631 + instance: null,
632 + rendered: [
633 + {
634 + type: 'div',
635 + nodeType: 'host',
636 + props: {},
637 + instance: null,
638 + rendered: ['One'],
639 + },
640 + {
641 + type: 'div',
642 + nodeType: 'host',
643 + props: {},
644 + instance: null,
645 + rendered: ['Two'],
646 + },
647 + {
648 + type: Foo,
649 + nodeType: 'component',
650 + props: {},
651 + instance: null,
652 + rendered: {
653 + type: 'div',
654 + nodeType: 'host',
655 + props: {},
656 + instance: null,
657 + rendered: ['Three'],
658 + },
659 + },
660 + ],
661 + },
662 + {
663 + type: 'div',
664 + nodeType: 'host',
665 + props: {},
666 + instance: null,
667 + rendered: ['Four'],
668 + },
669 + ],
670 + }),
671 + );
672 + });
673 +
674 it('root instance and createNodeMock ref return the same value', () => {
675 var createNodeMock = ref => ({node: ref});
676 var refInst = null;
@@ -602,7 +724,7 @@ describe('ReactTestRenderer', () => {
724 expect(tree.instance).toBeInstanceOf(Bam);
725 expect(tree.rendered.instance).toBeInstanceOf(Bar);
726
605 - cleanNode(tree);
727 + cleanNodeOrArray(tree);
728
729 expect(prettyFormat(tree)).toEqual(
730 prettyFormat({