Update debugRenderPhaseSideEffects behavior (#12057)
Update debugRenderPhaseSideEffects behavior This feature flag no longer double-invokes componentWillMount, componentWillReceiveProps, componentWillUpdate, or shouldComponentUpdate. It continues to double-invoke the constructor, render, and setState updater functions as well as the recently added, static getDerivedStateFromProps method Tests have been updated.
Brian Vaughn committed
Jan 24, 2018 at 15:06 UTC
431dca925a9f76b683d2ea5ec4520149ad15ff81
3 files changed
+35
-31
packages/react-reconciler/src/ReactFiberBeginWork.js
-9
@@ -220,15 +220,6 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
220
constructClassInstance(workInProgress, workInProgress.pendingProps);
221
mountClassInstance(workInProgress, renderExpirationTime);
222
223
- // Simulate an async bailout/interruption by invoking lifecycle twice.
224
- // We do this here rather than inside of ReactFiberClassComponent,
225
- // To more realistically simulate the interruption behavior of async,
226
- // Which would never call componentWillMount() twice on the same instance.
227
- if (debugRenderPhaseSideEffects) {
228
- constructClassInstance(workInProgress, workInProgress.pendingProps);
229
- mountClassInstance(workInProgress, renderExpirationTime);
230
- }
231
-
223
shouldUpdate = true;
224
} else {
225
invariant(false, 'Resuming work not yet implemented.');
packages/react-reconciler/src/ReactFiberClassComponent.js
+15
-15
@@ -199,11 +199,6 @@ export default function(
199
);
200
stopPhaseTimer();
201
202
- // Simulate an async bailout/interruption by invoking lifecycle twice.
203
- if (debugRenderPhaseSideEffects) {
204
- instance.shouldComponentUpdate(newProps, newState, newContext);
205
- }
206
-
202
if (__DEV__) {
203
warning(
204
shouldUpdate !== undefined,
@@ -402,6 +397,12 @@ export default function(
397
const context = needsContext
398
? getMaskedContext(workInProgress, unmaskedContext)
399
: emptyObject;
400
+
401
+ // Instantiate twice to help detect side-effects.
402
+ if (debugRenderPhaseSideEffects) {
403
+ new ctor(props, context); // eslint-disable-line no-new
404
+ }
405
+
406
const instance = new ctor(props, context);
407
const state =
408
instance.state !== null && instance.state !== undefined
@@ -537,11 +538,6 @@ export default function(
538
startPhaseTimer(workInProgress, 'componentWillReceiveProps');
539
instance.UNSAFE_componentWillReceiveProps(newProps, newContext);
540
stopPhaseTimer();
540
-
541
- // Simulate an async bailout/interruption by invoking lifecycle twice.
542
- if (debugRenderPhaseSideEffects) {
543
- instance.UNSAFE_componentWillReceiveProps(newProps, newContext);
544
- }
541
}
542
543
if (instance.state !== oldState) {
@@ -589,6 +585,15 @@ export default function(
585
}
586
}
587
588
+ if (debugRenderPhaseSideEffects) {
589
+ // Invoke method an extra time to help detect side-effects.
590
+ type.getDerivedStateFromProps.call(
591
+ null,
592
+ props,
593
+ workInProgress.memoizedState,
594
+ );
595
+ }
596
+
597
const partialState = type.getDerivedStateFromProps.call(
598
null,
599
props,
@@ -916,11 +921,6 @@ export default function(
921
startPhaseTimer(workInProgress, 'componentWillUpdate');
922
instance.UNSAFE_componentWillUpdate(newProps, newState, newContext);
923
stopPhaseTimer();
919
-
920
- // Simulate an async bailout/interruption by invoking lifecycle twice.
921
- if (debugRenderPhaseSideEffects) {
922
- instance.UNSAFE_componentWillUpdate(newProps, newState, newContext);
923
- }
924
}
925
}
926
if (typeof instance.componentDidUpdate === 'function') {
packages/react/src/__tests__/ReactAsyncClassComponent-test.internal.js
+20
-7
@@ -28,6 +28,10 @@ describe('ReactAsyncClassComponent', () => {
28
let shouldComponentUpdate = false;
29
class ClassComponent extends React.Component {
30
state = {};
31
+ static getDerivedStateFromProps() {
32
+ log.push('getDerivedStateFromProps');
33
+ return null;
34
+ }
35
constructor(props) {
36
super(props);
37
log.push('constructor');
@@ -60,11 +64,21 @@ describe('ReactAsyncClassComponent', () => {
64
}
65
}
66
63
- const component = ReactTestRenderer.create(<ClassComponent />);
67
+ let component;
68
+
69
+ expect(() => {
70
+ component = ReactTestRenderer.create(<ClassComponent />);
71
+ }).toWarnDev(
72
+ 'ClassComponent: Defines both componentWillReceiveProps() ' +
73
+ 'and static getDerivedStateFromProps() methods. ' +
74
+ 'We recommend using only getDerivedStateFromProps().',
75
+ );
76
+
77
expect(log).toEqual([
78
'constructor',
66
- 'componentWillMount',
79
'constructor',
80
+ 'getDerivedStateFromProps',
81
+ 'getDerivedStateFromProps',
82
'componentWillMount',
83
'render',
84
'render',
@@ -77,11 +91,10 @@ describe('ReactAsyncClassComponent', () => {
91
component.update(<ClassComponent />);
92
expect(log).toEqual([
93
'componentWillReceiveProps',
80
- 'componentWillReceiveProps',
81
- 'shouldComponentUpdate',
94
+ 'getDerivedStateFromProps',
95
+ 'getDerivedStateFromProps',
96
'shouldComponentUpdate',
97
'componentWillUpdate',
84
- 'componentWillUpdate',
98
'render',
99
'render',
100
'componentDidUpdate',
@@ -93,8 +106,8 @@ describe('ReactAsyncClassComponent', () => {
106
component.update(<ClassComponent />);
107
expect(log).toEqual([
108
'componentWillReceiveProps',
96
- 'componentWillReceiveProps',
97
- 'shouldComponentUpdate',
109
+ 'getDerivedStateFromProps',
110
+ 'getDerivedStateFromProps',
111
'shouldComponentUpdate',
112
]);
113
});