From c9b0c26966dd336e72aaa17fcd2d3ec4aac03122 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Mon, 15 Jun 2015 19:08:16 -0700 Subject: [PATCH 1/2] Invert logic in findAllInRenderedTreeInternal With DOM components we won't be able to go from public instance to internal instance reliably, so do the traversal on internal instances. --- src/test/ReactTestUtils.js | 59 +++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index e207ea8140..6202a74e76 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -37,6 +37,34 @@ function Event(suffix) {} * @class ReactTestUtils */ +function findAllInRenderedTreeInternal(inst, test) { + if (!inst || !inst.getPublicInstance) { + return []; + } + var publicInst = inst.getPublicInstance() + var ret = test(publicInst) ? [publicInst] : []; + if (ReactTestUtils.isDOMComponent(publicInst)) { + var renderedChildren = inst._renderedComponent._renderedChildren; + var key; + for (key in renderedChildren) { + if (!renderedChildren.hasOwnProperty(key)) { + continue; + } + ret = ret.concat( + findAllInRenderedTreeInternal( + renderedChildren[key], + test + ) + ); + } + } else if (ReactTestUtils.isCompositeComponent(publicInst)) { + ret = ret.concat( + findAllInRenderedTreeInternal(inst._renderedComponent, test) + ); + } + return ret; +} + /** * Todo: Support the entire DOM.scry query syntax. For now, these simple * utilities will suffice for testing purposes. @@ -131,36 +159,7 @@ var ReactTestUtils = { if (!inst) { return []; } - var ret = test(inst) ? [inst] : []; - if (ReactTestUtils.isDOMComponent(inst)) { - var internalInstance = ReactInstanceMap.get(inst); - var renderedChildren = internalInstance - ._renderedComponent - ._renderedChildren; - var key; - for (key in renderedChildren) { - if (!renderedChildren.hasOwnProperty(key)) { - continue; - } - if (!renderedChildren[key].getPublicInstance) { - continue; - } - ret = ret.concat( - ReactTestUtils.findAllInRenderedTree( - renderedChildren[key].getPublicInstance(), - test - ) - ); - } - } else if (ReactTestUtils.isCompositeComponent(inst)) { - ret = ret.concat( - ReactTestUtils.findAllInRenderedTree( - ReactTestUtils.getRenderedChildOfCompositeComponent(inst), - test - ) - ); - } - return ret; + return findAllInRenderedTreeInternal(ReactInstanceMap.get(inst), test); }, /** From 4070c4ca20b1d08a00fe278d561642e87373c09f Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Tue, 16 Jun 2015 16:37:37 -0700 Subject: [PATCH 2/2] Disallow passing a DOM component to findAllInRenderedTree We won't be able to support this after DOM-components-as-refs but we don't expect many people to be passing DOM components to this function anyway, and it should be fairly straightforward for people to clean up failing unit tests using this function. --- .../__tests__/ReactMockedComponent-test.js | 23 ++++------- src/test/ReactTestUtils.js | 5 +++ src/test/__tests__/ReactTestUtils-test.js | 41 +++++++++++++------ 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/renderers/shared/reconciler/__tests__/ReactMockedComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactMockedComponent-test.js index 2a7c3c4622..5d10bbdbf7 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactMockedComponent-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactMockedComponent-test.js @@ -73,23 +73,20 @@ describe('ReactMockedComponent', function() { }, render: function() { - return ; + return
; }, }); - var instance = ReactTestUtils.renderIntoDocument(); - instance.update(); - }); - it('should find an implicitly mocked component in the tree', function() { - var instance = ReactTestUtils.renderIntoDocument( -
- ); + var instance = ReactTestUtils.renderIntoDocument(); + var found = ReactTestUtils.findRenderedComponentWithType( instance, AutoMockedComponent ); expect(typeof found).toBe('object'); + + instance.update(); }); it('has custom methods on the implicitly mocked component', () => { @@ -113,23 +110,19 @@ describe('ReactMockedComponent', function() { }, render: function() { - return ; + return
; }, }); var instance = ReactTestUtils.renderIntoDocument(); - instance.update(); - }); - it('should find an explicitly mocked component in the tree', function() { - var instance = ReactTestUtils.renderIntoDocument( -
- ); var found = ReactTestUtils.findRenderedComponentWithType( instance, MockedComponent ); expect(typeof found).toBe('object'); + + instance.update(); }); it('has custom methods on the explicitly mocked component', () => { diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index 6202a74e76..9a83c9a234 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -28,6 +28,7 @@ var SyntheticEvent = require('SyntheticEvent'); var assign = require('Object.assign'); var emptyObject = require('emptyObject'); var findDOMNode = require('findDOMNode'); +var invariant = require('invariant'); var topLevelTypes = EventConstants.topLevelTypes; @@ -159,6 +160,10 @@ var ReactTestUtils = { if (!inst) { return []; } + invariant( + ReactTestUtils.isCompositeComponent(inst), + 'findAllInRenderedTree(...): instance must be a composite component' + ); return findAllInRenderedTreeInternal(ReactInstanceMap.get(inst), test); }, diff --git a/src/test/__tests__/ReactTestUtils-test.js b/src/test/__tests__/ReactTestUtils-test.js index 2c784152c7..f45af53c2d 100644 --- a/src/test/__tests__/ReactTestUtils-test.js +++ b/src/test/__tests__/ReactTestUtils-test.js @@ -177,20 +177,28 @@ describe('ReactTestUtils', function() { expect(result).toEqual(
foo
); }); - it('Test scryRenderedDOMComponentsWithClass with TextComponent', function() { - var renderedComponent = ReactTestUtils.renderIntoDocument(
Hello Jim
); + it('can scryRenderedDOMComponentsWithClass with TextComponent', function() { + var Wrapper = React.createClass({ + render: function() { + return
Hello Jim
; + }, + }); + var renderedComponent = ReactTestUtils.renderIntoDocument(); var scryResults = ReactTestUtils.scryRenderedDOMComponentsWithClass( renderedComponent, - 'NonExistantClass' + 'NonExistentClass' ); expect(scryResults.length).toBe(0); }); - it('Test scryRenderedDOMComponentsWithClass with className contains \\n', function() { - var renderedComponent = ReactTestUtils.renderIntoDocument( -
Hello Jim
- ); + it('can scryRenderedDOMComponentsWithClass with className contains \\n', function() { + var Wrapper = React.createClass({ + render: function() { + return
Hello Jim
; + }, + }); + var renderedComponent = ReactTestUtils.renderIntoDocument(); var scryResults = ReactTestUtils.scryRenderedDOMComponentsWithClass( renderedComponent, 'x' @@ -199,26 +207,33 @@ describe('ReactTestUtils', function() { }); it('traverses children in the correct order', function() { - var container = document.createElement('div'); + var Wrapper = React.createClass({ + render: function() { + return
{this.props.children}
; + }, + }); + var container = document.createElement('div'); React.render( -
+ {null}
purple
-
, +
, container ); var tree = React.render( -
+
orange
purple
-
, +
, container ); var log = []; ReactTestUtils.findAllInRenderedTree(tree, function(child) { - log.push(React.findDOMNode(child).textContent); + if (ReactTestUtils.isDOMComponent(child)) { + log.push(React.findDOMNode(child).textContent); + } }); // Should be document order, not mount order (which would be purple, orange)