Merge pull request #4151 from spicyj/fairti

Disallow passing a DOM component to findAllInRenderedTree

src/renderers/shared/reconciler/__tests__/ReactMockedComponent-test.js

@@ -73,23 +73,20 @@ describe('ReactMockedComponent', function() {
       },
 
       render: function() {
-        return <AutoMockedComponent prop={this.state.foo} />;
+        return <div><AutoMockedComponent prop={this.state.foo} /></div>;
       },
 
     });
-    var instance = ReactTestUtils.renderIntoDocument(<Wrapper />);
-    instance.update();
-  });
 
-  it('should find an implicitly mocked component in the tree', function() {
-    var instance = ReactTestUtils.renderIntoDocument(
-      <div><span><AutoMockedComponent prop="1" /></span></div>
-    );
+    var instance = ReactTestUtils.renderIntoDocument(<Wrapper />);
+
     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 <MockedComponent prop={this.state.foo} />;
+        return <div><MockedComponent prop={this.state.foo} /></div>;
       },
 
     });
     var instance = ReactTestUtils.renderIntoDocument(<Wrapper />);
-    instance.update();
-  });
 
-  it('should find an explicitly mocked component in the tree', function() {
-    var instance = ReactTestUtils.renderIntoDocument(
-      <div><span><MockedComponent prop="1" /></span></div>
-    );
     var found = ReactTestUtils.findRenderedComponentWithType(
       instance,
       MockedComponent
     );
     expect(typeof found).toBe('object');
+
+    instance.update();
   });
 
   it('has custom methods on the explicitly mocked component', () => {

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;
 
@@ -37,6 +38,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 +160,11 @@ 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;
+    invariant(
+      ReactTestUtils.isCompositeComponent(inst),
+      'findAllInRenderedTree(...): instance must be a composite component'
+    );
+    return findAllInRenderedTreeInternal(ReactInstanceMap.get(inst), test);
   },
 
   /**

src/test/__tests__/ReactTestUtils-test.js

@@ -177,20 +177,28 @@ describe('ReactTestUtils', function() {
     expect(result).toEqual(<div>foo</div>);
   });
 
-  it('Test scryRenderedDOMComponentsWithClass with TextComponent', function() {
-    var renderedComponent = ReactTestUtils.renderIntoDocument(<div>Hello <span>Jim</span></div>);
+  it('can scryRenderedDOMComponentsWithClass with TextComponent', function() {
+    var Wrapper = React.createClass({
+      render: function() {
+        return <div>Hello <span>Jim</span></div>;
+      },
+    });
+    var renderedComponent = ReactTestUtils.renderIntoDocument(<Wrapper />);
     var scryResults = ReactTestUtils.scryRenderedDOMComponentsWithClass(
       renderedComponent,
-      'NonExistantClass'
+      'NonExistentClass'
     );
     expect(scryResults.length).toBe(0);
 
   });
 
-  it('Test scryRenderedDOMComponentsWithClass with className contains \\n', function() {
-    var renderedComponent = ReactTestUtils.renderIntoDocument(
-      <div>Hello <span className={'x\ny'}>Jim</span></div>
-    );
+  it('can scryRenderedDOMComponentsWithClass with className contains \\n', function() {
+    var Wrapper = React.createClass({
+      render: function() {
+        return <div>Hello <span className={'x\ny'}>Jim</span></div>;
+      },
+    });
+    var renderedComponent = ReactTestUtils.renderIntoDocument(<Wrapper />);
     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 <div>{this.props.children}</div>;
+      },
+    });
 
+    var container = document.createElement('div');
     React.render(
-      <div>
+      <Wrapper>
         {null}
         <div>purple</div>
-      </div>,
+      </Wrapper>,
       container
     );
     var tree = React.render(
-      <div>
+      <Wrapper>
         <div>orange</div>
         <div>purple</div>
-      </div>,
+      </Wrapper>,
       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)