Allow assigning to this.state inside componentWillMount, with a warning
Andrew Clark committed
May 5, 2017 at 14:00 UTC
628b82c808af62832cf9a46b74810cb5cc899b40
3 files changed
+129
-70
scripts/fiber/tests-passing.txt
+1
@@ -506,6 +506,7 @@ src/renderers/__tests__/ReactCompositeComponentState-test.js
506
* should update state when called from child cWRP
507
* should merge state when sCU returns false
508
* should treat assigning to this.state inside cWRP as a replaceState, with a warning
509
+* should treat assigning to this.state inside cWM as a replaceState, with a warning
510
511
src/renderers/__tests__/ReactEmptyComponent-test.js
512
* should not produce child DOM nodes for null and false
src/renderers/__tests__/ReactCompositeComponentState-test.js
+40
@@ -447,4 +447,44 @@ describe('ReactCompositeComponent-state', () => {
447
'Use setState instead.',
448
);
449
});
450
+
451
+ it('should treat assigning to this.state inside cWM as a replaceState, with a warning', () => {
452
+ spyOn(console, 'error');
453
+
454
+ let ops = [];
455
+ class Test extends React.Component {
456
+ state = {step: 1, extra: true};
457
+ componentWillMount() {
458
+ this.setState({step: 2}, () => {
459
+ // Tests that earlier setState callbacks are not dropped
460
+ ops.push(
461
+ `callback -- step: ${this.state.step}, extra: ${!!this.state.extra}`,
462
+ );
463
+ });
464
+ // Treat like replaceState
465
+ this.state = {step: 3};
466
+ }
467
+ render() {
468
+ ops.push(
469
+ `render -- step: ${this.state.step}, extra: ${!!this.state.extra}`,
470
+ );
471
+ return null;
472
+ }
473
+ }
474
+
475
+ // Mount
476
+ const container = document.createElement('div');
477
+ ReactDOM.render(<Test />, container);
478
+
479
+ expect(ops).toEqual([
480
+ 'render -- step: 3, extra: false',
481
+ 'callback -- step: 3, extra: false',
482
+ ]);
483
+ expect(console.error.calls.count()).toEqual(1);
484
+ expect(console.error.calls.argsFor(0)[0]).toEqual(
485
+ 'Warning: Test.componentWillMount(): Assigning directly to ' +
486
+ "this.state is deprecated (except inside a component's constructor). " +
487
+ 'Use setState instead.',
488
+ );
489
+ });
490
});
src/renderers/shared/fiber/ReactFiberClassComponent.js
+88
-70
@@ -304,6 +304,59 @@ module.exports = function(
304
return instance;
305
}
306
307
+ function callComponentWillMount(workInProgress, instance) {
308
+ if (__DEV__) {
309
+ startPhaseTimer(workInProgress, 'componentWillMount');
310
+ }
311
+ const oldState = instance.state;
312
+ instance.componentWillMount();
313
+ if (__DEV__) {
314
+ stopPhaseTimer();
315
+ }
316
+
317
+ if (oldState !== instance.state) {
318
+ if (__DEV__) {
319
+ warning(
320
+ false,
321
+ '%s.componentWillMount(): Assigning directly to this.state is ' +
322
+ "deprecated (except inside a component's " +
323
+ 'constructor). Use setState instead.',
324
+ getComponentName(workInProgress),
325
+ );
326
+ }
327
+ updater.enqueueReplaceState(instance, instance.state, null);
328
+ }
329
+ }
330
+
331
+ function callComponentWillReceiveProps(
332
+ workInProgress,
333
+ instance,
334
+ newProps,
335
+ newContext,
336
+ ) {
337
+ if (__DEV__) {
338
+ startPhaseTimer(workInProgress, 'componentWillReceiveProps');
339
+ }
340
+ const oldState = instance.state;
341
+ instance.componentWillReceiveProps(newProps, newContext);
342
+ if (__DEV__) {
343
+ stopPhaseTimer();
344
+ }
345
+
346
+ if (instance.state !== oldState) {
347
+ if (__DEV__) {
348
+ warning(
349
+ false,
350
+ '%s.componentWillReceiveProps(): Assigning directly to ' +
351
+ "this.state is deprecated (except inside a component's " +
352
+ 'constructor). Use setState instead.',
353
+ getComponentName(workInProgress),
354
+ );
355
+ }
356
+ updater.enqueueReplaceState(instance, instance.state, null);
357
+ }
358
+ }
359
+
360
// Invokes the mount life-cycles on a previously never rendered instance.
361
function mountClassInstance(
362
workInProgress: Fiber,
@@ -339,13 +392,7 @@ module.exports = function(
392
}
393
394
if (typeof instance.componentWillMount === 'function') {
342
- if (__DEV__) {
343
- startPhaseTimer(workInProgress, 'componentWillMount');
344
- }
345
- instance.componentWillMount();
346
- if (__DEV__) {
347
- stopPhaseTimer();
348
- }
395
+ callComponentWillMount(workInProgress, instance);
396
// If we had additional state updates during this life-cycle, let's
397
// process them now.
398
const updateQueue = workInProgress.updateQueue;
@@ -365,36 +412,6 @@ module.exports = function(
412
}
413
}
414
368
- function callComponentWillReceiveProps(
369
- workInProgress,
370
- instance,
371
- newProps,
372
- newContext,
373
- ) {
374
- if (typeof instance.componentWillReceiveProps === 'function') {
375
- if (__DEV__) {
376
- startPhaseTimer(workInProgress, 'componentWillReceiveProps');
377
- }
378
- instance.componentWillReceiveProps(newProps, newContext);
379
- if (__DEV__) {
380
- stopPhaseTimer();
381
- }
382
-
383
- if (instance.state !== workInProgress.memoizedState) {
384
- if (__DEV__) {
385
- warning(
386
- false,
387
- '%s.componentWillReceiveProps(): Assigning directly to ' +
388
- "this.state is deprecated (except inside a component's " +
389
- 'constructor). Use setState instead.',
390
- getComponentName(workInProgress),
391
- );
392
- }
393
- updater.enqueueReplaceState(instance, instance.state, null);
394
- }
395
- }
396
- }
397
-
415
// Called on a preexisting class instance. Returns false if a resumed render
416
// could be reused.
417
function resumeMountClassInstance(
@@ -421,7 +438,11 @@ module.exports = function(
438
439
const oldContext = instance.context;
440
const oldProps = workInProgress.memoizedProps;
424
- if (oldProps !== newProps || oldContext !== newContext) {
441
+
442
+ if (
443
+ typeof instance.componentWillReceiveProps === 'function' &&
444
+ (oldProps !== newProps || oldContext !== newContext)
445
+ ) {
446
callComponentWillReceiveProps(
447
workInProgress,
448
instance,
@@ -430,6 +451,19 @@ module.exports = function(
451
);
452
}
453
454
+ // Process the update queue before calling shouldComponentUpdate
455
+ const updateQueue = workInProgress.updateQueue;
456
+ if (updateQueue !== null) {
457
+ newState = beginUpdateQueue(
458
+ workInProgress,
459
+ updateQueue,
460
+ instance,
461
+ newState,
462
+ newProps,
463
+ priorityLevel,
464
+ );
465
+ }
466
+
467
// TODO: Should we deal with a setState that happened after the last
468
// componentWillMount and before this componentWillMount? Probably
469
// unsupported anyway.
@@ -452,19 +486,6 @@ module.exports = function(
486
return false;
487
}
488
455
- // componentWillMount may have called setState. Process the update queue.
456
- let newUpdateQueue = workInProgress.updateQueue;
457
- if (newUpdateQueue !== null) {
458
- newState = beginUpdateQueue(
459
- workInProgress,
460
- newUpdateQueue,
461
- instance,
462
- newState,
463
- newProps,
464
- priorityLevel,
465
- );
466
- }
467
-
489
// Update the input pointers now so that they are correct when we call
490
// componentWillMount
491
instance.props = newProps;
@@ -472,27 +493,21 @@ module.exports = function(
493
instance.context = newContext;
494
495
if (typeof instance.componentWillMount === 'function') {
475
- if (__DEV__) {
476
- startPhaseTimer(workInProgress, 'componentWillMount');
477
- }
478
- instance.componentWillMount();
479
- if (__DEV__) {
480
- stopPhaseTimer();
496
+ callComponentWillMount(workInProgress, instance);
497
+ // componentWillMount may have called setState. Process the update queue.
498
+ const newUpdateQueue = workInProgress.updateQueue;
499
+ if (newUpdateQueue !== null) {
500
+ newState = beginUpdateQueue(
501
+ workInProgress,
502
+ updateQueue,
503
+ instance,
504
+ newState,
505
+ newProps,
506
+ priorityLevel,
507
+ );
508
}
509
}
510
484
- // componentWillMount may have called setState. Process the update queue.
485
- newUpdateQueue = workInProgress.updateQueue;
486
- if (newUpdateQueue !== null) {
487
- newState = beginUpdateQueue(
488
- workInProgress,
489
- newUpdateQueue,
490
- instance,
491
- newState,
492
- newProps,
493
- priorityLevel,
494
- );
495
- }
511
if (typeof instance.componentDidMount === 'function') {
512
workInProgress.effectTag |= Update;
513
}
@@ -531,7 +546,10 @@ module.exports = function(
546
// ever the previously attempted to render - not the "current". However,
547
// during componentDidUpdate we pass the "current" props.
548
534
- if (oldProps !== newProps || oldContext !== newContext) {
549
+ if (
550
+ typeof instance.componentWillReceiveProps === 'function' &&
551
+ (oldProps !== newProps || oldContext !== newContext)
552
+ ) {
553
callComponentWillReceiveProps(
554
workInProgress,
555
instance,