Skip to content

Commit

Permalink
Warn against state updates from useEffect destroy functions (#18307)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Brian Vaughn committed Mar 13, 2020
1 parent 0705b72 commit 730389b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 20 deletions.
47 changes: 31 additions & 16 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,14 @@ const {

type ExecutionContext = number;

const NoContext = /* */ 0b000000;
const BatchedContext = /* */ 0b000001;
const EventContext = /* */ 0b000010;
const DiscreteEventContext = /* */ 0b000100;
const LegacyUnbatchedContext = /* */ 0b001000;
const RenderContext = /* */ 0b010000;
const CommitContext = /* */ 0b100000;
const NoContext = /* */ 0b0000000;
const BatchedContext = /* */ 0b0000001;
const EventContext = /* */ 0b0000010;
const DiscreteEventContext = /* */ 0b0000100;
const LegacyUnbatchedContext = /* */ 0b0001000;
const RenderContext = /* */ 0b0010000;
const CommitContext = /* */ 0b0100000;
const PassiveEffectContext = /* */ 0b1000000;

type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
const RootIncomplete = 0;
Expand Down Expand Up @@ -2283,6 +2284,7 @@ function flushPassiveEffectsImpl() {
);
const prevExecutionContext = executionContext;
executionContext |= CommitContext;
executionContext |= PassiveEffectContext;
const prevInteractions = pushInteractions(root);

if (runAllPassiveEffectDestroysBeforeCreates) {
Expand Down Expand Up @@ -2812,15 +2814,28 @@ function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
} else {
didWarnStateUpdateForUnmountedComponent = new Set([componentName]);
}
console.error(
"Can't perform a React state update on an unmounted component. This " +
'is a no-op, but it indicates a memory leak in your application. To ' +
'fix, cancel all subscriptions and asynchronous tasks in %s.%s',
tag === ClassComponent
? 'the componentWillUnmount method'
: 'a useEffect cleanup function',
getStackByFiberInDevAndProd(fiber),
);

// If we are currently flushing passive effects, change the warning text.
if ((executionContext & PassiveEffectContext) !== NoContext) {
console.error(
"Can't perform a React state update from within a useEffect cleanup function. " +
'To fix, move state updates to the useEffect() body in %s.%s',
tag === ClassComponent
? 'the componentWillUnmount method'
: 'a useEffect cleanup function',
getStackByFiberInDevAndProd(fiber),
);
} else {
console.error(
"Can't perform a React state update on an unmounted component. This " +
'is a no-op, but it indicates a memory leak in your application. To ' +
'fix, cancel all subscriptions and asynchronous tasks in %s.%s',
tag === ClassComponent
? 'the componentWillUnmount method'
: 'a useEffect cleanup function',
getStackByFiberInDevAndProd(fiber),
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1025,8 +1025,10 @@ describe('ReactHooksWithNoopRenderer', () => {
);

if (
deferPassiveEffectCleanupDuringUnmount &&
runAllPassiveEffectDestroysBeforeCreates
require('shared/ReactFeatureFlags')
.deferPassiveEffectCleanupDuringUnmount &&
require('shared/ReactFeatureFlags')
.runAllPassiveEffectDestroysBeforeCreates
) {
it('defers passive effect destroy functions during unmount', () => {
function Child({bar, foo}) {
Expand Down Expand Up @@ -1256,7 +1258,7 @@ describe('ReactHooksWithNoopRenderer', () => {
});
});

it('still warns about state updates from within passive unmount function', () => {
it('shows a unique warning for state updates from within passive unmount function', () => {
function Component() {
Scheduler.unstable_yieldValue('Component');
const [didLoad, setDidLoad] = React.useState(false);
Expand Down Expand Up @@ -1285,7 +1287,7 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(() => {
expect(Scheduler).toFlushAndYield(['passive destroy']);
}).toErrorDev(
"Warning: Can't perform a React state update on an unmounted component.",
"Warning: Can't perform a React state update from within a useEffect cleanup function.",
);
});
});
Expand Down

0 comments on commit 730389b

Please sign in to comment.