Skip to content

Commit

Permalink
Remove the warning for setState on unmounted components (facebook#22114)
Browse files Browse the repository at this point in the history 
* Remove warning for setState on unmounted components

* Trigger CI
  • Loading branch information
@gaearon
gaearon committed Aug 18, 2021
1 parent 6abda7f commit 7ed0706
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 291 deletions.
28 changes: 4 additions & 24 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ describe('ReactCompositeComponent', () => {
ReactDOM.render(<MyComponent />, container2);
});

it('should warn about `forceUpdate` on unmounted components', () => {
it('should not warn about `forceUpdate` on unmounted components', () => {
const container = document.createElement('div');
document.body.appendChild(container);

Expand All @@ -325,19 +325,11 @@ describe('ReactCompositeComponent', () => {

ReactDOM.unmountComponentAtNode(container);

expect(() => instance.forceUpdate()).toErrorDev(
"Warning: 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 the componentWillUnmount method.\n' +
' in Component (at **)',
);

// No additional warning should be recorded
instance.forceUpdate();
instance.forceUpdate();
});

it('should warn about `setState` on unmounted components', () => {
it('should not warn about `setState` on unmounted components', () => {
const container = document.createElement('div');
document.body.appendChild(container);

Expand Down Expand Up @@ -365,22 +357,10 @@ describe('ReactCompositeComponent', () => {
expect(renders).toBe(1);

instance.setState({value: 1});

expect(renders).toBe(2);

ReactDOM.render(<div />, container);

expect(() => {
instance.setState({value: 2});
}).toErrorDev(
"Warning: 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 the componentWillUnmount method.\n' +
' in Component (at **)\n' +
' in span',
);

instance.setState({value: 2});
expect(renders).toBe(2);
});

Expand Down
103 changes: 0 additions & 103 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type {Fiber, FiberRoot} from './ReactInternalTypes';
import type {Lanes, Lane} from './ReactFiberLane.new';
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
import type {StackCursor} from './ReactFiberStack.new';
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';
import type {Flags} from './ReactFiberFlags';

import {
Expand Down Expand Up @@ -53,10 +52,6 @@ import {
scheduleSyncCallback,
scheduleLegacySyncCallback,
} from './ReactFiberSyncTaskQueue.new';
import {
NoFlags as NoHookEffect,
Passive as HookPassive,
} from './ReactHookEffectTags';
import {
logCommitStarted,
logCommitStopped,
Expand Down Expand Up @@ -119,7 +114,6 @@ import {LegacyRoot} from './ReactRootTags';
import {
NoFlags,
Placement,
PassiveStatic,
Incomplete,
HostEffectMask,
Hydrating,
Expand Down Expand Up @@ -344,10 +338,6 @@ let nestedPassiveUpdateCount: number = 0;
let currentEventTime: number = NoTimestamp;
let currentEventTransitionLane: Lanes = NoLanes;

// Dev only flag that tracks if passive effects are currently being flushed.
// We warn about state updates for unmounted components differently in this case.
let isFlushingPassiveEffects = false;

export function getWorkInProgressRoot(): FiberRoot | null {
return workInProgressRoot;
}
Expand Down Expand Up @@ -454,7 +444,6 @@ export function scheduleUpdateOnFiber(

const root = markUpdateLaneFromFiberToRoot(fiber, lane);
if (root === null) {
warnAboutUpdateOnUnmountedFiberInDEV(fiber);
return null;
}

Expand Down Expand Up @@ -2048,10 +2037,6 @@ function flushPassiveEffectsImpl() {
markPassiveEffectsStarted(lanes);
}

if (__DEV__) {
isFlushingPassiveEffects = true;
}

const prevExecutionContext = executionContext;
executionContext |= CommitContext;

Expand All @@ -2068,10 +2053,6 @@ function flushPassiveEffectsImpl() {
}
}

if (__DEV__) {
isFlushingPassiveEffects = false;
}

if (__DEV__) {
if (enableDebugTracing) {
logPassiveEffectsStopped();
Expand Down Expand Up @@ -2503,90 +2484,6 @@ function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) {
}
}

let didWarnStateUpdateForUnmountedComponent: Set<string> | null = null;
function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
if (__DEV__) {
const tag = fiber.tag;
if (
tag !== HostRoot &&
tag !== ClassComponent &&
tag !== FunctionComponent &&
tag !== ForwardRef &&
tag !== MemoComponent &&
tag !== SimpleMemoComponent
) {
// Only warn for user-defined components, not internal ones like Suspense.
return;
}

if ((fiber.flags & PassiveStatic) !== NoFlags) {
const updateQueue: FunctionComponentUpdateQueue | null = (fiber.updateQueue: any);
if (updateQueue !== null) {
const lastEffect = updateQueue.lastEffect;
if (lastEffect !== null) {
const firstEffect = lastEffect.next;

let effect = firstEffect;
do {
if (effect.destroy !== undefined) {
if ((effect.tag & HookPassive) !== NoHookEffect) {
return;
}
}
effect = effect.next;
} while (effect !== firstEffect);
}
}
}
// We show the whole stack but dedupe on the top component's name because
// the problematic code almost always lies inside that component.
const componentName = getComponentNameFromFiber(fiber) || 'ReactComponent';
if (didWarnStateUpdateForUnmountedComponent !== null) {
if (didWarnStateUpdateForUnmountedComponent.has(componentName)) {
return;
}
didWarnStateUpdateForUnmountedComponent.add(componentName);
} else {
didWarnStateUpdateForUnmountedComponent = new Set([componentName]);
}

if (isFlushingPassiveEffects) {
// Do not warn if we are currently flushing passive effects!
//
// React can't directly detect a memory leak, but there are some clues that warn about one.
// One of these clues is when an unmounted React component tries to update its state.
// For example, if a component forgets to remove an event listener when unmounting,
// that listener may be called later and try to update state,
// at which point React would warn about the potential leak.
//
// Warning signals are the most useful when they're strong.
// (So we should avoid false positive warnings.)
// Updating state from within an effect cleanup function is sometimes a necessary pattern, e.g.:
// 1. Updating an ancestor that a component had registered itself with on mount.
// 2. Resetting state when a component is hidden after going offscreen.
} else {
const previousFiber = ReactCurrentFiberCurrent;
try {
setCurrentDebugFiberInDEV(fiber);
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.',
tag === ClassComponent
? 'the componentWillUnmount method'
: 'a useEffect cleanup function',
);
} finally {
if (previousFiber) {
setCurrentDebugFiberInDEV(fiber);
} else {
resetCurrentDebugFiberInDEV();
}
}
}
}
}

let beginWork;
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
const dummyFiber = null;
Expand Down
103 changes: 0 additions & 103 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type {Fiber, FiberRoot} from './ReactInternalTypes';
import type {Lanes, Lane} from './ReactFiberLane.old';
import type {SuspenseState} from './ReactFiberSuspenseComponent.old';
import type {StackCursor} from './ReactFiberStack.old';
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old';
import type {Flags} from './ReactFiberFlags';

import {
Expand Down Expand Up @@ -53,10 +52,6 @@ import {
scheduleSyncCallback,
scheduleLegacySyncCallback,
} from './ReactFiberSyncTaskQueue.old';
import {
NoFlags as NoHookEffect,
Passive as HookPassive,
} from './ReactHookEffectTags';
import {
logCommitStarted,
logCommitStopped,
Expand Down Expand Up @@ -119,7 +114,6 @@ import {LegacyRoot} from './ReactRootTags';
import {
NoFlags,
Placement,
PassiveStatic,
Incomplete,
HostEffectMask,
Hydrating,
Expand Down Expand Up @@ -344,10 +338,6 @@ let nestedPassiveUpdateCount: number = 0;
let currentEventTime: number = NoTimestamp;
let currentEventTransitionLane: Lanes = NoLanes;

// Dev only flag that tracks if passive effects are currently being flushed.
// We warn about state updates for unmounted components differently in this case.
let isFlushingPassiveEffects = false;

export function getWorkInProgressRoot(): FiberRoot | null {
return workInProgressRoot;
}
Expand Down Expand Up @@ -454,7 +444,6 @@ export function scheduleUpdateOnFiber(

const root = markUpdateLaneFromFiberToRoot(fiber, lane);
if (root === null) {
warnAboutUpdateOnUnmountedFiberInDEV(fiber);
return null;
}

Expand Down Expand Up @@ -2048,10 +2037,6 @@ function flushPassiveEffectsImpl() {
markPassiveEffectsStarted(lanes);
}

if (__DEV__) {
isFlushingPassiveEffects = true;
}

const prevExecutionContext = executionContext;
executionContext |= CommitContext;

Expand All @@ -2068,10 +2053,6 @@ function flushPassiveEffectsImpl() {
}
}

if (__DEV__) {
isFlushingPassiveEffects = false;
}

if (__DEV__) {
if (enableDebugTracing) {
logPassiveEffectsStopped();
Expand Down Expand Up @@ -2503,90 +2484,6 @@ function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) {
}
}

let didWarnStateUpdateForUnmountedComponent: Set<string> | null = null;
function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
if (__DEV__) {
const tag = fiber.tag;
if (
tag !== HostRoot &&
tag !== ClassComponent &&
tag !== FunctionComponent &&
tag !== ForwardRef &&
tag !== MemoComponent &&
tag !== SimpleMemoComponent
) {
// Only warn for user-defined components, not internal ones like Suspense.
return;
}

if ((fiber.flags & PassiveStatic) !== NoFlags) {
const updateQueue: FunctionComponentUpdateQueue | null = (fiber.updateQueue: any);
if (updateQueue !== null) {
const lastEffect = updateQueue.lastEffect;
if (lastEffect !== null) {
const firstEffect = lastEffect.next;

let effect = firstEffect;
do {
if (effect.destroy !== undefined) {
if ((effect.tag & HookPassive) !== NoHookEffect) {
return;
}
}
effect = effect.next;
} while (effect !== firstEffect);
}
}
}
// We show the whole stack but dedupe on the top component's name because
// the problematic code almost always lies inside that component.
const componentName = getComponentNameFromFiber(fiber) || 'ReactComponent';
if (didWarnStateUpdateForUnmountedComponent !== null) {
if (didWarnStateUpdateForUnmountedComponent.has(componentName)) {
return;
}
didWarnStateUpdateForUnmountedComponent.add(componentName);
} else {
didWarnStateUpdateForUnmountedComponent = new Set([componentName]);
}

if (isFlushingPassiveEffects) {
// Do not warn if we are currently flushing passive effects!
//
// React can't directly detect a memory leak, but there are some clues that warn about one.
// One of these clues is when an unmounted React component tries to update its state.
// For example, if a component forgets to remove an event listener when unmounting,
// that listener may be called later and try to update state,
// at which point React would warn about the potential leak.
//
// Warning signals are the most useful when they're strong.
// (So we should avoid false positive warnings.)
// Updating state from within an effect cleanup function is sometimes a necessary pattern, e.g.:
// 1. Updating an ancestor that a component had registered itself with on mount.
// 2. Resetting state when a component is hidden after going offscreen.
} else {
const previousFiber = ReactCurrentFiberCurrent;
try {
setCurrentDebugFiberInDEV(fiber);
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.',
tag === ClassComponent
? 'the componentWillUnmount method'
: 'a useEffect cleanup function',
);
} finally {
if (previousFiber) {
setCurrentDebugFiberInDEV(fiber);
} else {
resetCurrentDebugFiberInDEV();
}
}
}
}
}

let beginWork;
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
const dummyFiber = null;
Expand Down
Loading

0 comments on commit 7ed0706

Please sign in to comment.