Repository: facebook/react
Pull request: Remove the warning for setState on unmounted components

We have a warning that fires when you `setState` on an unmounted components. This is a proposal to remove it.

### Why was this warning added?

The warning states that it protects against memory leaks. The original use case (rewritten to Hooks) goes like this:

```js
useEffect(() => {
  function handleChange() {
     setState(store.getState())
  }
  store.subscribe(handleChange)
  return () => store.unsubscribe(handleChange)
}, [])
```

If you forget the `unsubscribe` call, after the component unmounts you'll get a memory leak. So we warn you if you set state after unmounting because we think you might've forgotten to do that.

### Why does this warning create problems?

The problem is that in practice, there's usually a much more common case in the user code:

```js
async function handleSubmit() {
  setPending(true)
  await post('/someapi') // component might unmount while we're waiting
  setPending(false)
}
```

This will trigger the `setState` warning. However, there is no actual problem here. There's no "memory leak" here: the POST comes back, we try to set state, the component is unmounted (so nothing happens), and then we're done. Nothing keeps holding onto the component indefinitely.

### Avoiding false positives is too difficult

So effectively the above case is a false positive but that's the most common one people try to solve. Even the "canonical" solution to this is pretty clumsy:

```js
let isMountedRef = useRef(false)
useEffect(() => {
  isMountedRef.current = true
  return () => {
    isMountedRef.current = false
  }
}, [])

async function handleSubmit() {
  setPending(true)
  await post('/someapi')
  if (isMountedRef.current) {
    setPending(false)
  }
}
```

It's unfortunate because we're adding slightly more runtime overhead, slightly more code, and making the code slightly less readable — all to avoid an effectively false positive warning.

Bu there are other issues, too.

In the future, we'd like to offer a feature that lets you preserve DOM and state even when the component is not visible, but disconnect its effects. With this feature, the code above doesn't work well. You'll "miss" the `setPending(false)` call because `isMountedRef.current` is `false` while the component is in a hidden tab. So once you return to the hidden tab and make it visible again, it will look as if your mutation hasn't "come through". By trying to evade the warning, we've made the behavior worse.

In addition, we've seen that this warning pushes people towards moving mutations (like POST) into effects just because effects offer an easy way to "ignore" something using the cleanup function and a flag. However, this *also* usually makes the code worse. The user action is associated with a particular event — not with the component being displayed — and so moving this code into effect introduces extra fragility. Such as the risk of the effect re-firing when some related data changes. So the warning against a problematic pattern ends up pushing people towards _more_ problematic patterns though the original code was actually fine.

### Could we not remove this?

We could make the warning less aggressive. For example, `console.warn` instead of `console.error`. However, as long as it shows up, the institutional knowledge will push people towards using a ref or moving it to an effect, and neither is desirable.

We could make add some threshold to the warning. Such as only firing it after N setStates or M seconds. However, it's hard to find one-size-fit-all heuristics, and it's probably going to be confusing either way.

In practice, direct subscriptions in components are not that common anymore. Especially now that we have custom Hooks. They tend to be concentrated in libraries, which eventually would likely move to `useMutableSource` where appropriate anyway.

So the proposal is to stop compromising the common product case, and not warn at all.

Changed files:
filename: packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
patch: @@ -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);
 
@@ -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);
 
@@ -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);
   });
 

