Skip to content

Commit cba51ba

Browse files
bvaughnbvaughn
authored
Warn if unsafe lifecycle methods are found in an async subtree (#12060)
1 parent 4d65408 commit cba51ba

10 files changed

+529
-12
lines changed

‎packages/react-call-return/src/__tests__/ReactCallReturn-test.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,10 @@ describe('ReactCallReturn', () => {
290290
<Return value={2} />
291291
</Call>,
292292
);
293-
ReactNoop.flush();
293+
expect(ReactNoop.flush).toWarnDev(
294+
'componentWillMount: Please update the following components ' +
295+
'to use componentDidMount instead: Return',
296+
);
294297

295298
expect(ops).toEqual([
296299
'Mount Return 1',

‎packages/react-reconciler/src/ReactDebugAsyncWarnings.js

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/**
2+
* Copyright (c) 2013-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
import type {Fiber} from './ReactFiber';
11+
12+
import getComponentName from 'shared/getComponentName';
13+
import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';
14+
import {AsyncUpdates} from './ReactTypeOfInternalContext';
15+
import warning from 'fbjs/lib/warning';
16+
17+
type LIFECYCLE =
18+
| 'UNSAFE_componentWillMount'
19+
| 'UNSAFE_componentWillReceiveProps'
20+
| 'UNSAFE_componentWillUpdate';
21+
type LifecycleToComponentsMap = {[lifecycle: LIFECYCLE]: Array<Fiber>};
22+
type FiberToLifecycleMap = Map<Fiber, LifecycleToComponentsMap>;
23+
24+
const ReactDebugAsyncWarnings = {
25+
discardPendingWarnings(): void {},
26+
flushPendingAsyncWarnings(): void {},
27+
recordLifecycleWarnings(fiber: Fiber, instance: any): void {},
28+
};
29+
30+
if (__DEV__) {
31+
const LIFECYCLE_SUGGESTIONS = {
32+
UNSAFE_componentWillMount: 'componentDidMount',
33+
UNSAFE_componentWillReceiveProps: 'static getDerivedStateFromProps',
34+
UNSAFE_componentWillUpdate: 'componentDidUpdate',
35+
};
36+
37+
let pendingWarningsMap: FiberToLifecycleMap = new Map();
38+
39+
// Tracks components we have already warned about.
40+
const didWarnSet = new Set();
41+
42+
ReactDebugAsyncWarnings.discardPendingWarnings = () => {
43+
pendingWarningsMap = new Map();
44+
};
45+
46+
ReactDebugAsyncWarnings.flushPendingAsyncWarnings = () => {
47+
((pendingWarningsMap: any): FiberToLifecycleMap).forEach(
48+
(lifecycleWarningsMap, asyncRoot) => {
49+
const lifecyclesWarningMesages = [];
50+
51+
Object.keys(lifecycleWarningsMap).forEach(lifecycle => {
52+
const lifecycleWarnings = lifecycleWarningsMap[lifecycle];
53+
if (lifecycleWarnings.length > 0) {
54+
const componentNames = new Set();
55+
lifecycleWarnings.forEach(fiber => {
56+
componentNames.add(getComponentName(fiber) || 'Component');
57+
didWarnSet.add(fiber.type);
58+
});
59+
60+
const formatted = lifecycle.replace('UNSAFE_', '');
61+
const suggestion = LIFECYCLE_SUGGESTIONS[lifecycle];
62+
const sortedComponentNames = Array.from(componentNames)
63+
.sort()
64+
.join(', ');
65+
66+
lifecyclesWarningMesages.push(
67+
`${formatted}: Please update the following components to use ` +
68+
`${suggestion} instead: ${sortedComponentNames}`,
69+
);
70+
}
71+
});
72+
73+
if (lifecyclesWarningMesages.length > 0) {
74+
const asyncRootComponentStack = getStackAddendumByWorkInProgressFiber(
75+
asyncRoot,
76+
);
77+
78+
warning(
79+
false,
80+
'Unsafe lifecycle methods were found within the following async tree:%s' +
81+
'\n\n%s' +
82+
'\n\nLearn more about this warning here:' +
83+
'\nhttps://fb.me/react-async-component-lifecycle-hooks',
84+
asyncRootComponentStack,
85+
lifecyclesWarningMesages.join('\n\n'),
86+
);
87+
}
88+
},
89+
);
90+
91+
pendingWarningsMap = new Map();
92+
};
93+
94+
const getAsyncRoot = (fiber: Fiber): Fiber => {
95+
let maybeAsyncRoot = null;
96+
97+
while (fiber !== null) {
98+
if (fiber.internalContextTag & AsyncUpdates) {
99+
maybeAsyncRoot = fiber;
100+
}
101+
102+
fiber = fiber.return;
103+
}
104+
105+
return maybeAsyncRoot;
106+
};
107+
108+
ReactDebugAsyncWarnings.recordLifecycleWarnings = (
109+
fiber: Fiber,
110+
instance: any,
111+
) => {
112+
const asyncRoot = getAsyncRoot(fiber);
113+
114+
// Dedup strategy: Warn once per component.
115+
// This is difficult to track any other way since component names
116+
// are often vague and are likely to collide between 3rd party libraries.
117+
// An expand property is probably okay to use here since it's DEV-only,
118+
// and will only be set in the event of serious warnings.
119+
if (didWarnSet.has(fiber.type)) {
120+
return;
121+
}
122+
123+
let warningsForRoot;
124+
if (!pendingWarningsMap.has(asyncRoot)) {
125+
warningsForRoot = {
126+
UNSAFE_componentWillMount: [],
127+
UNSAFE_componentWillReceiveProps: [],
128+
UNSAFE_componentWillUpdate: [],
129+
};
130+
131+
pendingWarningsMap.set(asyncRoot, warningsForRoot);
132+
} else {
133+
warningsForRoot = pendingWarningsMap.get(asyncRoot);
134+
}
135+
136+
const unsafeLifecycles = [];
137+
if (
138+
typeof instance.componentWillMount === 'function' ||
139+
typeof instance.UNSAFE_componentWillMount === 'function'
140+
) {
141+
unsafeLifecycles.push('UNSAFE_componentWillMount');
142+
}
143+
if (
144+
typeof instance.componentWillReceiveProps === 'function' ||
145+
typeof instance.UNSAFE_componentWillReceiveProps === 'function'
146+
) {
147+
unsafeLifecycles.push('UNSAFE_componentWillReceiveProps');
148+
}
149+
if (
150+
typeof instance.componentWillUpdate === 'function' ||
151+
typeof instance.UNSAFE_componentWillUpdate === 'function'
152+
) {
153+
unsafeLifecycles.push('UNSAFE_componentWillUpdate');
154+
}
155+
156+
if (unsafeLifecycles.length > 0) {
157+
unsafeLifecycles.forEach(lifecycle => {
158+
((warningsForRoot: any): LifecycleToComponentsMap)[lifecycle].push(
159+
fiber,
160+
);
161+
});
162+
}
163+
};
164+
}
165+
166+
export default ReactDebugAsyncWarnings;

‎packages/react-reconciler/src/ReactFiberClassComponent.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
enableAsyncSubtreeAPI,
1717
warnAboutDeprecatedLifecycles,
1818
} from 'shared/ReactFeatureFlags';
19+
import ReactDebugAsyncWarnings from './ReactDebugAsyncWarnings';
1920
import {isMounted} from 'react-reconciler/reflection';
2021
import * as ReactInstanceMap from 'shared/ReactInstanceMap';
2122
import emptyObject from 'fbjs/lib/emptyObject';
@@ -642,6 +643,17 @@ export default function(
642643
workInProgress.internalContextTag |= AsyncUpdates;
643644
}
644645

646+
if (__DEV__) {
647+
// If we're inside of an async sub-tree,
648+
// Warn about any unsafe lifecycles on this class component.
649+
if (workInProgress.internalContextTag & AsyncUpdates) {
650+
ReactDebugAsyncWarnings.recordLifecycleWarnings(
651+
workInProgress,
652+
instance,
653+
);
654+
}
655+
}
656+
645657
if (
646658
typeof instance.UNSAFE_componentWillMount === 'function' ||
647659
typeof instance.componentWillMount === 'function'

‎packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime';
1616
import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';
1717
import ReactErrorUtils from 'shared/ReactErrorUtils';
1818
import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState';
19+
import ReactDebugAsyncWarnings from './ReactDebugAsyncWarnings';
1920
import {
2021
PerformedWork,
2122
Placement,
@@ -310,6 +311,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
310311
}
311312

312313
function commitAllLifeCycles() {
314+
if (__DEV__) {
315+
ReactDebugAsyncWarnings.flushPendingAsyncWarnings();
316+
}
317+
313318
while (nextEffect !== null) {
314319
const effectTag = nextEffect.effectTag;
315320

@@ -651,6 +656,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
651656
}
652657

653658
function performFailedUnitOfWork(workInProgress: Fiber): Fiber | null {
659+
if (__DEV__) {
660+
ReactDebugAsyncWarnings.discardPendingWarnings();
661+
}
662+
654663
// The current, flushed, state of this fiber is the alternate.
655664
// Ideally nothing should rely on this, but relying on it here
656665
// means that we don't need an additional field on the work in

‎packages/react-reconciler/src/__tests__/ReactIncremental-test.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,8 +1430,8 @@ describe('ReactIncremental', () => {
14301430
changeState() {
14311431
this.setState({foo: 'bar'});
14321432
}
1433-
componentWillUpdate() {
1434-
ops.push('componentWillUpdate');
1433+
componentDidUpdate() {
1434+
ops.push('componentDidUpdate');
14351435
}
14361436
render() {
14371437
ops.push('render');
@@ -1451,7 +1451,7 @@ describe('ReactIncremental', () => {
14511451
instance.changeState();
14521452
ReactNoop.flush();
14531453

1454-
expect(ops).toEqual(['componentWillUpdate', 'render']);
1454+
expect(ops).toEqual(['render', 'componentDidUpdate']);
14551455
expect(instance.state).toEqual({foo: 'bar'});
14561456
});
14571457

@@ -2335,7 +2335,10 @@ describe('ReactIncremental', () => {
23352335
}
23362336

23372337
ReactNoop.render(<MyComponent />);
2338-
ReactNoop.flush();
2338+
expect(ReactNoop.flush).toWarnDev(
2339+
'componentWillReceiveProps: Please update the following components ' +
2340+
'to use static getDerivedStateFromProps instead: MyComponent',
2341+
);
23392342

23402343
expect(ops).toEqual([
23412344
'render',

‎packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,12 @@ describe('ReactDebugFiberPerf', () => {
254254
</Parent>,
255255
);
256256
addComment('Should not print a warning');
257-
ReactNoop.flush();
257+
expect(ReactNoop.flush).toWarnDev([
258+
'componentWillMount: Please update the following components ' +
259+
'to use componentDidMount instead: NotCascading' +
260+
'\n\ncomponentWillReceiveProps: Please update the following components ' +
261+
'to use static getDerivedStateFromProps instead: NotCascading',
262+
]);
258263
ReactNoop.render(
259264
<Parent>
260265
<NotCascading />
@@ -288,7 +293,14 @@ describe('ReactDebugFiberPerf', () => {
288293
}
289294
ReactNoop.render(<AllLifecycles />);
290295
addComment('Mount');
291-
ReactNoop.flush();
296+
expect(ReactNoop.flush).toWarnDev(
297+
'componentWillMount: Please update the following components ' +
298+
'to use componentDidMount instead: AllLifecycles' +
299+
'\n\ncomponentWillReceiveProps: Please update the following components ' +
300+
'to use static getDerivedStateFromProps instead: AllLifecycles' +
301+
'\n\ncomponentWillUpdate: Please update the following components ' +
302+
'to use componentDidUpdate instead: AllLifecycles',
303+
);
292304
ReactNoop.render(<AllLifecycles />);
293305
addComment('Update');
294306
ReactNoop.flush();

‎packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ describe('ReactIncrementalReflection', () => {
5959
ops = [];
6060

6161
// Render the rest and commit the updates.
62-
ReactNoop.flush();
62+
expect(ReactNoop.flush).toWarnDev(
63+
'componentWillMount: Please update the following components ' +
64+
'to use componentDidMount instead: Component',
65+
);
6366

6467
expect(ops).toEqual(['componentDidMount', true]);
6568

@@ -97,7 +100,10 @@ describe('ReactIncrementalReflection', () => {
97100
}
98101

99102
ReactNoop.render(<Foo mount={true} />);
100-
ReactNoop.flush();
103+
expect(ReactNoop.flush).toWarnDev(
104+
'componentWillMount: Please update the following components ' +
105+
'to use componentDidMount instead: Component',
106+
);
101107

102108
expect(ops).toEqual(['Component']);
103109
ops = [];
@@ -184,7 +190,12 @@ describe('ReactIncrementalReflection', () => {
184190
// not find any host nodes in it.
185191
expect(ReactNoop.findInstance(classInstance)).toBe(null);
186192

187-
ReactNoop.flush();
193+
expect(ReactNoop.flush).toWarnDev(
194+
'componentWillMount: Please update the following components ' +
195+
'to use componentDidMount instead: Component' +
196+
'\n\ncomponentWillUpdate: Please update the following components ' +
197+
'to use componentDidUpdate instead: Component',
198+
);
188199

189200
const hostSpan = classInstance.span;
190201
expect(hostSpan).toBeDefined();

‎packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,10 @@ describe('ReactIncrementalScheduling', () => {
346346
}
347347

348348
ReactNoop.render(<Foo step={1} />);
349-
ReactNoop.flush();
349+
expect(ReactNoop.flush).toWarnDev(
350+
'componentWillReceiveProps: Please update the following components ' +
351+
'to use static getDerivedStateFromProps instead: Foo',
352+
);
350353

351354
ReactNoop.render(<Foo step={2} />);
352355
expect(ReactNoop.flush()).toEqual([

‎packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,12 +307,16 @@ describe('ReactIncrementalUpdates', () => {
307307
}
308308
}
309309
ReactNoop.render(<Foo />);
310-
ReactNoop.flush();
310+
expect(ReactNoop.flush).toWarnDev(
311+
'componentWillReceiveProps: Please update the following components ' +
312+
'to use static getDerivedStateFromProps instead: Foo',
313+
);
311314

312315
ops = [];
313316

314317
ReactNoop.flushSync(() => {
315318
instance.setState({a: 'a'});
319+
316320
ReactNoop.render(<Foo />); // Trigger componentWillReceiveProps
317321
});
318322

0 commit comments

Comments
 (0)