diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 1c02667d7bc9b..d4eec830da3a5 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -307,7 +307,7 @@ describe('ReactCompositeComponent', () => { ReactDOM.render(, 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(
, 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); }); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 065d17fcc7533..7797448a82e97 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -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 | 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; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 710e853ce4fd7..163692456231e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -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 | 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; diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 8fe9b19815eb8..1cb098f93b7a1 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -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([, ]); - 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); }); });