Merge pull request #2808 from sebmarkbage/modernclasses

New class instantiation and initialization process

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(

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) {
-    // Merge with `_pendingState` if it exists, otherwise with existing state.
+    var compositeLifeCycleState = this._compositeLifeCycleState;
-    this.replaceState(
+    invariant(
-      assign({}, this._pendingState || this._instance.state, partialState),
+      ReactCurrentOwner.current == null,
-      callback
+      '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._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;
+    var initialState = inst.state;
-    if (__DEV__) {
+    if (initialState === undefined) {
-      // We allow auto-mocks to proceed as if they're returning null.
+      inst.state = initialState = null;
-      if (typeof initialState === 'undefined' &&
-          inst.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',
+      '%s.state: must be set to an object or null',
       this.getName() || 'ReactCompositeComponent'
     );
-    inst.state = initialState;
 
     this._pendingState = null;
     this._pendingForceUpdate = false;

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 = <StatefulComponent />;
     expect(function() {
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      instance = ReactTestUtils.renderIntoDocument(<StatefulComponent />);
-    }).not.toThrow();
+    }).toThrow(
-
+      'Invariant Violation: setState(...): Can only update a mounted or ' +
-    // The return value of getInitialState overrides anything from setState
+      'mounting component. This usually means you called setState() on an ' + 
-    expect(instance.state.stateField).toEqual('somethingelse');
+      '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(

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 ' +
+          'Invariant Violation: setState(...): Cannot call setState() on an ' +
-          'unmounting component. This usually means you called setState() ' +
+          'unmounting component.'
-          'on an unmounted 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 ' +
+      'Invariant Violation: setProps(...): Can only update a mounted or ' +
-      'component.'
+      'mounting component. This usually means you called setProps() on an ' +
+      'unmounted component.'
     );
   });
 

src/modern/class/__tests__/ReactES6Class-test.js

@@ -65,6 +65,129 @@ describe('ReactES6Class', function() {
     test(<Foo bar="bar" />, '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 <span className={this.state.bar} />;
+      }
+    }
+    test(<Foo initialValue="foo" />, '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 <div className="foo" />;
+        }
+        return <span className={this.state.bar} />;
+      }
+    }
+    var instance = test(<Foo initialValue="foo" />, 'DIV', 'foo');
+    instance.changeState();
+    test(<Foo />, '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 <span className={this.state.bar} />;
+      }
+    }
+    test(<Foo initialValue="foo" />, '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 <span />;
+        }
+      }
+      expect(() => test(<Foo />, '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 <span />;
+      }
+    }
+    test(<Foo />, '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 (
+          <Inner
+            name={this.state.bar}
+            onClick={this.handleClick.bind(this)}
+          />
+        );
+      }
+    }
+    test(<Foo initialValue="foo" />, '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 (
+          <Inner
+            name={this.state.bar}
+            onClick={this.handleClick}
+          />
+        );
+      }
+    }
+    test(<Foo initialValue="foo" />, '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 <span className={this.props.value} />;
+      }
+    }
+    var instance = test(<Foo value="foo" />, 'SPAN', 'foo');
+    expect(lifeCycles).toEqual([
+      'will-mount',
+      'did-mount'
+    ]);
+    lifeCycles = []; // reset
+    test(<Foo value="bar" />, '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 <span className="foo" />;
       }
     }
     test(<Foo />, 'SPAN', 'foo');
-    // TODO: expect(getInitialStateWasCalled).toBe(false);
+    expect(getInitialStateWasCalled).toBe(false);
-    // TODO: expect(componentWillMountWasCalled).toBe(false);
+    expect(console.warn.calls.length).toBe(3);
-    expect(console.warn.calls.length).toBe(4);
     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.'
     );
   });