From 7bbdcdba9615e5a1f6704e9a07a44b2bc5530487 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 13 Mar 2014 16:23:10 -0700 Subject: [PATCH] Reassign variable of rendered component The component that gets passed into renderComponent isn't guaranteed to be the instance that gets mounted. We want to clone the instance. Unit tests need to reason about the mounted instance. The first code mod changes: ReactTestUtils.renderIntoDocument() into = ReactTestUtils.renderIntoDocument() Using this scripts: scripts/bin/codemod -m -d ~/www --extensions js \ '^(\s*)ReactTestUtils\.renderIntoDocument\(\s*([$a-zA-Z0-9_]+)\s*\)' \ '\1\2 = ReactTestUtils.renderIntoDocument(\2)' In the second case I do the same for React.renderComponent. However, there are alot more unnecessary matches so I only codemod if the same identifier occurs later in the file. scripts/bin/codemod -m -d ~/www --extensions js \ '^(\s*)React.renderComponent\(\s*([$a-zA-Z0-9_]+)\s*?,(.*?\n?.*?\s\2\b)' \ '\1\2 = React.renderComponent(\2,\3' And one more for ReactMount.renderComponent used by internals. scripts/bin/codemod -m -d ~/www --extensions js \ '^(\s*)ReactMount.renderComponent\(\s*([$a-zA-Z0-9_]+)\s*?,(.*?\n?.*?\s\2\b)' \ '\1\2 = ReactMount.renderComponent(\2,\3' This still matches many unnecessary cases where the second occurance of the identifier is a redeclaration or comment. But this code mod doesn't hurt in those cases. Finally I have to do the same for: this. = React.renderComponent(this., This is a common pattern for production code but not tests. Some of these call sites will likely break when we move to true descriptors. scripts/bin/codemod -m -d ~/www --extensions js \ '^(\s*)React.renderComponent\((\s*)this\.([$a-zA-Z0-9\_\.]+)\s*?,' \ '\1this.\3 = React.renderComponent(\2this.\3,' --- src/browser/ui/__tests__/ReactDOMComponent-test.js | 2 +- .../__tests__/ReactEventTopLevelCallback-test.js | 14 +++++++------- .../ui/dom/__tests__/CSSPropertyOperations-test.js | 2 +- .../components/__tests__/ReactDOMButton-test.js | 2 +- src/core/__tests__/ReactComponent-test.js | 12 ++++++------ .../ReactCompositeComponentDOMMinimalism-test.js | 6 +++--- .../__tests__/ReactCompositeComponentError-test.js | 2 +- .../__tests__/ReactCompositeComponentMixin-test.js | 4 ++-- .../__tests__/ReactCompositeComponentState-test.js | 2 +- src/core/__tests__/ReactIdentity-test.js | 8 ++++---- src/core/__tests__/ReactPropTransferer-test.js | 8 ++++---- src/core/__tests__/ReactPropTypes-test.js | 4 ++-- src/core/__tests__/ReactStateSetters-test.js | 12 ++++++------ src/core/__tests__/ReactUpdates-test.js | 4 ++-- src/utils/__tests__/sliceChildren-test.js | 4 ++-- 15 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/browser/ui/__tests__/ReactDOMComponent-test.js b/src/browser/ui/__tests__/ReactDOMComponent-test.js index de94aa62a8..f49941b132 100644 --- a/src/browser/ui/__tests__/ReactDOMComponent-test.js +++ b/src/browser/ui/__tests__/ReactDOMComponent-test.js @@ -351,7 +351,7 @@ describe('ReactDOMComponent', function() { var callback = function() {}; var instance =
; - React.renderComponent(instance, container); + instance = React.renderComponent(instance, container); var rootNode = instance.getDOMNode(); var rootNodeID = ReactMount.getID(rootNode); diff --git a/src/browser/ui/__tests__/ReactEventTopLevelCallback-test.js b/src/browser/ui/__tests__/ReactEventTopLevelCallback-test.js index aeca33e6cc..03230cc357 100644 --- a/src/browser/ui/__tests__/ReactEventTopLevelCallback-test.js +++ b/src/browser/ui/__tests__/ReactEventTopLevelCallback-test.js @@ -44,8 +44,8 @@ describe('ReactEventTopLevelCallback', function() { var childControl =
Child
; var parentContainer = document.createElement('div'); var parentControl =
Parent
; - ReactMount.renderComponent(childControl, childContainer); - ReactMount.renderComponent(parentControl, parentContainer); + childControl = ReactMount.renderComponent(childControl, childContainer); + parentControl = ReactMount.renderComponent(parentControl, parentContainer); parentControl.getDOMNode().appendChild(childContainer); var callback = ReactEventTopLevelCallback.createTopLevelCallback('test'); @@ -66,9 +66,9 @@ describe('ReactEventTopLevelCallback', function() { var parentControl =
Parent
; var grandParentContainer = document.createElement('div'); var grandParentControl =
Parent
; - ReactMount.renderComponent(childControl, childContainer); - ReactMount.renderComponent(parentControl, parentContainer); - ReactMount.renderComponent(grandParentControl, grandParentContainer); + childControl = ReactMount.renderComponent(childControl, childContainer); + parentControl = ReactMount.renderComponent(parentControl, parentContainer); + grandParentControl = ReactMount.renderComponent(grandParentControl, grandParentContainer); parentControl.getDOMNode().appendChild(childContainer); grandParentControl.getDOMNode().appendChild(parentContainer); @@ -90,8 +90,8 @@ describe('ReactEventTopLevelCallback', function() { var childControl =
Child
; var parentContainer = document.createElement('div'); var parentControl =
Parent
; - ReactMount.renderComponent(childControl, childContainer); - ReactMount.renderComponent(parentControl, parentContainer); + childControl = ReactMount.renderComponent(childControl, childContainer); + parentControl = ReactMount.renderComponent(parentControl, parentContainer); parentControl.getDOMNode().appendChild(childContainer); // ReactEventEmitter.handleTopLevel might remove the target from the DOM. diff --git a/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js b/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js index cc5fcf37fa..6411adfebe 100644 --- a/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js +++ b/src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js @@ -86,7 +86,7 @@ describe('CSSPropertyOperations', function() { }; var div =
; var root = document.createElement('div'); - React.renderComponent(div, root); + div = React.renderComponent(div, root); expect(/style=".*"/.test(root.innerHTML)).toBe(true); }); diff --git a/src/browser/ui/dom/components/__tests__/ReactDOMButton-test.js b/src/browser/ui/dom/components/__tests__/ReactDOMButton-test.js index b529f630d5..e9dbf98dd0 100644 --- a/src/browser/ui/dom/components/__tests__/ReactDOMButton-test.js +++ b/src/browser/ui/dom/components/__tests__/ReactDOMButton-test.js @@ -42,7 +42,7 @@ describe('ReactDOMButton', function() { } function mounted(button) { - ReactTestUtils.renderIntoDocument(button); + button = ReactTestUtils.renderIntoDocument(button); return button; } diff --git a/src/core/__tests__/ReactComponent-test.js b/src/core/__tests__/ReactComponent-test.js index 1899cfb7cc..f0fdf7f634 100644 --- a/src/core/__tests__/ReactComponent-test.js +++ b/src/core/__tests__/ReactComponent-test.js @@ -52,7 +52,7 @@ describe('ReactComponent', function() { it('should throw when supplying a ref outside of render method', function() { var instance =
; expect(function() { - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); }).toThrow(); }); @@ -68,7 +68,7 @@ describe('ReactComponent', function() { var instance = } />; expect(function() { - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); }).toThrow( 'Invariant Violation: attachRef(test, ...): Only a component\'s owner ' + 'can store a ref to it.' @@ -91,7 +91,7 @@ describe('ReactComponent', function() { }); var instance = } />; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); }); it('should not have refs on unmounted components', function() { @@ -110,7 +110,7 @@ describe('ReactComponent', function() { }); var instance = } />; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); }); it('should correctly determine if a component is mounted', function() { @@ -146,7 +146,7 @@ describe('ReactComponent', function() { }); var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(instance._mountDepth).toBe(0); expect(instance.refs.child._mountDepth).toBe(1); }); @@ -197,7 +197,7 @@ describe('ReactComponent', function() { }); var root = ; - ReactTestUtils.renderIntoDocument(root); + root = ReactTestUtils.renderIntoDocument(root); expect(root._mountDepth).toBe(0); expect(root.refs.switcher._mountDepth).toBe(1); diff --git a/src/core/__tests__/ReactCompositeComponentDOMMinimalism-test.js b/src/core/__tests__/ReactCompositeComponentDOMMinimalism-test.js index a1bd918a5f..cd2c9f11e7 100644 --- a/src/core/__tests__/ReactCompositeComponentDOMMinimalism-test.js +++ b/src/core/__tests__/ReactCompositeComponentDOMMinimalism-test.js @@ -78,7 +78,7 @@ describe('ReactCompositeComponentDOMMinimalism', function() { A string child ); - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expectSingleChildlessDiv(instance); }); @@ -88,7 +88,7 @@ describe('ReactCompositeComponentDOMMinimalism', function() { {'Interpolated String Child'} ); - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expectSingleChildlessDiv(instance); }); @@ -100,7 +100,7 @@ describe('ReactCompositeComponentDOMMinimalism', function() { ); - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); reactComponentExpect(instance) .expectRenderedChild() .toBeCompositeComponentWithType(LowerLevelComposite) diff --git a/src/core/__tests__/ReactCompositeComponentError-test.js b/src/core/__tests__/ReactCompositeComponentError-test.js index 35b17c0a5a..587ebe53f1 100644 --- a/src/core/__tests__/ReactCompositeComponentError-test.js +++ b/src/core/__tests__/ReactCompositeComponentError-test.js @@ -35,7 +35,7 @@ describe('ReactCompositeComponent-error', function() { }); var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(ReactErrorUtils.guard.mostRecentCall.args[1]) .toEqual('Component.someHandler'); }); diff --git a/src/core/__tests__/ReactCompositeComponentMixin-test.js b/src/core/__tests__/ReactCompositeComponentMixin-test.js index cc9c30b07c..482019ab57 100644 --- a/src/core/__tests__/ReactCompositeComponentMixin-test.js +++ b/src/core/__tests__/ReactCompositeComponentMixin-test.js @@ -103,7 +103,7 @@ describe('ReactCompositeComponent-mixin', function() { it('should support merging propTypes and statics', function() { var listener = mocks.getMockFunction(); var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); var instancePropTypes = instance.constructor.propTypes; @@ -118,7 +118,7 @@ describe('ReactCompositeComponent-mixin', function() { it('should support chaining delegate functions', function() { var listener = mocks.getMockFunction(); var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(listener.mock.calls).toEqual([ ['MixinA didMount'], diff --git a/src/core/__tests__/ReactCompositeComponentState-test.js b/src/core/__tests__/ReactCompositeComponentState-test.js index b5dd6f48e6..fe9c6bff7d 100644 --- a/src/core/__tests__/ReactCompositeComponentState-test.js +++ b/src/core/__tests__/ReactCompositeComponentState-test.js @@ -109,7 +109,7 @@ describe('ReactCompositeComponent-state', function() { it('should support setting state', function() { var stateListener = mocks.getMockFunction(); var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); instance.setProps({nextColor: 'green'}); instance.setFavoriteColor('blue'); instance.forceUpdate(); diff --git a/src/core/__tests__/ReactIdentity-test.js b/src/core/__tests__/ReactIdentity-test.js index 1524c71d3a..1f7ba7be36 100644 --- a/src/core/__tests__/ReactIdentity-test.js +++ b/src/core/__tests__/ReactIdentity-test.js @@ -52,7 +52,7 @@ describe('ReactIdentity', function() { }}
; - React.renderComponent(instance, document.createElement('div')); + instance = React.renderComponent(instance, document.createElement('div')); var node = instance.getDOMNode(); reactComponentExpect(instance).toBeDOMComponentWithChildCount(2); checkId(node.childNodes[0], '.0.$first:0'); @@ -68,7 +68,7 @@ describe('ReactIdentity', function() {
; - React.renderComponent(instance, document.createElement('div')); + instance = React.renderComponent(instance, document.createElement('div')); var node = instance.getDOMNode(); reactComponentExpect(instance).toBeDOMComponentWithChildCount(4); checkId(node.childNodes[0], '.0.$apple'); @@ -92,7 +92,7 @@ describe('ReactIdentity', function() {
; - React.renderComponent(instance, document.createElement('div')); + instance = React.renderComponent(instance, document.createElement('div')); var node = instance.getDOMNode(); reactComponentExpect(instance).toBeDOMComponentWithChildCount(3); @@ -282,7 +282,7 @@ describe('ReactIdentity', function() { var wrapped = ; - React.renderComponent(wrapped, document.createElement('div')); + wrapped = React.renderComponent(wrapped, document.createElement('div')); var beforeID = ReactMount.getID(wrapped.getDOMNode().firstChild); diff --git a/src/core/__tests__/ReactPropTransferer-test.js b/src/core/__tests__/ReactPropTransferer-test.js index b840b56352..4e20f84adc 100644 --- a/src/core/__tests__/ReactPropTransferer-test.js +++ b/src/core/__tests__/ReactPropTransferer-test.js @@ -48,7 +48,7 @@ describe('ReactPropTransferer', function() { it('should leave explicitly specified properties intact', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); reactComponentExpect(instance) .expectRenderedChild() @@ -63,7 +63,7 @@ describe('ReactPropTransferer', function() { it('should transfer unspecified properties', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); reactComponentExpect(instance) .expectRenderedChild() @@ -77,7 +77,7 @@ describe('ReactPropTransferer', function() { className="hidden_elem" style={{width: '100%'}} />; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); reactComponentExpect(instance) .expectRenderedChild() @@ -103,7 +103,7 @@ describe('ReactPropTransferer', function() { Hello! ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); reactComponentExpect(instance) .expectRenderedChild() .toBeDOMComponentWithTag('div') diff --git a/src/core/__tests__/ReactPropTypes-test.js b/src/core/__tests__/ReactPropTypes-test.js index ece37be5c6..c5ca40cc2c 100644 --- a/src/core/__tests__/ReactPropTypes-test.js +++ b/src/core/__tests__/ReactPropTypes-test.js @@ -455,7 +455,7 @@ describe('Component Type', function() { it('should be able to define a single child as label', () => { var instance = } />; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); // No warnings should have been logged. expect(console.warn.mock.calls.length).toBe(0); @@ -463,7 +463,7 @@ describe('Component Type', function() { it('should warn when passing no label and isRequired is set', () => { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(console.warn.mock.calls.length).toBe(1); }); diff --git a/src/core/__tests__/ReactStateSetters-test.js b/src/core/__tests__/ReactStateSetters-test.js index ed57e77f01..9d5661175a 100644 --- a/src/core/__tests__/ReactStateSetters-test.js +++ b/src/core/__tests__/ReactStateSetters-test.js @@ -55,7 +55,7 @@ describe('ReactStateSetters', function() { it('createStateSetter should update state', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(instance.state).toEqual({foo: 'foo'}); var setter = ReactStateSetters.createStateSetter( @@ -78,7 +78,7 @@ describe('ReactStateSetters', function() { it('createStateKeySetter should update state', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(instance.state).toEqual({foo: 'foo'}); var setter = ReactStateSetters.createStateKeySetter(instance, 'foo'); @@ -94,7 +94,7 @@ describe('ReactStateSetters', function() { it('createStateKeySetter is memoized', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(instance.state).toEqual({foo: 'foo'}); var foo1 = ReactStateSetters.createStateKeySetter(instance, 'foo'); @@ -109,7 +109,7 @@ describe('ReactStateSetters', function() { it('createStateSetter should update state from mixin', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(instance.state).toEqual({foo: 'foo'}); var setter = instance.createStateSetter( @@ -131,7 +131,7 @@ describe('ReactStateSetters', function() { it('createStateKeySetter should update state with mixin', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(instance.state).toEqual({foo: 'foo'}); var setter = instance.createStateKeySetter('foo'); @@ -147,7 +147,7 @@ describe('ReactStateSetters', function() { it('createStateKeySetter is memoized with mixin', function() { var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(instance.state).toEqual({foo: 'foo'}); var foo1 = instance.createStateKeySetter('foo'); diff --git a/src/core/__tests__/ReactUpdates-test.js b/src/core/__tests__/ReactUpdates-test.js index 01accb6b93..690af3b443 100644 --- a/src/core/__tests__/ReactUpdates-test.js +++ b/src/core/__tests__/ReactUpdates-test.js @@ -318,7 +318,7 @@ describe('ReactUpdates', function() { expect(childRenderCount).toBe(0); var instance = ; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); expect(parentRenderCount).toBe(1); expect(childRenderCount).toBe(1); @@ -437,7 +437,7 @@ describe('ReactUpdates', function() { }); var root = ; - ReactTestUtils.renderIntoDocument(root); + root = ReactTestUtils.renderIntoDocument(root); function expectUpdates(desiredWillUpdates, desiredDidUpdates) { expect(willUpdates).toEqual(desiredWillUpdates); diff --git a/src/utils/__tests__/sliceChildren-test.js b/src/utils/__tests__/sliceChildren-test.js index 5ccc826867..195bcd4e6e 100644 --- a/src/utils/__tests__/sliceChildren-test.js +++ b/src/utils/__tests__/sliceChildren-test.js @@ -53,7 +53,7 @@ describe('sliceChildren', function() { function renderAndSlice(set, start, end) { var instance = {set}; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); var rendered = reactComponentExpect(instance) .expectRenderedChild() .instance(); @@ -106,7 +106,7 @@ describe('sliceChildren', function() { var c =
; var instance = {a}{b}{c}; - ReactTestUtils.renderIntoDocument(instance); + instance = ReactTestUtils.renderIntoDocument(instance); var rendered = reactComponentExpect(instance) .expectRenderedChild() .instance();