Call cleanup of insertion effects when hidden (#30954)

Insertion effects do not unmount when a subtree is removed while
offscreen.

Current behavior for an insertion effect is if the component goes

- *visible -> removed:* calls insertion effect cleanup
- *visible -> offscreen -> removed:* insertion effect cleanup is never
called

This makes it so we always call insertion effect cleanup when removing
the component.

Likely also fixes #26670

---------

Co-authored-by: Rick Hanlon <rickhanlonii@fb.com>

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -40,6 +40,7 @@ import type {
 import {
   alwaysThrottleRetries,
   enableCreateEventHandleAPI,
+  enableHiddenSubtreeInsertionEffectCleanup,
   enablePersistedModeClonedFlag,
   enableProfilerTimer,
   enableProfilerCommitHooks,
@@ -147,6 +148,7 @@ import {
   getExecutionContext,
   CommitContext,
   NoContext,
+  setIsRunningInsertionEffect,
 } from './ReactFiberWorkLoop';
 import {
   NoFlags as NoHookEffect,
@@ -1324,7 +1326,78 @@ function commitDeletionEffectsOnFiber(
     case ForwardRef:
     case MemoComponent:
     case SimpleMemoComponent: {
-      if (!offscreenSubtreeWasHidden) {
+      if (enableHiddenSubtreeInsertionEffectCleanup) {
+        // When deleting a fiber, we may need to destroy insertion or layout effects.
+        // Insertion effects are not destroyed on hidden, only when destroyed, so now
+        // we need to destroy them. Layout effects are destroyed when hidden, so
+        // we only need to destroy them if the tree is visible.
+        const updateQueue: FunctionComponentUpdateQueue | null =
+          (deletedFiber.updateQueue: any);
+        if (updateQueue !== null) {
+          const lastEffect = updateQueue.lastEffect;
+          if (lastEffect !== null) {
+            const firstEffect = lastEffect.next;
+
+            let effect = firstEffect;
+            do {
+              const tag = effect.tag;
+              const inst = effect.inst;
+              const destroy = inst.destroy;
+              if (destroy !== undefined) {
+                if ((tag & HookInsertion) !== NoHookEffect) {
+                  // TODO: add insertion effect marks and profiling.
+                  if (__DEV__) {
+                    setIsRunningInsertionEffect(true);
+                  }
+
+                  inst.destroy = undefined;
+                  safelyCallDestroy(
+                    deletedFiber,
+                    nearestMountedAncestor,
+                    destroy,
+                  );
+
+                  if (__DEV__) {
+                    setIsRunningInsertionEffect(false);
+                  }
+                } else if (
+                  !offscreenSubtreeWasHidden &&
+                  (tag & HookLayout) !== NoHookEffect
+                ) {
+                  // Offscreen fibers already unmounted their layout effects.
+                  // We only need to destroy layout effects for visible trees.
+                  if (enableSchedulingProfiler) {
+                    markComponentLayoutEffectUnmountStarted(deletedFiber);
+                  }
+
+                  if (shouldProfile(deletedFiber)) {
+                    startLayoutEffectTimer();
+                    inst.destroy = undefined;
+                    safelyCallDestroy(
+                      deletedFiber,
+                      nearestMountedAncestor,
+                      destroy,
+                    );
+                    recordLayoutEffectDuration(deletedFiber);
+                  } else {
+                    inst.destroy = undefined;
+                    safelyCallDestroy(
+                      deletedFiber,
+                      nearestMountedAncestor,
+                      destroy,
+                    );
+                  }
+
+                  if (enableSchedulingProfiler) {
+                    markComponentLayoutEffectUnmountStopped();
+                  }
+                }
+              }
+              effect = effect.next;
+            } while (effect !== firstEffect);
+          }
+        }
+      } else if (!offscreenSubtreeWasHidden) {
         const updateQueue: FunctionComponentUpdateQueue | null =
           (deletedFiber.updateQueue: any);
         if (updateQueue !== null) {

packages/react-reconciler/src/__tests__/Activity-test.js

@@ -7,6 +7,7 @@ let Activity;
 let useState;
 let useLayoutEffect;
 let useEffect;
+let useInsertionEffect;
 let useMemo;
 let useRef;
 let startTransition;
@@ -25,6 +26,7 @@ describe('Activity', () => {
     LegacyHidden = React.unstable_LegacyHidden;
     Activity = React.unstable_Activity;
     useState = React.useState;
+    useInsertionEffect = React.useInsertionEffect;
     useLayoutEffect = React.useLayoutEffect;
     useEffect = React.useEffect;
     useMemo = React.useMemo;
@@ -43,6 +45,13 @@ describe('Activity', () => {
   }
 
   function LoggedText({text, children}) {
+    useInsertionEffect(() => {
+      Scheduler.log(`mount insertion ${text}`);
+      return () => {
+        Scheduler.log(`unmount insertion ${text}`);
+      };
+    });
+
     useEffect(() => {
       Scheduler.log(`mount ${text}`);
       return () => {
@@ -1436,6 +1445,63 @@ describe('Activity', () => {
     );
   });
 
+  // @gate enableActivity
+  it('insertion effects are not disconnected when the visibility changes', async () => {
+    function Child({step}) {
+      useInsertionEffect(() => {
+        Scheduler.log(`Commit mount [${step}]`);
+        return () => {
+          Scheduler.log(`Commit unmount [${step}]`);
+        };
+      }, [step]);
+      return <Text text={step} />;
+    }
+
+    function App({show, step}) {
+      return (
+        <Activity mode={show ? 'visible' : 'hidden'}>
+          {useMemo(
+            () => (
+              <Child step={step} />
+            ),
+            [step],
+          )}
+        </Activity>
+      );
+    }
+
+    const root = ReactNoop.createRoot();
+    await act(() => {
+      root.render(<App show={true} step={1} />);
+    });
+    assertLog([1, 'Commit mount [1]']);
+    expect(root).toMatchRenderedOutput(<span prop={1} />);
+
+    // Hide the tree. This will not unmount insertion effects.
+    await act(() => {
+      root.render(<App show={false} step={1} />);
+    });
+    assertLog([]);
+    expect(root).toMatchRenderedOutput(<span hidden={true} prop={1} />);
+
+    // Update.
+    await act(() => {
+      root.render(<App show={false} step={2} />);
+    });
+    // The update is pre-rendered so insertion effects are fired
+    assertLog([2, 'Commit unmount [1]', 'Commit mount [2]']);
+    expect(root).toMatchRenderedOutput(<span hidden={true} prop={2} />);
+
+    // Reveal the tree.
+    await act(() => {
+      root.render(<App show={true} step={2} />);
+    });
+    // The update doesn't render because it was already pre-rendered, and the
+    // insertion effect already fired.
+    assertLog([]);
+    expect(root).toMatchRenderedOutput(<span prop={2} />);
+  });
+
   describe('manual interactivity', () => {
     // @gate enableActivity
     it('should attach ref only for mode null', async () => {
@@ -1904,6 +1970,9 @@ describe('Activity', () => {
       'outer',
       'middle',
       'inner',
+      'mount insertion inner',
+      'mount insertion middle',
+      'mount insertion outer',
       'mount layout inner',
       'mount layout middle',
       'mount layout outer',
@@ -1964,6 +2033,22 @@ describe('Activity', () => {
     });
 
     assertLog(['unmount layout inner', 'unmount inner']);
+
+    await act(() => {
+      root.render(null);
+    });
+
+    assertLog([
+      'unmount insertion outer',
+      'unmount layout outer',
+      'unmount insertion middle',
+      'unmount layout middle',
+      ...(gate('enableHiddenSubtreeInsertionEffectCleanup')
+        ? ['unmount insertion inner']
+        : []),
+      'unmount outer',
+      'unmount middle',
+    ]);
   });
 
   // @gate enableActivity

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js

@@ -19,6 +19,7 @@ let resolveText;
 let ReactNoop;
 let Scheduler;
 let Suspense;
+let Activity;
 let useState;
 let useReducer;
 let useEffect;
@@ -64,6 +65,7 @@ describe('ReactHooksWithNoopRenderer', () => {
     useTransition = React.useTransition;
     useDeferredValue = React.useDeferredValue;
     Suspense = React.Suspense;
+    Activity = React.unstable_Activity;
     ContinuousEventPriority =
       require('react-reconciler/constants').ContinuousEventPriority;
     if (gate(flags => flags.enableSuspenseList)) {
@@ -2997,6 +2999,57 @@ describe('ReactHooksWithNoopRenderer', () => {
         root.render(<NotInsertion />);
       });
     });
+
+    // @gate enableActivity
+    it('warns when setState is called from offscreen deleted insertion effect cleanup', async () => {
+      function App(props) {
+        const [, setX] = useState(0);
+        useInsertionEffect(() => {
+          if (props.throw) {
+            throw Error('No');
+          }
+          return () => {
+            setX(1);
+          };
+        }, [props.throw, props.foo]);
+        return null;
+      }
+
+      const root = ReactNoop.createRoot();
+      await act(() => {
+        root.render(
+          <Activity mode="hidden">
+            <App foo="hello" />
+          </Activity>,
+        );
+      });
+
+      if (gate(flags => flags.enableHiddenSubtreeInsertionEffectCleanup)) {
+        await expect(async () => {
+          await act(() => {
+            root.render(<Activity mode="hidden" />);
+          });
+        }).toErrorDev(['useInsertionEffect must not schedule updates.']);
+      } else {
+        await expect(async () => {
+          await act(() => {
+            root.render(<Activity mode="hidden" />);
+          });
+        }).toErrorDev([]);
+      }
+
+      // Should not warn for regular effects after throw.
+      function NotInsertion() {
+        const [, setX] = useState(0);
+        useEffect(() => {
+          setX(1);
+        }, []);
+        return null;
+      }
+      await act(() => {
+        root.render(<NotInsertion />);
+      });
+    });
   });
 
   describe('useLayoutEffect', () => {