Remove warning for dangling passive effects

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.
facebook · Oct 20, 2021 · bfc35df · bfc35df
1 parent 21cc354
commit bfc35df
Show file tree

Hide file tree

Showing 5 changed files with 2 additions and 113 deletions.
diff --git 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(
-          <React.StrictMode>
-            <App />
-          </React.StrictMode>,
-          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(<App />));
       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(<App />);
-      }).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
@@ -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<mixed> | void | null,
 ): void {
-  if (__DEV__) {
-    warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
-  }
   if (
     __DEV__ &&
     enableStrictEffects &&
@@ -1704,9 +1700,6 @@ function updateEffect(
   create: () => (() => void) | void,
   deps: Array<mixed> | 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
@@ -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<mixed> | void | null,
 ): void {
-  if (__DEV__) {
-    warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
-  }
   if (
     __DEV__ &&
     enableStrictEffects &&
@@ -1704,9 +1700,6 @@ function updateEffect(
   create: () => (() => void) | void,
   deps: Array<mixed> | 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
@@ -96,12 +96,7 @@ import {
   createWorkInProgress,
   assignFiberPropertiesInDEV,
 } from './ReactFiber.new';
-import {
-  NoMode,
-  StrictLegacyMode,
-  ProfileMode,
-  ConcurrentMode,
-} from './ReactTypeOfMode';
+import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode';
 import {
   HostRoot,
   IndeterminateComponent,
@@ -2820,34 +2815,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
@@ -96,12 +96,7 @@ import {
   createWorkInProgress,
   assignFiberPropertiesInDEV,
 } from './ReactFiber.old';
-import {
-  NoMode,
-  StrictLegacyMode,
-  ProfileMode,
-  ConcurrentMode,
-} from './ReactTypeOfMode';
+import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode';
 import {
   HostRoot,
   IndeterminateComponent,
@@ -2820,34 +2815,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) {