@samitouri / QOS-React-2 / commits / 291a92c4a6

New class instantiation and initialization process

This allows state to be set up in the constructor instead of through getInitialState. getInitialState is now considered part of "classic". Therefore, they move into ReactClass's constructor. As a consequence of this, we no longer have a mapping between the internal representation and the public instance during the mounting process. Because the constructor hasn't returned yet. We used to have a special case for calling setState in getInitialState which was just ignored. This makes that throw and the component is considered unmounted during the construction phase.

Sebastian Markbage committed Jan 20, 2015 at 14:27 UTC 291a92c4a6d8a3b00121f2fa8cf398cf22d73449

5 files changed +271 -78

src/classic/class/ReactClass.js

+26 -4

@@ -742,7 +742,8 @@ var ReactClassMixin = {

742 var internalInstance = ReactInstanceMap.get(this);

743 invariant(

744 internalInstance,

745 - 'setProps(...): Can only update a mounted component.'

745 + 'setProps(...): Can only update a mounted or mounting component. ' +

746 + 'This usually means you called setProps() on an unmounted component.'

747 );

748 internalInstance.setProps(

749 partialProps,

@@ -797,6 +798,30 @@ var ReactClass = {

798 if (this.__reactAutoBindMap) {

799 bindAutoBindMethods(this);

800 }

801 +

802 + this.props = props;

803 + this.state = null;

804 +

805 + // ReactClasses doesn't have constructors. Instead, they use the

806 + // getInitialState and componentWillMount methods for initialization.

807 +

808 + var initialState = this.getInitialState ? this.getInitialState() : null;

809 + if (__DEV__) {

810 + // We allow auto-mocks to proceed as if they're returning null.

811 + if (typeof initialState === 'undefined' &&

812 + this.getInitialState._isMockFunction) {

813 + // This is probably bad practice. Consider warning here and

814 + // deprecating this convenience.

815 + initialState = null;

816 + }

817 + }

818 + invariant(

819 + typeof initialState === 'object' && !Array.isArray(initialState),

820 + '%s.getInitialState(): must return an object or null',

821 + Constructor.displayName || 'ReactCompositeComponent'

822 + );

823 +

824 + this.state = initialState;

825 };

826 Constructor.prototype = new ReactClassBase();

827 Constructor.prototype.constructor = Constructor;

@@ -823,9 +848,6 @@ var ReactClass = {

848 if (Constructor.prototype.getInitialState) {

849 Constructor.prototype.getInitialState.isReactClassApproved = {};

850 }

826 - if (Constructor.prototype.componentWillMount) {

827 - Constructor.prototype.componentWillMount.isReactClassApproved = {};

828 - }

851 }

852

853 invariant(

src/core/ReactCompositeComponent.js

+48 -47

@@ -40,20 +40,6 @@ function getDeclarationErrorAddendum(component) {

40 return '';

41 }

43 -function validateLifeCycleOnReplaceState(instance) {

44 - var compositeLifeCycleState = instance._compositeLifeCycleState;

45 - invariant(

46 - ReactCurrentOwner.current == null,

47 - 'replaceState(...): Cannot update during an existing state transition ' +

48 - '(such as within `render`). Render methods should be a pure function ' +

49 - 'of props and state.'

50 - );

51 - invariant(compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING,

52 - 'replaceState(...): Cannot update while unmounting component. This ' +

53 - 'usually means you called setState() on an unmounted component.'

54 - );

55 -}

56 -

43 /**

44 * `ReactCompositeComponent` maintains an auxiliary life cycle state in

45 * `this._compositeLifeCycleState` (which can be null).

@@ -123,7 +109,8 @@ var ReactCompositeComponentMixin = assign({},

109 this._rootNodeID = null;

110

111 this._instance.props = element.props;

126 - this._instance.state = null;

112 + // instance.state get set up to its proper initial value in mount

113 + // which may be null.

114 this._instance.context = null;

115 this._instance.refs = emptyObject;

116

@@ -190,15 +177,7 @@ var ReactCompositeComponentMixin = assign({},

177 }

178 inst.props = this._processProps(this._currentElement.props);

179

193 - var initialState = inst.getInitialState ? inst.getInitialState() : null;

180 if (__DEV__) {

195 - // We allow auto-mocks to proceed as if they're returning null.

196 - if (typeof initialState === 'undefined' &&

197 - inst.getInitialState._isMockFunction) {

198 - // This is probably bad practice. Consider warning here and

199 - // deprecating this convenience.

200 - initialState = null;

201 - }

181 // Since plain JS classes are defined without any special initialization

182 // logic, we can not catch common errors early. Therefore, we have to

183 // catch them here, at initialization time, instead.

@@ -210,14 +189,6 @@ var ReactCompositeComponentMixin = assign({},

189 'Did you mean to define a state property instead?',

190 this.getName() || 'a component'

191 );

213 - warning(

214 - !inst.componentWillMount ||

215 - inst.componentWillMount.isReactClassApproved,

216 - 'componentWillMount was defined on %s, a plain JavaScript class. ' +

217 - 'This is only supported for classes created using React.createClass. ' +

218 - 'Did you mean to define a constructor instead?',

219 - this.getName() || 'a component'

220 - );

192 warning(

193 !inst.propTypes,

194 'propTypes was defined as an instance property on %s. Use a static ' +

@@ -239,9 +210,14 @@ var ReactCompositeComponentMixin = assign({},

210 (this.getName() || 'A component')

211 );

212 }

213 +

214 + var initialState = inst.state;

215 + if (initialState === undefined) {

216 + inst.state = initialState = null;

217 + }

218 invariant(

219 typeof initialState === 'object' && !Array.isArray(initialState),

244 - '%s.getInitialState(): must return an object or null',

220 + '%s.state: must be set to an object or null',

221 this.getName() || 'ReactCompositeComponent'

222 );

223 inst.state = initialState;

@@ -396,11 +372,32 @@ var ReactCompositeComponentMixin = assign({},

372 * @protected

373 */

374 setState: function(partialState, callback) {

375 + var compositeLifeCycleState = this._compositeLifeCycleState;

376 + invariant(

377 + ReactCurrentOwner.current == null,

378 + 'setState(...): Cannot update during an existing state transition ' +

379 + '(such as within `render`). Render methods should be a pure function ' +

380 + 'of props and state.'

381 + );

382 + invariant(

383 + compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING,

384 + 'setState(...): Cannot call setState() on an unmounting component.'

385 + );

386 // Merge with `_pendingState` if it exists, otherwise with existing state.

400 - this.replaceState(

401 - assign({}, this._pendingState || this._instance.state, partialState),

402 - callback

387 + this._pendingState = assign(

388 + {},

389 + this._pendingState || this._instance.state,

390 + partialState

391 );

392 + if (this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING) {

393 + // If we're in a componentWillMount handler, don't enqueue a rerender

394 + // because ReactUpdates assumes we're in a browser context (which is wrong

395 + // for server rendering) and we're about to do a render anyway.

396 + // TODO: The callback here is ignored when setState is called from

397 + // componentWillMount. Either fix it or disallow doing so completely in

398 + // favor of getInitialState.

399 + ReactUpdates.enqueueUpdate(this, callback);

400 + }

401 },

402

403 /**

@@ -416,7 +413,18 @@ var ReactCompositeComponentMixin = assign({},

413 * @protected

414 */

415 replaceState: function(completeState, callback) {

419 - validateLifeCycleOnReplaceState(this);

416 + var compositeLifeCycleState = this._compositeLifeCycleState;

417 + invariant(

418 + ReactCurrentOwner.current == null,

419 + 'replaceState(...): Cannot update during an existing state transition ' +

420 + '(such as within `render`). Render methods should be a pure function ' +

421 + 'of props and state.'

422 + );

423 + invariant(

424 + compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING,

425 + 'replaceState(...): Cannot call replaceState() on an unmounting ' +

426 + 'component.'

427 + );

428 this._pendingState = completeState;

429 if (this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING) {

430 // If we're in a componentWillMount handler, don't enqueue a rerender

@@ -1004,22 +1012,15 @@ var ShallowMixin = assign({},

1012 // No context for shallow-mounted components.

1013 inst.props = this._processProps(this._currentElement.props);

1014

1007 - var initialState = inst.getInitialState ? inst.getInitialState() : null;

1008 - if (__DEV__) {

1009 - // We allow auto-mocks to proceed as if they're returning null.

1010 - if (typeof initialState === 'undefined' &&

1011 - inst.getInitialState._isMockFunction) {

1012 - // This is probably bad practice. Consider warning here and

1013 - // deprecating this convenience.

1014 - initialState = null;

1015 - }

1015 + var initialState = inst.state;

1016 + if (initialState === undefined) {

1017 + inst.state = initialState = null;

1018 }

1019 invariant(

1020 typeof initialState === 'object' && !Array.isArray(initialState),

1019 - '%s.getInitialState(): must return an object or null',

1021 + '%s.state: must be set to an object or null',

1022 this.getName() || 'ReactCompositeComponent'

1023 );

1022 - inst.state = initialState;

1024

1025 this._pendingState = null;

1026 this._pendingForceUpdate = false;

src/core/__tests__/ReactComponentLifeCycle-test.js

+14 -11

@@ -104,7 +104,11 @@ describe('ReactComponentLifeCycle', function() {

104 ReactInstanceMap = require('ReactInstanceMap');

105

106 getCompositeLifeCycle = function(instance) {

107 - return ReactInstanceMap.get(instance)._compositeLifeCycleState;

107 + var internalInstance = ReactInstanceMap.get(instance);

108 + if (!internalInstance) {

109 + return null;

110 + }

111 + return internalInstance._compositeLifeCycleState;

112 };

113

114 getLifeCycleState = function(instance) {

@@ -221,7 +225,7 @@ describe('ReactComponentLifeCycle', function() {

225 }).not.toThrow();

226 });

227

224 - it('should allow update state inside of getInitialState', function() {

228 + it('should not allow update state inside of getInitialState', function() {

229 var StatefulComponent = React.createClass({

230 getInitialState: function() {

231 this.setState({stateField: 'something'});

@@ -234,16 +238,15 @@ describe('ReactComponentLifeCycle', function() {

238 );

239 }

240 });

237 - var instance = <StatefulComponent />;

241 expect(function() {

239 - instance = ReactTestUtils.renderIntoDocument(instance);

240 - }).not.toThrow();

241 -

242 - // The return value of getInitialState overrides anything from setState

243 - expect(instance.state.stateField).toEqual('somethingelse');

242 + instance = ReactTestUtils.renderIntoDocument(<StatefulComponent />);

243 + }).toThrow(

244 + 'Invariant Violation: setState(...): Can only update a mounted or ' +

245 + 'mounting component. This usually means you called setState() on an ' +

246 + 'unmounted component.'

247 + );

248 });

249

246 -

250 it('should carry through each of the phases of setup', function() {

251 var LifeCycleComponent = React.createClass({

252 getInitialState: function() {

@@ -317,9 +320,9 @@ describe('ReactComponentLifeCycle', function() {

320 GET_INIT_STATE_RETURN_VAL

321 );

322 expect(instance._testJournal.lifeCycleAtStartOfGetInitialState)

320 - .toBe(ComponentLifeCycle.MOUNTED);

323 + .toBe(ComponentLifeCycle.UNMOUNTED);

324 expect(instance._testJournal.compositeLifeCycleAtStartOfGetInitialState)

322 - .toBe(CompositeComponentLifeCycle.MOUNTING);

325 + .toBe(null);

326

327 // componentWillMount

328 expect(instance._testJournal.stateAtStartOfWillMount).toEqual(

src/core/__tests__/ReactCompositeComponent-test.js

+5 -5

@@ -343,9 +343,8 @@ describe('ReactCompositeComponent', function() {

343 },

344 componentWillUnmount: function() {

345 expect(() => this.setState({ value: 2 })).toThrow(

346 - 'Invariant Violation: replaceState(...): Cannot update while ' +

347 - 'unmounting component. This usually means you called setState() ' +

348 - 'on an unmounted component.'

346 + 'Invariant Violation: setState(...): Cannot call setState() on an ' +

347 + 'unmounting component.'

348 );

349 },

350 render: function() {

@@ -383,8 +382,9 @@ describe('ReactCompositeComponent', function() {

382 expect(function() {

383 instance.setProps({ value: 2 });

384 }).toThrow(

386 - 'Invariant Violation: setProps(...): Can only update a mounted ' +

387 - 'component.'

385 + 'Invariant Violation: setProps(...): Can only update a mounted or ' +

386 + 'mounting component. This usually means you called setProps() on an ' +

387 + 'unmounted component.'

388 );

389 });

390

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

+178 -11

@@ -65,6 +65,129 @@ describe('ReactES6Class', function() {

65 test(<Foo bar="bar" />, 'DIV', 'bar');

66 });

68 + it('renders based on state using initial values in this.props', function() {

69 + class Foo extends React.Component {

70 + constructor(props) {

71 + super(props);

72 + this.state = { bar: this.props.initialValue };

73 + }

74 + render() {

75 + return ;

76 + }

77 + }

78 + test(<Foo initialValue="foo" />, 'SPAN', 'foo');

79 + });

80 +

81 + it('renders based on state using props in the constructor', function() {

82 + class Foo extends React.Component {

83 + constructor(props) {

84 + this.state = { bar: props.initialValue };

85 + }

86 + changeState() {

87 + this.setState({ bar: 'bar' });

88 + }

89 + render() {

90 + if (this.state.bar === 'foo') {

91 + return <div className="foo" />;

92 + }

93 + return ;

94 + }

95 + }

96 + var instance = test(<Foo initialValue="foo" />, 'DIV', 'foo');

97 + instance.changeState();

98 + test(<Foo />, 'SPAN', 'bar');

99 + });

100 +

101 + it('renders only once when setting state in componentWillMount', function() {

102 + var renderCount = 0;

103 + class Foo extends React.Component {

104 + constructor(props) {

105 + this.state = { bar: props.initialValue };

106 + }

107 + componentWillMount() {

108 + this.setState({ bar: 'bar' });

109 + }

110 + render() {

111 + renderCount++;

112 + return ;

113 + }

114 + }

115 + test(<Foo initialValue="foo" />, 'SPAN', 'bar');

116 + expect(renderCount).toBe(1);

117 + });

118 +

119 + it('should throw with non-object in the initial state property', function() {

120 + [['an array'], 'a string', 1234].forEach(function(state) {

121 + class Foo {

122 + constructor() {

123 + this.state = state;

124 + }

125 + render() {

126 + return ;

127 + }

128 + }

129 + expect(() => test(<Foo />, 'span', '')).toThrow(

130 + 'Invariant Violation: Foo.state: ' +

131 + 'must be set to an object or null'

132 + );

133 + });

134 + });

135 +

136 + it('should render with null in the initial state property', function() {

137 + class Foo extends React.Component {

138 + constructor() {

139 + this.state = null;

140 + }

141 + render() {

142 + return ;

143 + }

144 + }

145 + test(<Foo />, 'SPAN', '');

146 + });

147 +

148 + it('setState through an event handler', function() {

149 + class Foo extends React.Component {

150 + constructor(props) {

151 + this.state = { bar: props.initialValue };

152 + }

153 + handleClick() {

154 + this.setState({ bar: 'bar' });

155 + }

156 + render() {

157 + return (

158 + <Inner

159 + name={this.state.bar}

160 + onClick={this.handleClick.bind(this)}

161 + />

162 + );

163 + }

164 + }

165 + test(<Foo initialValue="foo" />, 'DIV', 'foo');

166 + attachedListener();

167 + expect(renderedName).toBe('bar');

168 + });

169 +

170 + it('should not implicitly bind event handlers', function() {

171 + class Foo extends React.Component {

172 + constructor(props) {

173 + this.state = { bar: props.initialValue };

174 + }

175 + handleClick() {

176 + this.setState({ bar: 'bar' });

177 + }

178 + render() {

179 + return (

180 + <Inner

181 + name={this.state.bar}

182 + onClick={this.handleClick}

183 + />

184 + );

185 + }

186 + }

187 + test(<Foo initialValue="foo" />, 'DIV', 'foo');

188 + expect(attachedListener).toThrow();

189 + });

190 +

191 it('renders using forceUpdate even when there is no state', function() {

192 class Foo extends React.Component {

193 constructor(props) {

@@ -88,11 +211,62 @@ describe('ReactES6Class', function() {

211 expect(renderedName).toBe('bar');

212 });

213

214 + it('will call all the normal life cycle methods', function() {

215 + var lifeCycles = [];

216 + class Foo {

217 + constructor() {

218 + this.state = {};

219 + }

220 + componentWillMount() {

221 + lifeCycles.push('will-mount');

222 + }

223 + componentDidMount() {

224 + lifeCycles.push('did-mount');

225 + }

226 + componentWillReceiveProps(nextProps) {

227 + lifeCycles.push('receive-props', nextProps);

228 + }

229 + shouldComponentUpdate(nextProps, nextState) {

230 + lifeCycles.push('should-update', nextProps, nextState);

231 + return true;

232 + }

233 + componentWillUpdate(nextProps, nextState) {

234 + lifeCycles.push('will-update', nextProps, nextState);

235 + }

236 + componentDidUpdate(prevProps, prevState) {

237 + lifeCycles.push('did-update', prevProps, prevState);

238 + }

239 + componentWillUnmount() {

240 + lifeCycles.push('will-unmount');

241 + }

242 + render() {

243 + return ;

244 + }

245 + }

246 + var instance = test(<Foo value="foo" />, 'SPAN', 'foo');

247 + expect(lifeCycles).toEqual([

248 + 'will-mount',

249 + 'did-mount'

250 + ]);

251 + lifeCycles = []; // reset

252 + test(<Foo value="bar" />, 'SPAN', 'bar');

253 + expect(lifeCycles).toEqual([

254 + 'receive-props', { value: 'bar' },

255 + 'should-update', { value: 'bar' }, {},

256 + 'will-update', { value: 'bar' }, {},

257 + 'did-update', { value: 'foo' }, {}

258 + ]);

259 + lifeCycles = []; // reset

260 + React.unmountComponentAtNode(container);

261 + expect(lifeCycles).toEqual([

262 + 'will-unmount'

263 + ]);

264 + });

265 +

266 it('warns when classic properties are defined on the instance, ' +

267 'but does not invoke them.', function() {

268 spyOn(console, 'warn');

269 var getInitialStateWasCalled = false;

95 - var componentWillMountWasCalled = false;

270 class Foo extends React.Component {

271 constructor() {

272 this.contextTypes = {};

@@ -102,27 +276,20 @@ describe('ReactES6Class', function() {

276 getInitialStateWasCalled = true;

277 return {};

278 }

105 - componentWillMount() {

106 - componentWillMountWasCalled = true;

107 - }

279 render() {

280 return ;

281 }

282 }

283 test(<Foo />, 'SPAN', 'foo');

113 - // TODO: expect(getInitialStateWasCalled).toBe(false);

114 - // TODO: expect(componentWillMountWasCalled).toBe(false);

115 - expect(console.warn.calls.length).toBe(4);

284 + expect(getInitialStateWasCalled).toBe(false);

285 + expect(console.warn.calls.length).toBe(3);

286 expect(console.warn.calls[0].args[0]).toContain(

287 'getInitialState was defined on Foo, a plain JavaScript class.'

288 );

289 expect(console.warn.calls[1].args[0]).toContain(

120 - 'componentWillMount was defined on Foo, a plain JavaScript class.'

121 - );

122 - expect(console.warn.calls[2].args[0]).toContain(

290 'propTypes was defined as an instance property on Foo.'

291 );

125 - expect(console.warn.calls[3].args[0]).toContain(

292 + expect(console.warn.calls[2].args[0]).toContain(

293 'contextTypes was defined as an instance property on Foo.'

294 );

295 });