From b7e79d86fa38ff28a028f8e3ebd37605b5aad432 Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Sun, 5 Feb 2017 10:18:59 -0800 Subject: [PATCH 1/8] Add toTree() method to stack and fiber TestRenderer --- scripts/fiber/tests-passing.txt | 2 + .../reconciler/ReactCompositeComponent.js | 16 +- .../testing/ReactTestRendererFiber.js | 65 ++++++++ .../__tests__/ReactTestRenderer-test.js | 147 ++++++++++++++++++ src/renderers/testing/stack/ReactTestMount.js | 15 ++ .../testing/stack/ReactTestRendererStack.js | 25 +++ .../testing/stack/ReactTestTextComponent.js | 4 + 7 files changed, 273 insertions(+), 1 deletion(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 42c5ccc37f6..3f16270684f 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1725,6 +1725,8 @@ src/renderers/testing/__tests__/ReactTestRenderer-test.js * supports updates when using refs * supports error boundaries * can update text nodes +* toTree() renders simple components returning host components +* toTree() renders complicated trees of composites and hosts * can update text nodes when rendered as root * can render and update root fragments diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index e7aa14ef93a..50ea1e0c3eb 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -702,7 +702,7 @@ var ReactCompositeComponent = { } else { if (__DEV__) { const componentName = this.getName(); - + if (!warningAboutMissingGetChildContext[componentName]) { warningAboutMissingGetChildContext[componentName] = true; warning( @@ -1300,6 +1300,20 @@ var ReactCompositeComponent = { return inst; }, + toTree: function() { + const element = this._currentElement; + // not using `children`, but I don't want to rewrite without destructuring + // eslint-disable-next-line no-unused-vars + const { children, ...propsWithoutChildren } = element.props; + return { + nodeType: ReactNodeTypes.getType(element), + type: element.type, + props: propsWithoutChildren, + instance: this._instance, + rendered: this._renderedComponent.toTree(), + }; + }, + // Stub _instantiateReactComponent: null, diff --git a/src/renderers/testing/ReactTestRendererFiber.js b/src/renderers/testing/ReactTestRendererFiber.js index 2ce0d797881..4e626686654 100644 --- a/src/renderers/testing/ReactTestRendererFiber.js +++ b/src/renderers/testing/ReactTestRendererFiber.js @@ -16,6 +16,14 @@ var ReactFiberReconciler = require('ReactFiberReconciler'); var ReactGenericBatching = require('ReactGenericBatching'); var emptyObject = require('emptyObject'); +var ReactTypeOfWork = require('ReactTypeOfWork'); +var { + FunctionalComponent, + ClassComponent, + HostComponent, + HostText, + HostRoot, +} = ReactTypeOfWork; import type { TestRendererOptions } from 'ReactTestMount'; @@ -237,6 +245,60 @@ function toJSON(inst : Instance | TextInstance) : ReactTestRendererNode { } } +const propsWithoutChildren = (props) => { + /* eslint-disable no-unused-vars */ + var { children, ...otherProps } = props; + /* eslint-enable */ + return otherProps; +}; + +function nodeAndSiblingsArray(nodeWithSibling) { + var node = nodeWithSibling; + var array = [node]; + while (node.sibling !== null) { + array.push(node.sibling); + node = node.sibling; + } + return array; +} + +function toTree(node) { + switch (node.tag) { + case HostRoot: // 3 + return toTree(node.progressedChild); + case ClassComponent: + return { + nodeType: node.tag, + type: node.type, + props: propsWithoutChildren(node.memoizedProps), + instance: node.stateNode, + rendered: toTree(node.child), + }; + case FunctionalComponent: // 1 + return { + nodeType: node.tag, + type: node.type, + props: propsWithoutChildren(node.memoizedProps), + instance: null, + rendered: toTree(node.child), + }; + case HostComponent: // 5 + return { + nodeType: node.tag, + type: node.type, + props: propsWithoutChildren(node.memoizedProps), + instance: null, // TODO: use createNodeMock here somehow? + rendered: nodeAndSiblingsArray(node.child).map(toTree), + }; + case HostText: // 6 + return node.stateNode.text; + default: + throw new Error( + `toTree() does not yet know how to handle nodes with tag=${node.tag}.` + ); + } +} + var ReactTestFiberRenderer = { create(element : ReactElement, options : TestRendererOptions) { var createNodeMock = defaultTestOptions.createNodeMock; @@ -264,6 +326,9 @@ var ReactTestFiberRenderer = { } return container.children.map(toJSON); }, + toTree() { + return toTree(root); + }, update(newElement : ReactElement) { if (root == null) { return; diff --git a/src/renderers/testing/__tests__/ReactTestRenderer-test.js b/src/renderers/testing/__tests__/ReactTestRenderer-test.js index 75100954635..bc78ac6c4cc 100644 --- a/src/renderers/testing/__tests__/ReactTestRenderer-test.js +++ b/src/renderers/testing/__tests__/ReactTestRenderer-test.js @@ -14,8 +14,33 @@ var React = require('React'); var ReactTestRenderer = require('ReactTestRenderer'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); +var ReactNodeTypes = require('ReactNodeTypes'); +var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactFeatureFlags; +var ClassComponent = ReactDOMFeatureFlags.useFiber + ? ReactTypeOfWork.ClassComponent + : ReactNodeTypes.COMPOSITE; +var FunctionalComponent = ReactDOMFeatureFlags.useFiber + ? ReactTypeOfWork.FunctionalComponent + : ReactNodeTypes.COMPOSITE; +var HostComponent = ReactDOMFeatureFlags.useFiber + ? ReactTypeOfWork.HostComponent + : ReactNodeTypes.HOST; + +// Kind of hacky, but we nullify all the instances to test the tree structure +// with jasmine's deep equality function, and test the instances separate +function nullifyInstances(node) { + if (node && node.instance) { + node.instance = null; + } + if (Array.isArray(node.rendered)) { + node.rendered.forEach(nullifyInstances); + } else if (typeof node.rendered === 'object') { + nullifyInstances(node.rendered); + } +} + describe('ReactTestRenderer', () => { beforeEach(() => { ReactFeatureFlags = require('ReactFeatureFlags'); @@ -517,6 +542,128 @@ describe('ReactTestRenderer', () => { }); }); + it('toTree() renders simple components returning host components', () => { + + var Qoo = () => ( + Hello World! + ); + + var renderer = ReactTestRenderer.create(); + var tree = renderer.toTree(); + + nullifyInstances(tree); + + expect(tree).toEqual({ + nodeType: FunctionalComponent, + type: Qoo, + props: {}, + instance: null, + rendered: { + nodeType: HostComponent, + type: 'span', + props: { className: 'Qoo' }, + instance: null, + rendered: ['Hello World!'], + }, + }); + + }); + + it('toTree() renders complicated trees of composites and hosts', () => { + // SFC returning host. no children props. + var Qoo = () => ( + Hello World! + ); + + // SFC returning host. passes through children. + var Foo = ({ className, children }) => ( +
+ Literal + {children} +
+ ); + + // class composite returning composite. passes through children. + class Bar extends React.Component { + render() { + const { special, children } = this.props; + return ( + + {children} + + ); + } + } + + // class composite return composite. no children props. + class Bam extends React.Component { + render() { + return ( + + + + ); + } + } + + var renderer = ReactTestRenderer.create(); + var tree = renderer.toTree(); + + // we test for the presence of instances before nulling them out + expect(tree.instance).toBeInstanceOf(Bam); + expect(tree.rendered.instance).toBeInstanceOf(Bar) + + nullifyInstances(tree); + + expect(tree).toEqual({ + type: Bam, + nodeType: ClassComponent, + props: {}, + instance: null, + rendered: { + type: Bar, + nodeType: ClassComponent, + props: { special: true }, + instance: null, + rendered: { + type: Foo, + nodeType: FunctionalComponent, + props: { className: 'special' }, + instance: null, + rendered: { + type: 'div', + nodeType: HostComponent, + props: { className: 'Foo special' }, + instance: null, + rendered: [ + { + type: 'span', + nodeType: HostComponent, + props: { className: 'Foo2' }, + instance: null, + rendered: ['Literal'], + }, + { + type: Qoo, + nodeType: FunctionalComponent, + props: {}, + instance: null, + rendered: { + type: 'span', + nodeType: HostComponent, + props: { className: 'Qoo' }, + instance: null, + rendered: ['Hello World!'], + }, + }, + ], + }, + }, + }, + }); + + }); + if (ReactDOMFeatureFlags.useFiber) { it('can update text nodes when rendered as root', () => { var renderer = ReactTestRenderer.create(['Hello', 'world']); diff --git a/src/renderers/testing/stack/ReactTestMount.js b/src/renderers/testing/stack/ReactTestMount.js index 65dd8c4d0a2..2f17206d19a 100644 --- a/src/renderers/testing/stack/ReactTestMount.js +++ b/src/renderers/testing/stack/ReactTestMount.js @@ -14,6 +14,7 @@ var React = require('React'); var ReactReconciler = require('ReactReconciler'); var ReactUpdates = require('ReactUpdates'); +var ReactNodeTypes = require('ReactNodeTypes'); var emptyObject = require('emptyObject'); var getHostComponentFromComposite = require('getHostComponentFromComposite'); @@ -138,6 +139,20 @@ ReactTestInstance.prototype.unmount = function(nextElement) { }); this._component = null; }; +ReactTestInstance.prototype.toTree = function() { + const component = this._component._renderedComponent; + const element = component._currentElement; + // not using `children`, but I don't want to rewrite without destructuring + // eslint-disable-next-line no-unused-vars + const { children, ...propsWithoutChildren } = element.props; + return { + nodeType: ReactNodeTypes.getType(element), + type: element.type, + props: propsWithoutChildren, + instance: component._instance, + rendered: component._renderedComponent.toTree(), + }; +}; ReactTestInstance.prototype.toJSON = function() { var inst = getHostComponentFromComposite(this._component); if (inst === null) { diff --git a/src/renderers/testing/stack/ReactTestRendererStack.js b/src/renderers/testing/stack/ReactTestRendererStack.js index e0a9355e331..e4716c1d462 100644 --- a/src/renderers/testing/stack/ReactTestRendererStack.js +++ b/src/renderers/testing/stack/ReactTestRendererStack.js @@ -23,6 +23,7 @@ var ReactTestReconcileTransaction = require('ReactTestReconcileTransaction'); var ReactUpdates = require('ReactUpdates'); var ReactTestTextComponent = require('ReactTestTextComponent'); var ReactTestEmptyComponent = require('ReactTestEmptyComponent'); +var ReactNodeTypes = require('ReactNodeTypes'); var invariant = require('invariant'); import type { ReactElement } from 'ReactElementType'; @@ -123,6 +124,30 @@ class ReactTestComponent { return object; } + toTree() { + const element = this._currentElement; + // not using `children`, but I don't want to rewrite without destructuring + // eslint-disable-next-line no-unused-vars + const { children, ...propsWithoutChildren } = element.props; + + const rendered = []; + for (var key in this._renderedChildren) { + var inst = this._renderedChildren[key]; + var json = inst.toTree(); + if (json !== undefined) { + rendered.push(json); + } + } + + return { + nodeType: ReactNodeTypes.getType(element), + type: element.type, + props: propsWithoutChildren, + instance: this._nodeMock, + rendered: rendered, + }; + } + getHostNode(): void {} unmountComponent(safely, skipLifecycle): void { // $FlowFixMe https://github.com/facebook/flow/issues/1805 diff --git a/src/renderers/testing/stack/ReactTestTextComponent.js b/src/renderers/testing/stack/ReactTestTextComponent.js index e87e1bd4b78..f1af33b1567 100644 --- a/src/renderers/testing/stack/ReactTestTextComponent.js +++ b/src/renderers/testing/stack/ReactTestTextComponent.js @@ -30,6 +30,10 @@ class ReactTestTextComponent { return this._currentElement; } + toTree() { + return this._currentElement; + } + mountComponent(): void {} getHostNode(): void {} unmountComponent(): void {} From cf10eb3cd84f150b8373a09e1d13d9fe640bbc60 Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Sun, 5 Feb 2017 16:00:26 -0800 Subject: [PATCH 2/8] Address PR feedback --- .../reconciler/ReactCompositeComponent.js | 14 ----- .../testing/ReactTestRendererFiber.js | 27 ++++----- .../__tests__/ReactTestRenderer-test.js | 60 +++++++++---------- src/renderers/testing/stack/ReactTestMount.js | 46 ++++++++++---- .../testing/stack/ReactTestRendererStack.js | 25 -------- .../testing/stack/ReactTestTextComponent.js | 4 -- 6 files changed, 74 insertions(+), 102 deletions(-) diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index 50ea1e0c3eb..f534cfd5c71 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -1300,20 +1300,6 @@ var ReactCompositeComponent = { return inst; }, - toTree: function() { - const element = this._currentElement; - // not using `children`, but I don't want to rewrite without destructuring - // eslint-disable-next-line no-unused-vars - const { children, ...propsWithoutChildren } = element.props; - return { - nodeType: ReactNodeTypes.getType(element), - type: element.type, - props: propsWithoutChildren, - instance: this._instance, - rendered: this._renderedComponent.toTree(), - }; - }, - // Stub _instantiateReactComponent: null, diff --git a/src/renderers/testing/ReactTestRendererFiber.js b/src/renderers/testing/ReactTestRendererFiber.js index 4e626686654..f8bcad2a336 100644 --- a/src/renderers/testing/ReactTestRendererFiber.js +++ b/src/renderers/testing/ReactTestRendererFiber.js @@ -17,6 +17,7 @@ var ReactFiberReconciler = require('ReactFiberReconciler'); var ReactGenericBatching = require('ReactGenericBatching'); var emptyObject = require('emptyObject'); var ReactTypeOfWork = require('ReactTypeOfWork'); +var invariant = require('invariant'); var { FunctionalComponent, ClassComponent, @@ -245,13 +246,6 @@ function toJSON(inst : Instance | TextInstance) : ReactTestRendererNode { } } -const propsWithoutChildren = (props) => { - /* eslint-disable no-unused-vars */ - var { children, ...otherProps } = props; - /* eslint-enable */ - return otherProps; -}; - function nodeAndSiblingsArray(nodeWithSibling) { var node = nodeWithSibling; var array = [node]; @@ -268,34 +262,37 @@ function toTree(node) { return toTree(node.progressedChild); case ClassComponent: return { - nodeType: node.tag, + nodeType: 'component', type: node.type, - props: propsWithoutChildren(node.memoizedProps), + props: { ...node.memoizedProps }, instance: node.stateNode, rendered: toTree(node.child), }; case FunctionalComponent: // 1 return { - nodeType: node.tag, + nodeType: 'component', type: node.type, - props: propsWithoutChildren(node.memoizedProps), + props: { ...node.memoizedProps }, instance: null, rendered: toTree(node.child), }; case HostComponent: // 5 return { - nodeType: node.tag, + nodeType: 'host', type: node.type, - props: propsWithoutChildren(node.memoizedProps), + props: { ...node.memoizedProps }, instance: null, // TODO: use createNodeMock here somehow? rendered: nodeAndSiblingsArray(node.child).map(toTree), }; case HostText: // 6 return node.stateNode.text; default: - throw new Error( - `toTree() does not yet know how to handle nodes with tag=${node.tag}.` + invariant( + false, + 'toTree() does not yet know how to handle nodes with tag=%s', + node.tag ); + break; } } diff --git a/src/renderers/testing/__tests__/ReactTestRenderer-test.js b/src/renderers/testing/__tests__/ReactTestRenderer-test.js index bc78ac6c4cc..e64a957dac2 100644 --- a/src/renderers/testing/__tests__/ReactTestRenderer-test.js +++ b/src/renderers/testing/__tests__/ReactTestRenderer-test.js @@ -14,30 +14,26 @@ var React = require('React'); var ReactTestRenderer = require('ReactTestRenderer'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); -var ReactNodeTypes = require('ReactNodeTypes'); -var ReactTypeOfWork = require('ReactTypeOfWork'); +var prettyFormat = require('pretty-format'); var ReactFeatureFlags; -var ClassComponent = ReactDOMFeatureFlags.useFiber - ? ReactTypeOfWork.ClassComponent - : ReactNodeTypes.COMPOSITE; -var FunctionalComponent = ReactDOMFeatureFlags.useFiber - ? ReactTypeOfWork.FunctionalComponent - : ReactNodeTypes.COMPOSITE; -var HostComponent = ReactDOMFeatureFlags.useFiber - ? ReactTypeOfWork.HostComponent - : ReactNodeTypes.HOST; - // Kind of hacky, but we nullify all the instances to test the tree structure -// with jasmine's deep equality function, and test the instances separate -function nullifyInstances(node) { +// 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) { if (node && node.instance) { node.instance = null; } + if (node && node.props && node.props.children) { + // eslint-disable-next-line no-unused-vars + var { children, ...props } = node.props; + node.props = props; + } if (Array.isArray(node.rendered)) { - node.rendered.forEach(nullifyInstances); + node.rendered.forEach(cleanNode); } else if (typeof node.rendered === 'object') { - nullifyInstances(node.rendered); + cleanNode(node.rendered); } } @@ -551,21 +547,21 @@ describe('ReactTestRenderer', () => { var renderer = ReactTestRenderer.create(); var tree = renderer.toTree(); - nullifyInstances(tree); + cleanNode(tree); - expect(tree).toEqual({ - nodeType: FunctionalComponent, + expect(prettyFormat(tree)).toEqual(prettyFormat({ + nodeType: 'component', type: Qoo, props: {}, instance: null, rendered: { - nodeType: HostComponent, + nodeType: 'host', type: 'span', props: { className: 'Qoo' }, instance: null, rendered: ['Hello World!'], }, - }); + })); }); @@ -611,46 +607,46 @@ describe('ReactTestRenderer', () => { // we test for the presence of instances before nulling them out expect(tree.instance).toBeInstanceOf(Bam); - expect(tree.rendered.instance).toBeInstanceOf(Bar) + expect(tree.rendered.instance).toBeInstanceOf(Bar); - nullifyInstances(tree); + cleanNode(tree); - expect(tree).toEqual({ + expect(prettyFormat(tree)).toEqual(prettyFormat({ type: Bam, - nodeType: ClassComponent, + nodeType: 'component', props: {}, instance: null, rendered: { type: Bar, - nodeType: ClassComponent, + nodeType: 'component', props: { special: true }, instance: null, rendered: { type: Foo, - nodeType: FunctionalComponent, + nodeType: 'component', props: { className: 'special' }, instance: null, rendered: { type: 'div', - nodeType: HostComponent, + nodeType: 'host', props: { className: 'Foo special' }, instance: null, rendered: [ { type: 'span', - nodeType: HostComponent, + nodeType: 'host', props: { className: 'Foo2' }, instance: null, rendered: ['Literal'], }, { type: Qoo, - nodeType: FunctionalComponent, + nodeType: 'component', props: {}, instance: null, rendered: { type: 'span', - nodeType: HostComponent, + nodeType: 'host', props: { className: 'Qoo' }, instance: null, rendered: ['Hello World!'], @@ -660,7 +656,7 @@ describe('ReactTestRenderer', () => { }, }, }, - }); + })); }); diff --git a/src/renderers/testing/stack/ReactTestMount.js b/src/renderers/testing/stack/ReactTestMount.js index 2f17206d19a..3b1e6a1d2d3 100644 --- a/src/renderers/testing/stack/ReactTestMount.js +++ b/src/renderers/testing/stack/ReactTestMount.js @@ -140,18 +140,7 @@ ReactTestInstance.prototype.unmount = function(nextElement) { this._component = null; }; ReactTestInstance.prototype.toTree = function() { - const component = this._component._renderedComponent; - const element = component._currentElement; - // not using `children`, but I don't want to rewrite without destructuring - // eslint-disable-next-line no-unused-vars - const { children, ...propsWithoutChildren } = element.props; - return { - nodeType: ReactNodeTypes.getType(element), - type: element.type, - props: propsWithoutChildren, - instance: component._instance, - rendered: component._renderedComponent.toTree(), - }; + return toTree(this._component._renderedComponent); }; ReactTestInstance.prototype.toJSON = function() { var inst = getHostComponentFromComposite(this._component); @@ -161,6 +150,39 @@ ReactTestInstance.prototype.toJSON = function() { return inst.toJSON(); }; +function toTree(component) { + var element = component._currentElement; + if (!React.isValidElement(element)) { + return element; + } + if (!component._renderedComponent) { + var rendered = []; + for (var key in component._renderedChildren) { + var inst = component._renderedChildren[key]; + var json = toTree(inst); + if (json !== undefined) { + rendered.push(json); + } + } + + return { + nodeType: 'host', + type: element.type, + props: { ...element.props }, + instance: component._nodeMock, + rendered: rendered, + }; + } else { + return { + nodeType: 'component', + type: element.type, + props: { ...element.props }, + instance: component._instance, + rendered: toTree(component._renderedComponent), + }; + } +} + /** * As soon as `ReactMount` is refactored to not rely on the DOM, we can share * code between the two. For now, we'll hard code the ID logic. diff --git a/src/renderers/testing/stack/ReactTestRendererStack.js b/src/renderers/testing/stack/ReactTestRendererStack.js index e4716c1d462..e0a9355e331 100644 --- a/src/renderers/testing/stack/ReactTestRendererStack.js +++ b/src/renderers/testing/stack/ReactTestRendererStack.js @@ -23,7 +23,6 @@ var ReactTestReconcileTransaction = require('ReactTestReconcileTransaction'); var ReactUpdates = require('ReactUpdates'); var ReactTestTextComponent = require('ReactTestTextComponent'); var ReactTestEmptyComponent = require('ReactTestEmptyComponent'); -var ReactNodeTypes = require('ReactNodeTypes'); var invariant = require('invariant'); import type { ReactElement } from 'ReactElementType'; @@ -124,30 +123,6 @@ class ReactTestComponent { return object; } - toTree() { - const element = this._currentElement; - // not using `children`, but I don't want to rewrite without destructuring - // eslint-disable-next-line no-unused-vars - const { children, ...propsWithoutChildren } = element.props; - - const rendered = []; - for (var key in this._renderedChildren) { - var inst = this._renderedChildren[key]; - var json = inst.toTree(); - if (json !== undefined) { - rendered.push(json); - } - } - - return { - nodeType: ReactNodeTypes.getType(element), - type: element.type, - props: propsWithoutChildren, - instance: this._nodeMock, - rendered: rendered, - }; - } - getHostNode(): void {} unmountComponent(safely, skipLifecycle): void { // $FlowFixMe https://github.com/facebook/flow/issues/1805 diff --git a/src/renderers/testing/stack/ReactTestTextComponent.js b/src/renderers/testing/stack/ReactTestTextComponent.js index f1af33b1567..e87e1bd4b78 100644 --- a/src/renderers/testing/stack/ReactTestTextComponent.js +++ b/src/renderers/testing/stack/ReactTestTextComponent.js @@ -30,10 +30,6 @@ class ReactTestTextComponent { return this._currentElement; } - toTree() { - return this._currentElement; - } - mountComponent(): void {} getHostNode(): void {} unmountComponent(): void {} From cf8a7a719bd3ed9abd64426b8e062ff16c191b53 Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Mon, 6 Feb 2017 10:48:30 -0800 Subject: [PATCH 3/8] Refactor TestRenderer to use correct root --- .../testing/ReactTestRendererFiber.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/renderers/testing/ReactTestRendererFiber.js b/src/renderers/testing/ReactTestRendererFiber.js index f8bcad2a336..f6e910fbf20 100644 --- a/src/renderers/testing/ReactTestRendererFiber.js +++ b/src/renderers/testing/ReactTestRendererFiber.js @@ -259,7 +259,7 @@ function nodeAndSiblingsArray(nodeWithSibling) { function toTree(node) { switch (node.tag) { case HostRoot: // 3 - return toTree(node.progressedChild); + return toTree(node.child); case ClassComponent: return { nodeType: 'component', @@ -307,12 +307,12 @@ var ReactTestFiberRenderer = { createNodeMock, tag: 'CONTAINER', }; - var root = TestRenderer.createContainer(container); - TestRenderer.updateContainer(element, root, null, null); + var root = TestRenderer.createContainer(container).stateNode; + TestRenderer.updateContainer(element, root.current, null, null); return { toJSON() { - if (root == null || container == null) { + if (root.current == null || container == null) { return null; } if (container.children.length === 0) { @@ -324,27 +324,27 @@ var ReactTestFiberRenderer = { return container.children.map(toJSON); }, toTree() { - return toTree(root); + return toTree(root.current); }, update(newElement : ReactElement) { - if (root == null) { + if (root.current == null) { return; } - TestRenderer.updateContainer(newElement, root, null, null); + TestRenderer.updateContainer(newElement, root.current, null, null); }, unmount() { - if (root == null) { + if (root.current == null) { return; } - TestRenderer.updateContainer(null, root, null); + TestRenderer.updateContainer(null, root.current, null); container = null; root = null; }, getInstance() { - if (root == null) { + if (root.current == null) { return null; } - return TestRenderer.getPublicRootInstance(root); + return TestRenderer.getPublicRootInstance(root.current); }, }; }, From cc80a77cf980b43a7d9a33a20615171aa89e8b61 Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Tue, 7 Feb 2017 22:15:00 -0800 Subject: [PATCH 4/8] Rebase off master and fix root node references --- src/renderers/testing/ReactTestRendererFiber.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/renderers/testing/ReactTestRendererFiber.js b/src/renderers/testing/ReactTestRendererFiber.js index f6e910fbf20..3f1c7251491 100644 --- a/src/renderers/testing/ReactTestRendererFiber.js +++ b/src/renderers/testing/ReactTestRendererFiber.js @@ -307,8 +307,8 @@ var ReactTestFiberRenderer = { createNodeMock, tag: 'CONTAINER', }; - var root = TestRenderer.createContainer(container).stateNode; - TestRenderer.updateContainer(element, root.current, null, null); + var root = TestRenderer.createContainer(container); + TestRenderer.updateContainer(element, root, null, null); return { toJSON() { @@ -330,13 +330,13 @@ var ReactTestFiberRenderer = { if (root.current == null) { return; } - TestRenderer.updateContainer(newElement, root.current, null, null); + TestRenderer.updateContainer(newElement, root, null, null); }, unmount() { if (root.current == null) { return; } - TestRenderer.updateContainer(null, root.current, null); + TestRenderer.updateContainer(null, root, null); container = null; root = null; }, @@ -344,7 +344,7 @@ var ReactTestFiberRenderer = { if (root.current == null) { return null; } - return TestRenderer.getPublicRootInstance(root.current); + return TestRenderer.getPublicRootInstance(root); }, }; }, From bf7339dcf146380b927e98511085deefeadc7518 Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Wed, 8 Feb 2017 20:46:04 -0800 Subject: [PATCH 5/8] Add flow types --- .../testing/ReactTestRendererFiber.js | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/renderers/testing/ReactTestRendererFiber.js b/src/renderers/testing/ReactTestRendererFiber.js index 3f1c7251491..c391476857e 100644 --- a/src/renderers/testing/ReactTestRendererFiber.js +++ b/src/renderers/testing/ReactTestRendererFiber.js @@ -27,6 +27,8 @@ var { } = ReactTypeOfWork; import type { TestRendererOptions } from 'ReactTestMount'; +import type { Fiber } from 'ReactFiber'; +import type { FiberRoot } from 'ReactFiberRoot'; type ReactTestRendererJSON = {| type : string, @@ -246,17 +248,23 @@ function toJSON(inst : Instance | TextInstance) : ReactTestRendererNode { } } -function nodeAndSiblingsArray(nodeWithSibling) { +function nodeAndSiblingsArray(nodeWithSibling: ?Fiber) { + var array = []; var node = nodeWithSibling; - var array = [node]; - while (node.sibling !== null) { - array.push(node.sibling); + while (node != null) { + array.push(node); node = node.sibling; } return array; } -function toTree(node) { +function toTree(node: ?Fiber) { + if (node == null) { + invariant( + false, + 'node cannot be null' + ); + } switch (node.tag) { case HostRoot: // 3 return toTree(node.child); @@ -292,7 +300,6 @@ function toTree(node) { 'toTree() does not yet know how to handle nodes with tag=%s', node.tag ); - break; } } @@ -307,12 +314,13 @@ var ReactTestFiberRenderer = { createNodeMock, tag: 'CONTAINER', }; - var root = TestRenderer.createContainer(container); + var root: ?FiberRoot = TestRenderer.createContainer(container); + invariant(root != null, 'something went wrong'); TestRenderer.updateContainer(element, root, null, null); return { toJSON() { - if (root.current == null || container == null) { + if (root == null || root.current == null || container == null) { return null; } if (container.children.length === 0) { @@ -324,16 +332,19 @@ var ReactTestFiberRenderer = { return container.children.map(toJSON); }, toTree() { + if (root == null || root.current == null) { + return null; + } return toTree(root.current); }, update(newElement : ReactElement) { - if (root.current == null) { + if (root == null || root.current == null) { return; } TestRenderer.updateContainer(newElement, root, null, null); }, unmount() { - if (root.current == null) { + if (root == null || root.current == null) { return; } TestRenderer.updateContainer(null, root, null); @@ -341,7 +352,7 @@ var ReactTestFiberRenderer = { root = null; }, getInstance() { - if (root.current == null) { + if (root == null || root.current == null) { return null; } return TestRenderer.getPublicRootInstance(root); From 43d8f4a2142ea08dbcb1eb3a4a11cbb996236bc5 Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Wed, 8 Feb 2017 20:51:57 -0800 Subject: [PATCH 6/8] Add test for null rendering components --- .../testing/ReactTestRendererFiber.js | 5 +--- .../__tests__/ReactTestRenderer-test.js | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/renderers/testing/ReactTestRendererFiber.js b/src/renderers/testing/ReactTestRendererFiber.js index c391476857e..7aba7553ba4 100644 --- a/src/renderers/testing/ReactTestRendererFiber.js +++ b/src/renderers/testing/ReactTestRendererFiber.js @@ -260,10 +260,7 @@ function nodeAndSiblingsArray(nodeWithSibling: ?Fiber) { function toTree(node: ?Fiber) { if (node == null) { - invariant( - false, - 'node cannot be null' - ); + return null; } switch (node.tag) { case HostRoot: // 3 diff --git a/src/renderers/testing/__tests__/ReactTestRenderer-test.js b/src/renderers/testing/__tests__/ReactTestRenderer-test.js index e64a957dac2..ef2b6124f6b 100644 --- a/src/renderers/testing/__tests__/ReactTestRenderer-test.js +++ b/src/renderers/testing/__tests__/ReactTestRenderer-test.js @@ -22,6 +22,9 @@ var ReactFeatureFlags; // also delete children props because testing them is more annoying and not // really important to verify. function cleanNode(node) { + if (!node) { + return; + } if (node && node.instance) { node.instance = null; } @@ -565,6 +568,30 @@ describe('ReactTestRenderer', () => { }); + it('toTree() handles null rendering components', () => { + class Foo extends React.Component { + render() { + return null; + } + } + + var renderer = ReactTestRenderer.create(); + var tree = renderer.toTree(); + + expect(tree.instance).toBeInstanceOf(Foo); + + cleanNode(tree); + + expect(tree).toEqual({ + type: Foo, + nodeType: 'component', + props: { }, + instance: null, + rendered: null, + }); + + }); + it('toTree() renders complicated trees of composites and hosts', () => { // SFC returning host. no children props. var Qoo = () => ( From 878f629e243a7e485ff12ce5b8d0e25f8dd2d274 Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Thu, 9 Feb 2017 08:45:17 -0800 Subject: [PATCH 7/8] Remove last remaining lint error --- src/renderers/testing/stack/ReactTestMount.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/renderers/testing/stack/ReactTestMount.js b/src/renderers/testing/stack/ReactTestMount.js index 3b1e6a1d2d3..f2af0034064 100644 --- a/src/renderers/testing/stack/ReactTestMount.js +++ b/src/renderers/testing/stack/ReactTestMount.js @@ -14,7 +14,6 @@ var React = require('React'); var ReactReconciler = require('ReactReconciler'); var ReactUpdates = require('ReactUpdates'); -var ReactNodeTypes = require('ReactNodeTypes'); var emptyObject = require('emptyObject'); var getHostComponentFromComposite = require('getHostComponentFromComposite'); From 41a557b4cc66d27b51a21b5255bcba742144963e Mon Sep 17 00:00:00 2001 From: Leland Richardson Date: Thu, 9 Feb 2017 09:49:42 -0800 Subject: [PATCH 8/8] Add missing test --- scripts/fiber/tests-passing.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 3f16270684f..f2905d51d1d 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1726,6 +1726,7 @@ src/renderers/testing/__tests__/ReactTestRenderer-test.js * supports error boundaries * can update text nodes * toTree() renders simple components returning host components +* toTree() handles null rendering components * toTree() renders complicated trees of composites and hosts * can update text nodes when rendered as root * can render and update root fragments