filename: packages/react-reconciler/src/ReactFiberWorkLoop.new.js
patch: @@ -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 {
@@ -53,10 +52,6 @@ import {
   scheduleSyncCallback,
   scheduleLegacySyncCallback,
 } from './ReactFiberSyncTaskQueue.new';
-import {
-  NoFlags as NoHookEffect,
-  Passive as HookPassive,
-} from './ReactHookEffectTags';
 import {
   logCommitStarted,
   logCommitStopped,
@@ -119,7 +114,6 @@ import {LegacyRoot} from './ReactRootTags';
 import {
   NoFlags,
   Placement,
-  PassiveStatic,
   Incomplete,
   HostEffectMask,
   Hydrating,
@@ -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;
 }
@@ -454,7 +444,6 @@ export function scheduleUpdateOnFiber(
 
   const root = markUpdateLaneFromFiberToRoot(fiber, lane);
   if (root === null) {
-    warnAboutUpdateOnUnmountedFiberInDEV(fiber);
     return null;
   }
 
@@ -2048,10 +2037,6 @@ function flushPassiveEffectsImpl() {
     markPassiveEffectsStarted(lanes);
   }
 
-  if (__DEV__) {
-    isFlushingPassiveEffects = true;
-  }
-
   const prevExecutionContext = executionContext;
   executionContext |= CommitContext;
 
@@ -2068,10 +2053,6 @@ function flushPassiveEffectsImpl() {
     }
   }
 
-  if (__DEV__) {
-    isFlushingPassiveEffects = false;
-  }
-
   if (__DEV__) {
     if (enableDebugTracing) {
       logPassiveEffectsStopped();
@@ -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;

filename: packages/react-reconciler/src/ReactFiberWorkLoop.old.js
patch: @@ -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 {
@@ -53,10 +52,6 @@ import {
   scheduleSyncCallback,
   scheduleLegacySyncCallback,
 } from './ReactFiberSyncTaskQueue.old';
-import {
-  NoFlags as NoHookEffect,
-  Passive as HookPassive,
-} from './ReactHookEffectTags';
 import {
   logCommitStarted,
   logCommitStopped,
@@ -119,7 +114,6 @@ import {LegacyRoot} from './ReactRootTags';
 import {
   NoFlags,
   Placement,
-  PassiveStatic,
   Incomplete,
   HostEffectMask,
   Hydrating,
@@ -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;
 }
@@ -454,7 +444,6 @@ export function scheduleUpdateOnFiber(
 
   const root = markUpdateLaneFromFiberToRoot(fiber, lane);
   if (root === null) {
-    warnAboutUpdateOnUnmountedFiberInDEV(fiber);
     return null;
   }
 
@@ -2048,10 +2037,6 @@ function flushPassiveEffectsImpl() {
     markPassiveEffectsStarted(lanes);
   }
 
-  if (__DEV__) {
-    isFlushingPassiveEffects = true;
-  }
-
   const prevExecutionContext = executionContext;
   executionContext |= CommitContext;
 
@@ -2068,10 +2053,6 @@ function flushPassiveEffectsImpl() {
     }
   }
 
-  if (__DEV__) {
-    isFlushingPassiveEffects = false;
-  }
-
   if (__DEV__) {
     if (enableDebugTracing) {
       logPassiveEffectsStopped();
@@ -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;

filename: packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
patch: @@ -360,7 +360,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       expect(firstUpdater).toBe(secondUpdater);
     });
 
-    it('warns on set after unmount', () => {
+    it('does not warn on set after unmount', () => {
       let _updateCount;
       function Counter(props, ref) {
         const [, updateCount] = useState(0);
@@ -372,49 +372,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       expect(Scheduler).toFlushWithoutYielding();
       ReactNoop.render(null);
       expect(Scheduler).toFlushWithoutYielding();
-      expect(() => act(() => _updateCount(1))).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 a useEffect cleanup function.\n' +
-          '    in Counter (at **)',
-      );
-    });
-
-    it('dedupes the warning by component name', () => {
-      let _updateCountA;
-      function CounterA(props, ref) {
-        const [, updateCount] = useState(0);
-        _updateCountA = updateCount;
-        return null;
-      }
-      let _updateCountB;
-      function CounterB(props, ref) {
-        const [, updateCount] = useState(0);
-        _updateCountB = updateCount;
-        return null;
-      }
-
-      ReactNoop.render([<CounterA key="A" />, <CounterB key="B" />]);
-      expect(Scheduler).toFlushWithoutYielding();
-      ReactNoop.render(null);
-      expect(Scheduler).toFlushWithoutYielding();
-      expect(() => act(() => _updateCountA(1))).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 a useEffect cleanup function.\n' +
-          '    in CounterA (at **)',
-      );
-      // already cached so this logs no error
-      act(() => _updateCountA(2));
-      expect(() => act(() => _updateCountB(1))).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 a useEffect cleanup function.\n' +
-          '    in CounterB (at **)',
-      );
+      act(() => _updateCount(1));
     });
 
     it('works with memo', () => {
@@ -1401,7 +1359,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       });
     });
 
-    it('warns about state updates for unmounted components with no pending passive unmounts', () => {
+    it('does not warn about state updates for unmounted components with no pending passive unmounts', () => {
       let completePendingRequest = null;
       function Component() {
         Scheduler.unstable_yieldValue('Component');
@@ -1432,13 +1390,11 @@ describe('ReactHooksWithNoopRenderer', () => {
         expect(Scheduler).toFlushAndYieldThrough(['layout destroy']);
 
         // Simulate an XHR completing.
-        expect(completePendingRequest).toErrorDev(
-          "Warning: Can't perform a React state update on an unmounted component.",
-        );
+        completePendingRequest();
       });
     });
 
-    it('still warns if there are pending passive unmount effects but not for the current fiber', () => {
+    it('does not warn if there are pending passive unmount effects but not for the current fiber', () => {
       let completePendingRequest = null;
       function ComponentWithXHR() {
         Scheduler.unstable_yieldValue('Component');
@@ -1492,13 +1448,11 @@ describe('ReactHooksWithNoopRenderer', () => {
         expect(Scheduler).toFlushAndYieldThrough(['a:layout destroy']);
 
         // Simulate an XHR completing in the component without a pending passive effect..
-        expect(completePendingRequest).toErrorDev(
-          "Warning: Can't perform a React state update on an unmounted component.",
-        );
+        completePendingRequest();
       });
     });
 
-    it('warns if there are updates after pending passive unmount effects have been flushed', () => {
+    it('does not warn if there are updates after pending passive unmount effects have been flushed', () => {
       let updaterFunction;
 
       function Component() {
@@ -1529,14 +1483,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       expect(Scheduler).toFlushAndYield(['passive destroy']);
 
       act(() => {
-        expect(() => {
-          updaterFunction(true);
-        }).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 a useEffect cleanup function.\n' +
-            '    in Component (at **)',
-        );
+        updaterFunction(true);
       });
     });