diff --git a/src/classic/class/ReactClass.js b/src/classic/class/ReactClass.js index a206bd2038eff..62c668712a2b0 100644 --- a/src/classic/class/ReactClass.js +++ b/src/classic/class/ReactClass.js @@ -742,7 +742,8 @@ var ReactClassMixin = { var internalInstance = ReactInstanceMap.get(this); invariant( internalInstance, - 'setProps(...): Can only update a mounted component.' + 'setProps(...): Can only update a mounted or mounting component. ' + + 'This usually means you called setProps() on an unmounted component.' ); internalInstance.setProps( partialProps, @@ -797,6 +798,30 @@ var ReactClass = { if (this.__reactAutoBindMap) { bindAutoBindMethods(this); } + + this.props = props; + this.state = null; + + // ReactClasses doesn't have constructors. Instead, they use the + // getInitialState and componentWillMount methods for initialization. + + var initialState = this.getInitialState ? this.getInitialState() : null; + if (__DEV__) { + // We allow auto-mocks to proceed as if they're returning null. + if (typeof initialState === 'undefined' && + this.getInitialState._isMockFunction) { + // This is probably bad practice. Consider warning here and + // deprecating this convenience. + initialState = null; + } + } + invariant( + typeof initialState === 'object' && !Array.isArray(initialState), + '%s.getInitialState(): must return an object or null', + Constructor.displayName || 'ReactCompositeComponent' + ); + + this.state = initialState; }; Constructor.prototype = new ReactClassBase(); Constructor.prototype.constructor = Constructor; @@ -823,9 +848,6 @@ var ReactClass = { if (Constructor.prototype.getInitialState) { Constructor.prototype.getInitialState.isReactClassApproved = {}; } - if (Constructor.prototype.componentWillMount) { - Constructor.prototype.componentWillMount.isReactClassApproved = {}; - } } invariant( diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 41215f6cb1475..9b3f94b8cd2d3 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -40,20 +40,6 @@ function getDeclarationErrorAddendum(component) { return ''; } -function validateLifeCycleOnReplaceState(instance) { - var compositeLifeCycleState = instance._compositeLifeCycleState; - invariant( - ReactCurrentOwner.current == null, - 'replaceState(...): Cannot update during an existing state transition ' + - '(such as within `render`). Render methods should be a pure function ' + - 'of props and state.' - ); - invariant(compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING, - 'replaceState(...): Cannot update while unmounting component. This ' + - 'usually means you called setState() on an unmounted component.' - ); -} - /** * `ReactCompositeComponent` maintains an auxiliary life cycle state in * `this._compositeLifeCycleState` (which can be null). @@ -123,7 +109,8 @@ var ReactCompositeComponentMixin = assign({}, this._rootNodeID = null; this._instance.props = element.props; - this._instance.state = null; + // instance.state get set up to its proper initial value in mount + // which may be null. this._instance.context = null; this._instance.refs = emptyObject; @@ -190,15 +177,7 @@ var ReactCompositeComponentMixin = assign({}, } inst.props = this._processProps(this._currentElement.props); - var initialState = inst.getInitialState ? inst.getInitialState() : null; if (__DEV__) { - // We allow auto-mocks to proceed as if they're returning null. - if (typeof initialState === 'undefined' && - inst.getInitialState._isMockFunction) { - // This is probably bad practice. Consider warning here and - // deprecating this convenience. - initialState = null; - } // Since plain JS classes are defined without any special initialization // logic, we can not catch common errors early. Therefore, we have to // catch them here, at initialization time, instead. @@ -210,14 +189,6 @@ var ReactCompositeComponentMixin = assign({}, 'Did you mean to define a state property instead?', this.getName() || 'a component' ); - warning( - !inst.componentWillMount || - inst.componentWillMount.isReactClassApproved, - 'componentWillMount was defined on %s, a plain JavaScript class. ' + - 'This is only supported for classes created using React.createClass. ' + - 'Did you mean to define a constructor instead?', - this.getName() || 'a component' - ); warning( !inst.propTypes, 'propTypes was defined as an instance property on %s. Use a static ' + @@ -239,9 +210,14 @@ var ReactCompositeComponentMixin = assign({}, (this.getName() || 'A component') ); } + + var initialState = inst.state; + if (initialState === undefined) { + inst.state = initialState = null; + } invariant( typeof initialState === 'object' && !Array.isArray(initialState), - '%s.getInitialState(): must return an object or null', + '%s.state: must be set to an object or null', this.getName() || 'ReactCompositeComponent' ); inst.state = initialState; @@ -396,11 +372,32 @@ var ReactCompositeComponentMixin = assign({}, * @protected */ setState: function(partialState, callback) { + var compositeLifeCycleState = this._compositeLifeCycleState; + invariant( + ReactCurrentOwner.current == null, + 'setState(...): Cannot update during an existing state transition ' + + '(such as within `render`). Render methods should be a pure function ' + + 'of props and state.' + ); + invariant( + compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING, + 'setState(...): Cannot call setState() on an unmounting component.' + ); // Merge with `_pendingState` if it exists, otherwise with existing state. - this.replaceState( - assign({}, this._pendingState || this._instance.state, partialState), - callback + this._pendingState = assign( + {}, + this._pendingState || this._instance.state, + partialState ); + if (this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING) { + // If we're in a componentWillMount handler, don't enqueue a rerender + // because ReactUpdates assumes we're in a browser context (which is wrong + // for server rendering) and we're about to do a render anyway. + // TODO: The callback here is ignored when setState is called from + // componentWillMount. Either fix it or disallow doing so completely in + // favor of getInitialState. + ReactUpdates.enqueueUpdate(this, callback); + } }, /** @@ -416,7 +413,18 @@ var ReactCompositeComponentMixin = assign({}, * @protected */ replaceState: function(completeState, callback) { - validateLifeCycleOnReplaceState(this); + var compositeLifeCycleState = this._compositeLifeCycleState; + invariant( + ReactCurrentOwner.current == null, + 'replaceState(...): Cannot update during an existing state transition ' + + '(such as within `render`). Render methods should be a pure function ' + + 'of props and state.' + ); + invariant( + compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING, + 'replaceState(...): Cannot call replaceState() on an unmounting ' + + 'component.' + ); this._pendingState = completeState; if (this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING) { // If we're in a componentWillMount handler, don't enqueue a rerender @@ -1004,22 +1012,15 @@ var ShallowMixin = assign({}, // No context for shallow-mounted components. inst.props = this._processProps(this._currentElement.props); - var initialState = inst.getInitialState ? inst.getInitialState() : null; - if (__DEV__) { - // We allow auto-mocks to proceed as if they're returning null. - if (typeof initialState === 'undefined' && - inst.getInitialState._isMockFunction) { - // This is probably bad practice. Consider warning here and - // deprecating this convenience. - initialState = null; - } + var initialState = inst.state; + if (initialState === undefined) { + inst.state = initialState = null; } invariant( typeof initialState === 'object' && !Array.isArray(initialState), - '%s.getInitialState(): must return an object or null', + '%s.state: must be set to an object or null', this.getName() || 'ReactCompositeComponent' ); - inst.state = initialState; this._pendingState = null; this._pendingForceUpdate = false; diff --git a/src/core/__tests__/ReactComponentLifeCycle-test.js b/src/core/__tests__/ReactComponentLifeCycle-test.js index 8c108f1c806e1..1c69f7ebacff9 100644 --- a/src/core/__tests__/ReactComponentLifeCycle-test.js +++ b/src/core/__tests__/ReactComponentLifeCycle-test.js @@ -104,7 +104,11 @@ describe('ReactComponentLifeCycle', function() { ReactInstanceMap = require('ReactInstanceMap'); getCompositeLifeCycle = function(instance) { - return ReactInstanceMap.get(instance)._compositeLifeCycleState; + var internalInstance = ReactInstanceMap.get(instance); + if (!internalInstance) { + return null; + } + return internalInstance._compositeLifeCycleState; }; getLifeCycleState = function(instance) { @@ -221,7 +225,7 @@ describe('ReactComponentLifeCycle', function() { }).not.toThrow(); }); - it('should allow update state inside of getInitialState', function() { + it('should not allow update state inside of getInitialState', function() { var StatefulComponent = React.createClass({ getInitialState: function() { this.setState({stateField: 'something'}); @@ -234,16 +238,15 @@ describe('ReactComponentLifeCycle', function() { ); } }); - var instance = ; expect(function() { - instance = ReactTestUtils.renderIntoDocument(instance); - }).not.toThrow(); - - // The return value of getInitialState overrides anything from setState - expect(instance.state.stateField).toEqual('somethingelse'); + instance = ReactTestUtils.renderIntoDocument(); + }).toThrow( + 'Invariant Violation: setState(...): Can only update a mounted or ' + + 'mounting component. This usually means you called setState() on an ' + + 'unmounted component.' + ); }); - it('should carry through each of the phases of setup', function() { var LifeCycleComponent = React.createClass({ getInitialState: function() { @@ -317,9 +320,9 @@ describe('ReactComponentLifeCycle', function() { GET_INIT_STATE_RETURN_VAL ); expect(instance._testJournal.lifeCycleAtStartOfGetInitialState) - .toBe(ComponentLifeCycle.MOUNTED); + .toBe(ComponentLifeCycle.UNMOUNTED); expect(instance._testJournal.compositeLifeCycleAtStartOfGetInitialState) - .toBe(CompositeComponentLifeCycle.MOUNTING); + .toBe(null); // componentWillMount expect(instance._testJournal.stateAtStartOfWillMount).toEqual( diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index 908a0e0c97cf9..9e44e9e6bc7fc 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -343,9 +343,8 @@ describe('ReactCompositeComponent', function() { }, componentWillUnmount: function() { expect(() => this.setState({ value: 2 })).toThrow( - 'Invariant Violation: replaceState(...): Cannot update while ' + - 'unmounting component. This usually means you called setState() ' + - 'on an unmounted component.' + 'Invariant Violation: setState(...): Cannot call setState() on an ' + + 'unmounting component.' ); }, render: function() { @@ -383,8 +382,9 @@ describe('ReactCompositeComponent', function() { expect(function() { instance.setProps({ value: 2 }); }).toThrow( - 'Invariant Violation: setProps(...): Can only update a mounted ' + - 'component.' + 'Invariant Violation: setProps(...): Can only update a mounted or ' + + 'mounting component. This usually means you called setProps() on an ' + + 'unmounted component.' ); }); diff --git a/src/modern/class/__tests__/ReactES6Class-test.js b/src/modern/class/__tests__/ReactES6Class-test.js index b604d18550c73..807b67b74ae08 100644 --- a/src/modern/class/__tests__/ReactES6Class-test.js +++ b/src/modern/class/__tests__/ReactES6Class-test.js @@ -65,6 +65,129 @@ describe('ReactES6Class', function() { test(, 'DIV', 'bar'); }); + it('renders based on state using initial values in this.props', function() { + class Foo extends React.Component { + constructor(props) { + super(props); + this.state = { bar: this.props.initialValue }; + } + render() { + return ; + } + } + test(, 'SPAN', 'foo'); + }); + + it('renders based on state using props in the constructor', function() { + class Foo extends React.Component { + constructor(props) { + this.state = { bar: props.initialValue }; + } + changeState() { + this.setState({ bar: 'bar' }); + } + render() { + if (this.state.bar === 'foo') { + return
; + } + return ; + } + } + var instance = test(, 'DIV', 'foo'); + instance.changeState(); + test(, 'SPAN', 'bar'); + }); + + it('renders only once when setting state in componentWillMount', function() { + var renderCount = 0; + class Foo extends React.Component { + constructor(props) { + this.state = { bar: props.initialValue }; + } + componentWillMount() { + this.setState({ bar: 'bar' }); + } + render() { + renderCount++; + return ; + } + } + test(, 'SPAN', 'bar'); + expect(renderCount).toBe(1); + }); + + it('should throw with non-object in the initial state property', function() { + [['an array'], 'a string', 1234].forEach(function(state) { + class Foo { + constructor() { + this.state = state; + } + render() { + return ; + } + } + expect(() => test(, 'span', '')).toThrow( + 'Invariant Violation: Foo.state: ' + + 'must be set to an object or null' + ); + }); + }); + + it('should render with null in the initial state property', function() { + class Foo extends React.Component { + constructor() { + this.state = null; + } + render() { + return ; + } + } + test(, 'SPAN', ''); + }); + + it('setState through an event handler', function() { + class Foo extends React.Component { + constructor(props) { + this.state = { bar: props.initialValue }; + } + handleClick() { + this.setState({ bar: 'bar' }); + } + render() { + return ( + + ); + } + } + test(, 'DIV', 'foo'); + attachedListener(); + expect(renderedName).toBe('bar'); + }); + + it('should not implicitly bind event handlers', function() { + class Foo extends React.Component { + constructor(props) { + this.state = { bar: props.initialValue }; + } + handleClick() { + this.setState({ bar: 'bar' }); + } + render() { + return ( + + ); + } + } + test(, 'DIV', 'foo'); + expect(attachedListener).toThrow(); + }); + it('renders using forceUpdate even when there is no state', function() { class Foo extends React.Component { constructor(props) { @@ -88,11 +211,62 @@ describe('ReactES6Class', function() { expect(renderedName).toBe('bar'); }); + it('will call all the normal life cycle methods', function() { + var lifeCycles = []; + class Foo { + constructor() { + this.state = {}; + } + componentWillMount() { + lifeCycles.push('will-mount'); + } + componentDidMount() { + lifeCycles.push('did-mount'); + } + componentWillReceiveProps(nextProps) { + lifeCycles.push('receive-props', nextProps); + } + shouldComponentUpdate(nextProps, nextState) { + lifeCycles.push('should-update', nextProps, nextState); + return true; + } + componentWillUpdate(nextProps, nextState) { + lifeCycles.push('will-update', nextProps, nextState); + } + componentDidUpdate(prevProps, prevState) { + lifeCycles.push('did-update', prevProps, prevState); + } + componentWillUnmount() { + lifeCycles.push('will-unmount'); + } + render() { + return ; + } + } + var instance = test(, 'SPAN', 'foo'); + expect(lifeCycles).toEqual([ + 'will-mount', + 'did-mount' + ]); + lifeCycles = []; // reset + test(, 'SPAN', 'bar'); + expect(lifeCycles).toEqual([ + 'receive-props', { value: 'bar' }, + 'should-update', { value: 'bar' }, {}, + 'will-update', { value: 'bar' }, {}, + 'did-update', { value: 'foo' }, {} + ]); + lifeCycles = []; // reset + React.unmountComponentAtNode(container); + expect(lifeCycles).toEqual([ + 'will-unmount' + ]); + }); + it('warns when classic properties are defined on the instance, ' + 'but does not invoke them.', function() { spyOn(console, 'warn'); var getInitialStateWasCalled = false; - var componentWillMountWasCalled = false; class Foo extends React.Component { constructor() { this.contextTypes = {}; @@ -102,27 +276,20 @@ describe('ReactES6Class', function() { getInitialStateWasCalled = true; return {}; } - componentWillMount() { - componentWillMountWasCalled = true; - } render() { return ; } } test(, 'SPAN', 'foo'); - // TODO: expect(getInitialStateWasCalled).toBe(false); - // TODO: expect(componentWillMountWasCalled).toBe(false); - expect(console.warn.calls.length).toBe(4); + expect(getInitialStateWasCalled).toBe(false); + expect(console.warn.calls.length).toBe(3); expect(console.warn.calls[0].args[0]).toContain( 'getInitialState was defined on Foo, a plain JavaScript class.' ); expect(console.warn.calls[1].args[0]).toContain( - 'componentWillMount was defined on Foo, a plain JavaScript class.' - ); - expect(console.warn.calls[2].args[0]).toContain( 'propTypes was defined as an instance property on Foo.' ); - expect(console.warn.calls[3].args[0]).toContain( + expect(console.warn.calls[2].args[0]).toContain( 'contextTypes was defined as an instance property on Foo.' ); });