From cf6b3ff0e23fd955ed313b7a50c7dd825b3f2bc5 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Tue, 16 Jun 2015 17:51:06 -0700 Subject: [PATCH] Disallow passing a DOM component to reactComponentExpect 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. (This module also isn't public API and never has been.) --- .../__tests__/ReactCompositeComponent-test.js | 6 +-- .../__tests__/ReactIdentity-test.js | 8 ++-- .../__tests__/ReactMultiChildText-test.js | 24 +++-------- .../__tests__/refs-destruction-test.js | 43 +++++++++++-------- src/test/reactComponentExpect.js | 5 +++ 5 files changed, 42 insertions(+), 44 deletions(-) diff --git a/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js index 1565aba035..eb052aad3b 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js @@ -132,11 +132,11 @@ describe('ReactCompositeComponent', function() { var instance = ; instance = ReactTestUtils.renderIntoDocument(instance); - reactComponentExpect(instance.refs.x).toBeDOMComponentWithTag('a'); + expect(React.findDOMNode(instance.refs.x).tagName).toBe('A'); instance._toggleActivatedState(); - reactComponentExpect(instance.refs.x).toBeDOMComponentWithTag('b'); + expect(React.findDOMNode(instance.refs.x).tagName).toBe('B'); instance._toggleActivatedState(); - reactComponentExpect(instance.refs.x).toBeDOMComponentWithTag('a'); + expect(React.findDOMNode(instance.refs.x).tagName).toBe('A'); }); it('should not cache old DOM nodes when switching constructors', function() { diff --git a/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js b/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js index 293d878a0a..6a23f45c12 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js @@ -14,7 +14,6 @@ var React; var ReactFragment; var ReactTestUtils; -var reactComponentExpect; var ReactMount; describe('ReactIdentity', function() { @@ -24,7 +23,6 @@ describe('ReactIdentity', function() { React = require('React'); ReactFragment = require('ReactFragment'); ReactTestUtils = require('ReactTestUtils'); - reactComponentExpect = require('reactComponentExpect'); ReactMount = require('ReactMount'); }); @@ -52,7 +50,7 @@ describe('ReactIdentity', function() { instance = React.render(instance, document.createElement('div')); var node = React.findDOMNode(instance); - reactComponentExpect(instance).toBeDOMComponentWithChildCount(2); + expect(node.childNodes.length).toBe(2); checkId(node.childNodes[0], '.0.$first:0'); checkId(node.childNodes[1], '.0.$second:0'); }); @@ -68,7 +66,7 @@ describe('ReactIdentity', function() { instance = React.render(instance, document.createElement('div')); var node = React.findDOMNode(instance); - reactComponentExpect(instance).toBeDOMComponentWithChildCount(4); + expect(node.childNodes.length).toBe(4); checkId(node.childNodes[0], '.0.$apple'); checkId(node.childNodes[1], '.0.$banana'); checkId(node.childNodes[2], '.0.$0'); @@ -92,7 +90,7 @@ describe('ReactIdentity', function() { instance = React.render(instance, document.createElement('div')); var node = React.findDOMNode(instance); - reactComponentExpect(instance).toBeDOMComponentWithChildCount(3); + expect(node.childNodes.length).toBe(3); checkId(node.childNodes[0], '.0.$wrap1'); checkId(node.childNodes[0].firstChild, '.0.$wrap1.$squirrel'); diff --git a/src/renderers/shared/reconciler/__tests__/ReactMultiChildText-test.js b/src/renderers/shared/reconciler/__tests__/ReactMultiChildText-test.js index 98d13c33b0..4f1eb4ce73 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactMultiChildText-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactMultiChildText-test.js @@ -17,8 +17,6 @@ var React = require('React'); var ReactFragment = require('ReactFragment'); var ReactTestUtils = require('ReactTestUtils'); -var reactComponentExpect = require('reactComponentExpect'); - var frag = ReactFragment.create; // Helpers @@ -42,9 +40,10 @@ var testAllPermutations = function(testCases) { }; var expectChildren = function(d, children) { + var outerNode = React.findDOMNode(d); var textNode; if (typeof children === 'string') { - textNode = React.findDOMNode(d).firstChild; + textNode = outerNode.firstChild; if (children === '') { expect(textNode != null).toBe(false); @@ -54,32 +53,23 @@ var expectChildren = function(d, children) { expect(textNode.data).toBe('' + children); } } else { - expect(React.findDOMNode(d).childNodes.length).toBe(children.length); + expect(outerNode.childNodes.length).toBe(children.length); for (var i = 0; i < children.length; i++) { var child = children[i]; if (typeof child === 'string') { - reactComponentExpect(d) - .expectRenderedChildAt(i) - .toBeTextComponentWithValue(child); - - textNode = React.findDOMNode(d).childNodes[i].firstChild; + textNode = outerNode.childNodes[i].firstChild; if (child === '') { - expect(textNode != null).toBe(false); + expect(textNode).toBe(null); } else { - expect(textNode != null).toBe(true); + expect(textNode).not.toBe(null); expect(textNode.nodeType).toBe(3); expect(textNode.data).toBe('' + child); } } else { - var elementDOMNode = React.findDOMNode(reactComponentExpect(d) - .expectRenderedChildAt(i) - .toBeComponentOfType('div') - .instance() - ); - + var elementDOMNode = outerNode.childNodes[i]; expect(elementDOMNode.tagName).toBe('DIV'); } } diff --git a/src/renderers/shared/reconciler/__tests__/refs-destruction-test.js b/src/renderers/shared/reconciler/__tests__/refs-destruction-test.js index 64b79a978c..7af0e1fb62 100644 --- a/src/renderers/shared/reconciler/__tests__/refs-destruction-test.js +++ b/src/renderers/shared/reconciler/__tests__/refs-destruction-test.js @@ -11,33 +11,38 @@ 'use strict'; -var React = require('React'); -var reactComponentExpect = require('reactComponentExpect'); +var React; +var ReactTestUtils; -var TestComponent = React.createClass({ - render: function() { - return ( -
- {this.props.destroy ? null : -
- Lets try to destroy this. -
- } -
- ); - }, -}); +var TestComponent; describe('refs-destruction', function() { beforeEach(function() { require('mock-modules').dumpCache(); + + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); + + TestComponent = React.createClass({ + render: function() { + return ( +
+ {this.props.destroy ? null : +
+ Lets try to destroy this. +
+ } +
+ ); + }, + }); }); it('should remove refs when destroying the parent', function() { var container = document.createElement('div'); var testInstance = React.render(, container); - reactComponentExpect(testInstance.refs.theInnerDiv) - .toBeDOMComponentWithTag('div'); + expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv)) + .toBe(true); expect(Object.keys(testInstance.refs || {}).length).toEqual(1); React.unmountComponentAtNode(container); expect(Object.keys(testInstance.refs || {}).length).toEqual(0); @@ -46,8 +51,8 @@ describe('refs-destruction', function() { it('should remove refs when destroying the child', function() { var container = document.createElement('div'); var testInstance = React.render(, container); - reactComponentExpect(testInstance.refs.theInnerDiv) - .toBeDOMComponentWithTag('div'); + expect(ReactTestUtils.isDOMComponent(testInstance.refs.theInnerDiv)) + .toBe(true); expect(Object.keys(testInstance.refs || {}).length).toEqual(1); React.render(, container); expect(Object.keys(testInstance.refs || {}).length).toEqual(0); diff --git a/src/test/reactComponentExpect.js b/src/test/reactComponentExpect.js index 0b49f5276b..7c2e9f3465 100644 --- a/src/test/reactComponentExpect.js +++ b/src/test/reactComponentExpect.js @@ -16,6 +16,7 @@ var ReactInstanceMap = require('ReactInstanceMap'); var ReactTestUtils = require('ReactTestUtils'); var assign = require('Object.assign'); +var invariant = require('invariant'); function reactComponentExpect(instance) { if (instance instanceof reactComponentExpectInternal) { @@ -28,6 +29,10 @@ function reactComponentExpect(instance) { expect(instance).not.toBeNull(); + invariant( + ReactTestUtils.isCompositeComponent(instance), + 'reactComponentExpect(...): instance must be a composite component' + ); var internalInstance = ReactInstanceMap.get(instance); expect(typeof internalInstance).toBe('object');