Remove warning for dangling passive effects

acdlite · acdlite · commit 23ed7bf2e150 · 2021-10-21T18:28:26.000-04:00
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.
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
@@ -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
@@ -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) {
