From 3c4c1c4703e8f7362370c44a48ac9348229e3791 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 21 Oct 2021 18:39:59 -0400 Subject: [PATCH] Remove warning for dangling passive effects (#22609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In legacy mode, a test can get into a situation where passive effects are "dangling" — an update finished, and scheduled some passive effects, but the effects don't flush. This is why React warns if you don't wrap updates in act. The act API is responsible for flushing passive effects. But there are some cases where the act API (in legacy roots) intentionally doesn't warn, like updates that originate from roots and classes. It's possible those updates will render children that contain useEffect. Because of this, dangling effects are still possible, and React doesn't warn about it. So we implemented a second act warning for dangling effects. However, in concurrent roots, we now enforce that all APIs that schedule React work must be wrapped in act. There's no scenario where dangling passive effects can happen that doesn't already trigger the warning for updates. So the dangling effects warning is redundant. The warning was never part of a public release. It was only enabled in concurrent roots. So we can delete it. --- .../src/__tests__/ReactTestUtilsAct-test.js | 31 ---------------- .../src/ReactFiberHooks.new.js | 7 ---- .../src/ReactFiberHooks.old.js | 7 ---- .../src/ReactFiberWorkLoop.new.js | 35 +------------------ .../src/ReactFiberWorkLoop.old.js | 35 +------------------ 5 files changed, 2 insertions(+), 113 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js index 556772500ee2f..71adc286d08ba 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js @@ -98,43 +98,12 @@ describe('ReactTestUtils.act()', () => { }).toErrorDev([]); }); - it('warns in strict mode', () => { - expect(() => { - ReactDOM.render( - - - , - document.createElement('div'), - ); - }).toErrorDev([ - 'An update to App ran an effect, but was not wrapped in act(...)', - ]); - }); - // @gate __DEV__ it('does not warn in concurrent mode', () => { const root = ReactDOM.createRoot(document.createElement('div')); act(() => root.render()); Scheduler.unstable_flushAll(); }); - - it('warns in concurrent mode if root is strict', () => { - // TODO: We don't need this error anymore in concurrent mode because - // effects can only be scheduled as the result of an update, and we now - // enforce all updates must be wrapped with act, not just hook updates. - expect(() => { - const root = ReactDOM.createRoot(document.createElement('div'), { - unstable_strictMode: true, - }); - root.render(); - }).toErrorDev( - 'An update to Root inside a test was not wrapped in act(...)', - {withoutStack: true}, - ); - expect(() => Scheduler.unstable_flushAll()).toErrorDev( - 'An update to App ran an effect, but was not wrapped in act(...)', - ); - }); }); }); diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 266e98960cbb3..c3b76549a22aa 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -82,7 +82,6 @@ import { scheduleUpdateOnFiber, requestUpdateLane, requestEventTime, - warnIfNotCurrentlyActingEffectsInDEV, markSkippedUpdateLanes, isInterleavedUpdate, } from './ReactFiberWorkLoop.new'; @@ -1676,9 +1675,6 @@ function mountEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - if (__DEV__) { - warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); - } if ( __DEV__ && enableStrictEffects && @@ -1704,9 +1700,6 @@ function updateEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - if (__DEV__) { - warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); - } return updateEffectImpl(PassiveEffect, HookPassive, create, deps); } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index b2378cbf30d65..8bc1510deb455 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -82,7 +82,6 @@ import { scheduleUpdateOnFiber, requestUpdateLane, requestEventTime, - warnIfNotCurrentlyActingEffectsInDEV, markSkippedUpdateLanes, isInterleavedUpdate, } from './ReactFiberWorkLoop.old'; @@ -1676,9 +1675,6 @@ function mountEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - if (__DEV__) { - warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); - } if ( __DEV__ && enableStrictEffects && @@ -1704,9 +1700,6 @@ function updateEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - if (__DEV__) { - warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); - } return updateEffectImpl(PassiveEffect, HookPassive, create, deps); } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 2c065d6ee17ab..e35e5515bb0bf 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -97,12 +97,7 @@ import { createWorkInProgress, assignFiberPropertiesInDEV, } from './ReactFiber.new'; -import { - NoMode, - StrictLegacyMode, - ProfileMode, - ConcurrentMode, -} from './ReactTypeOfMode'; +import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode'; import { HostRoot, IndeterminateComponent, @@ -2860,34 +2855,6 @@ function shouldForceFlushFallbacksInDEV() { return __DEV__ && ReactCurrentActQueue.current !== null; } -export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void { - if (__DEV__) { - const isActEnvironment = - fiber.mode & ConcurrentMode - ? isConcurrentActEnvironment() - : isLegacyActEnvironment(fiber); - if ( - isActEnvironment && - (fiber.mode & StrictLegacyMode) !== NoMode && - ReactCurrentActQueue.current === null - ) { - console.error( - 'An update to %s ran an effect, but was not wrapped in act(...).\n\n' + - 'When testing, code that causes React state updates should be ' + - 'wrapped into act(...):\n\n' + - 'act(() => {\n' + - ' /* fire events that update state */\n' + - '});\n' + - '/* assert on the output */\n\n' + - "This ensures that you're testing the behavior the user would see " + - 'in the browser.' + - ' Learn more at https://reactjs.org/link/wrap-tests-with-act', - getComponentNameFromFiber(fiber), - ); - } - } -} - function warnIfUpdatesNotWrappedWithActDEV(fiber: Fiber): void { if (__DEV__) { if (fiber.mode & ConcurrentMode) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 97b835d3a18e4..cde46adb01ab0 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -97,12 +97,7 @@ import { createWorkInProgress, assignFiberPropertiesInDEV, } from './ReactFiber.old'; -import { - NoMode, - StrictLegacyMode, - ProfileMode, - ConcurrentMode, -} from './ReactTypeOfMode'; +import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode'; import { HostRoot, IndeterminateComponent, @@ -2860,34 +2855,6 @@ function shouldForceFlushFallbacksInDEV() { return __DEV__ && ReactCurrentActQueue.current !== null; } -export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void { - if (__DEV__) { - const isActEnvironment = - fiber.mode & ConcurrentMode - ? isConcurrentActEnvironment() - : isLegacyActEnvironment(fiber); - if ( - isActEnvironment && - (fiber.mode & StrictLegacyMode) !== NoMode && - ReactCurrentActQueue.current === null - ) { - console.error( - 'An update to %s ran an effect, but was not wrapped in act(...).\n\n' + - 'When testing, code that causes React state updates should be ' + - 'wrapped into act(...):\n\n' + - 'act(() => {\n' + - ' /* fire events that update state */\n' + - '});\n' + - '/* assert on the output */\n\n' + - "This ensures that you're testing the behavior the user would see " + - 'in the browser.' + - ' Learn more at https://reactjs.org/link/wrap-tests-with-act', - getComponentNameFromFiber(fiber), - ); - } - } -} - function warnIfUpdatesNotWrappedWithActDEV(fiber: Fiber): void { if (__DEV__) { if (fiber.mode & ConcurrentMode) {