Warn against state updates from useEffect destroy functions (#18307)
Don't warn about unmounted state updates from within passive destroy function * Fixed test conditional. (It broke after recent variant refactor.) * Changed warning wording for setState from within useEffect destroy callback
Brian Vaughn committed
Mar 13, 2020 at 15:33 UTC
730389b9d3865cb6d5c85e94b9b66f96e391718e
2 files changed
+37
-20
packages/react-reconciler/src/ReactFiberWorkLoop.js
+31
-16
@@ -199,13 +199,14 @@ const {
199
200
type ExecutionContext = number;
201
202
-const NoContext = /* */ 0b000000;
203
-const BatchedContext = /* */ 0b000001;
204
-const EventContext = /* */ 0b000010;
205
-const DiscreteEventContext = /* */ 0b000100;
206
-const LegacyUnbatchedContext = /* */ 0b001000;
207
-const RenderContext = /* */ 0b010000;
208
-const CommitContext = /* */ 0b100000;
202
+const NoContext = /* */ 0b0000000;
203
+const BatchedContext = /* */ 0b0000001;
204
+const EventContext = /* */ 0b0000010;
205
+const DiscreteEventContext = /* */ 0b0000100;
206
+const LegacyUnbatchedContext = /* */ 0b0001000;
207
+const RenderContext = /* */ 0b0010000;
208
+const CommitContext = /* */ 0b0100000;
209
+const PassiveEffectContext = /* */ 0b1000000;
210
211
type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
212
const RootIncomplete = 0;
@@ -2283,6 +2284,7 @@ function flushPassiveEffectsImpl() {
2284
);
2285
const prevExecutionContext = executionContext;
2286
executionContext |= CommitContext;
2287
+ executionContext |= PassiveEffectContext;
2288
const prevInteractions = pushInteractions(root);
2289
2290
if (runAllPassiveEffectDestroysBeforeCreates) {
@@ -2812,15 +2814,28 @@ function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
2814
} else {
2815
didWarnStateUpdateForUnmountedComponent = new Set([componentName]);
2816
}
2815
- console.error(
2816
- "Can't perform a React state update on an unmounted component. This " +
2817
- 'is a no-op, but it indicates a memory leak in your application. To ' +
2818
- 'fix, cancel all subscriptions and asynchronous tasks in %s.%s',
2819
- tag === ClassComponent
2820
- ? 'the componentWillUnmount method'
2821
- : 'a useEffect cleanup function',
2822
- getStackByFiberInDevAndProd(fiber),
2823
- );
2817
+
2818
+ // If we are currently flushing passive effects, change the warning text.
2819
+ if ((executionContext & PassiveEffectContext) !== NoContext) {
2820
+ console.error(
2821
+ "Can't perform a React state update from within a useEffect cleanup function. " +
2822
+ 'To fix, move state updates to the useEffect() body in %s.%s',
2823
+ tag === ClassComponent
2824
+ ? 'the componentWillUnmount method'
2825
+ : 'a useEffect cleanup function',
2826
+ getStackByFiberInDevAndProd(fiber),
2827
+ );
2828
+ } else {
2829
+ console.error(
2830
+ "Can't perform a React state update on an unmounted component. This " +
2831
+ 'is a no-op, but it indicates a memory leak in your application. To ' +
2832
+ 'fix, cancel all subscriptions and asynchronous tasks in %s.%s',
2833
+ tag === ClassComponent
2834
+ ? 'the componentWillUnmount method'
2835
+ : 'a useEffect cleanup function',
2836
+ getStackByFiberInDevAndProd(fiber),
2837
+ );
2838
+ }
2839
}
2840
}
2841
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+6
-4
@@ -1025,8 +1025,10 @@ describe('ReactHooksWithNoopRenderer', () => {
1025
);
1026
1027
if (
1028
- deferPassiveEffectCleanupDuringUnmount &&
1029
- runAllPassiveEffectDestroysBeforeCreates
1028
+ require('shared/ReactFeatureFlags')
1029
+ .deferPassiveEffectCleanupDuringUnmount &&
1030
+ require('shared/ReactFeatureFlags')
1031
+ .runAllPassiveEffectDestroysBeforeCreates
1032
) {
1033
it('defers passive effect destroy functions during unmount', () => {
1034
function Child({bar, foo}) {
@@ -1256,7 +1258,7 @@ describe('ReactHooksWithNoopRenderer', () => {
1258
});
1259
});
1260
1259
- it('still warns about state updates from within passive unmount function', () => {
1261
+ it('shows a unique warning for state updates from within passive unmount function', () => {
1262
function Component() {
1263
Scheduler.unstable_yieldValue('Component');
1264
const [didLoad, setDidLoad] = React.useState(false);
@@ -1285,7 +1287,7 @@ describe('ReactHooksWithNoopRenderer', () => {
1287
expect(() => {
1288
expect(Scheduler).toFlushAndYield(['passive destroy']);
1289
}).toErrorDev(
1288
- "Warning: Can't perform a React state update on an unmounted component.",
1290
+ "Warning: Can't perform a React state update from within a useEffect cleanup function.",
1291
);
1292
});
1293
});