Repository: facebook/react
Pull request: Bugfix: Reused components should commit with same timing as new ones 

## Based on #24931

When an Offscreen tree goes from hidden -> visible, the tree may include both reused components that were unmounted when the tree was hidden, and also brand new components that didn't exist in the hidden tree.

Currently when this happens, we commit all the reused components' effects first, before committing the new ones, using two separate traversals of the tree.

Instead, we should fire all the effects with the same timing as if it were a completely new tree. [See the test I wrote for an example](https://github.com/facebook/react/blob/4f8fedd74dcb4f8d3764f1d41140830204b5e191/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js#L803-L868).

This is also more efficient because we only need to traverse the tree once.

Changed files:
filename: packages/react-reconciler/src/ReactFiberCommitWork.new.js
patch: @@ -225,18 +225,6 @@ const callComponentWillUnmountWithTimer = function(current, instance) {
   }
 };
 
-// Capture errors so they don't interrupt mounting.
-function safelyCallCommitHookLayoutEffectListMount(
-  current: Fiber,
-  nearestMountedAncestor: Fiber | null,
-) {
-  try {
-    commitHookEffectListMount(HookLayout, current);
-  } catch (error) {
-    captureCommitPhaseError(current, nearestMountedAncestor, error);
-  }
-}
-
 // Capture errors so they don't interrupt unmounting.
 function safelyCallComponentWillUnmount(
   current: Fiber,
@@ -250,19 +238,6 @@ function safelyCallComponentWillUnmount(
   }
 }
 
-// Capture errors so they don't interrupt mounting.
-function safelyCallComponentDidMount(
-  current: Fiber,
-  nearestMountedAncestor: Fiber | null,
-  instance: any,
-) {
-  try {
-    instance.componentDidMount();
-  } catch (error) {
-    captureCommitPhaseError(current, nearestMountedAncestor, error);
-  }
-}
-
 // Capture errors so they don't interrupt mounting.
 function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
   try {
@@ -706,7 +681,7 @@ export function commitPassiveEffectDurations(
   }
 }
 
-function commitHookLayoutEffects(finishedWork: Fiber) {
+function commitHookLayoutEffects(finishedWork: Fiber, hookFlags: HookFlags) {
   // At this point layout effects have already been destroyed (during mutation phase).
   // This is done to prevent sibling component effects from interfering with each other,
   // e.g. a destroy function in one component should never override a ref set
@@ -718,14 +693,14 @@ function commitHookLayoutEffects(finishedWork: Fiber) {
   ) {
     try {
       startLayoutEffectTimer();
-      commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
+      commitHookEffectListMount(hookFlags, finishedWork);
     } catch (error) {
       captureCommitPhaseError(finishedWork, finishedWork.return, error);
     }
     recordLayoutEffectDuration(finishedWork);
   } else {
     try {
-      commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
+      commitHookEffectListMount(hookFlags, finishedWork);
     } catch (error) {
       captureCommitPhaseError(finishedWork, finishedWork.return, error);
     }
@@ -853,7 +828,7 @@ function commitClassLayoutLifecycles(
   }
 }
 
-function commitClassCallbacks(finishedWork: Fiber, current: Fiber | null) {
+function commitClassCallbacks(finishedWork: Fiber) {
   // TODO: I think this is now always non-null by the time it reaches the
   // commit phase. Consider removing the type check.
   const updateQueue: UpdateQueue<*> | null = (finishedWork.updateQueue: any);
@@ -897,7 +872,7 @@ function commitClassCallbacks(finishedWork: Fiber, current: Fiber | null) {
   }
 }
 
-function commitHostComponentMount(finishedWork: Fiber, current: Fiber | null) {
+function commitHostComponentMount(finishedWork: Fiber) {
   const type = finishedWork.type;
   const props = finishedWork.memoizedProps;
   const instance: Instance = finishedWork.stateNode;
@@ -978,6 +953,8 @@ function commitLayoutEffectOnFiber(
   finishedWork: Fiber,
   committedLanes: Lanes,
 ): void {
+  // When updating this function, also update reappearLayoutEffects, which does
+  // most of the same things when an offscreen tree goes from hidden -> visible.
   const flags = finishedWork.flags;
   switch (finishedWork.tag) {
     case FunctionComponent:
@@ -989,9 +966,7 @@ function commitLayoutEffectOnFiber(
         committedLanes,
       );
       if (flags & Update) {
-        if (!offscreenSubtreeWasHidden) {
-          commitHookLayoutEffects(finishedWork);
-        }
+        commitHookLayoutEffects(finishedWork, HookLayout | HookHasEffect);
       }
       break;
     }
@@ -1002,19 +977,15 @@ function commitLayoutEffectOnFiber(
         committedLanes,
       );
       if (flags & Update) {
-        if (!offscreenSubtreeWasHidden) {
-          commitClassLayoutLifecycles(finishedWork, current);
-        }
+        commitClassLayoutLifecycles(finishedWork, current);
       }
 
       if (flags & Callback) {
-        commitClassCallbacks(finishedWork, current);
+        commitClassCallbacks(finishedWork);
       }
 
       if (flags & Ref) {
-        if (!offscreenSubtreeWasHidden) {
-          safelyAttachRef(finishedWork, finishedWork.return);
-        }
+        safelyAttachRef(finishedWork, finishedWork.return);
       }
       break;
     }
@@ -1063,13 +1034,11 @@ function commitLayoutEffectOnFiber(
       // These effects should only be committed when components are first mounted,
       // aka when there is no current/alternate.
       if (current === null && flags & Update) {
-        commitHostComponentMount(finishedWork, current);
+        commitHostComponentMount(finishedWork);
       }
 
       if (flags & Ref) {
-        if (!offscreenSubtreeWasHidden) {
-          safelyAttachRef(finishedWork, finishedWork.return);
-        }
+        safelyAttachRef(finishedWork, finishedWork.return);
       }
       break;
     }
@@ -1117,19 +1086,25 @@ function commitLayoutEffectOnFiber(
           offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
 
           if (offscreenSubtreeWasHidden && !prevOffscreenSubtreeWasHidden) {
-            // This is the root of a reappearing boundary. Turn its layout
-            // effects back on.
-            recursivelyTraverseReappearLayoutEffects(finishedWork);
+            // This is the root of a reappearing boundary. As we continue
+            // traversing the layout effects, we must also re-mount layout
+            // effects that were unmounted when the Offscreen subtree was
+            // hidden. So this is a superset of the normal commitLayoutEffects.
+            const includeWorkInProgressEffects =
+              (finishedWork.subtreeFlags & LayoutMask) !== NoFlags;
+            recursivelyTraverseReappearLayoutEffects(
+              finishedRoot,
+              finishedWork,
+              committedLanes,
+              includeWorkInProgressEffects,
+            );
+          } else {
+            recursivelyTraverseLayoutEffects(
+              finishedRoot,
+              finishedWork,
+              committedLanes,
+            );
           }
-
-          // TODO: We shouldn't traverse twice when reappearing layout effects.
-          // Move this into the else block of the above if statement, and modify
-          // reappearLayoutEffects to fire regular layout effects, too.
-          recursivelyTraverseLayoutEffects(
-            finishedRoot,
-            finishedWork,
-            committedLanes,
-          );
           offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
           offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
         }
@@ -2723,90 +2698,177 @@ function recursivelyTraverseDisappearLayoutEffects(parentFiber: Fiber) {
   }
 }
 
-function reappearLayoutEffects(finishedWork: Fiber) {
+function reappearLayoutEffects(
+  finishedRoot: FiberRoot,
+  current: Fiber | null,
+  finishedWork: Fiber,
+  committedLanes: Lanes,
+  // This function visits both newly finished work and nodes that were re-used
+  // from a previously committed tree. We cannot check non-static flags if the
+  // node was reused.
+  includeWorkInProgressEffects: boolean,
+) {
   // Turn on layout effects in a tree that previously disappeared.
-  // TODO (Offscreen) Check: flags & LayoutStatic
+  const flags = finishedWork.flags;
   switch (finishedWork.tag) {
     case FunctionComponent:
     case ForwardRef:
     case SimpleMemoComponent: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
-
-      // TODO: Check for LayoutStatic flag
-      if (
-        enableProfilerTimer &&
-        enableProfilerCommitHooks &&
-        finishedWork.mode & ProfileMode
-      ) {
-        try {
-          startLayoutEffectTimer();
-          safelyCallCommitHookLayoutEffectListMount(
-            finishedWork,
-            finishedWork.return,
-          );
-        } finally {
-          recordLayoutEffectDuration(finishedWork);
-        }
-      } else {
-        safelyCallCommitHookLayoutEffectListMount(
-          finishedWork,
-          finishedWork.return,
-        );
-      }
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
+      // TODO: Check flags & LayoutStatic
+      commitHookLayoutEffects(finishedWork, HookLayout);
       break;
     }
     case ClassComponent: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
 
-      const instance = finishedWork.stateNode;
       // TODO: Check for LayoutStatic flag
+      const instance = finishedWork.stateNode;
       if (typeof instance.componentDidMount === 'function') {
-        safelyCallComponentDidMount(
-          finishedWork,
-          finishedWork.return,
-          instance,
-        );
+        try {
+          instance.componentDidMount();
+        } catch (error) {
+          captureCommitPhaseError(finishedWork, finishedWork.return, error);
+        }
       }
-      // TODO: Check for RefStatic flag
-      safelyAttachRef(finishedWork, finishedWork.return);
+
+      // Commit any callbacks that would have fired while the component
+      // was hidden.
       const updateQueue: UpdateQueue<
         *,
       > | null = (finishedWork.updateQueue: any);
       if (updateQueue !== null) {
         commitHiddenCallbacks(updateQueue, instance);
       }
+
+      // If this is newly finished work, check for setState callbacks
+      if (includeWorkInProgressEffects && flags & Callback) {
+        commitClassCallbacks(finishedWork);
+      }
+
+      // TODO: Check flags & RefStatic
+      safelyAttachRef(finishedWork, finishedWork.return);
       break;
     }
+    // Unlike commitLayoutEffectsOnFiber, we don't need to handle HostRoot
+    // because this function only visits nodes that are inside an
+    // Offscreen fiber.
+    // case HostRoot: {
+    //  ...
+    // }
     case HostComponent: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
 
-      // TODO: Check for RefStatic flag
+      // Renderers may schedule work to be done after host components are mounted
+      // (eg DOM renderer may schedule auto-focus for inputs and form controls).
+      // These effects should only be committed when components are first mounted,
+      // aka when there is no current/alternate.
+      if (includeWorkInProgressEffects && current === null && flags & Update) {
+        commitHostComponentMount(finishedWork);
+      }
+
+      // TODO: Check flags & Ref
       safelyAttachRef(finishedWork, finishedWork.return);
       break;
     }
+    case Profiler: {
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
+      // TODO: Figure out how Profiler updates should work with Offscreen
+      if (includeWorkInProgressEffects && flags & Update) {
+        commitProfilerUpdate(finishedWork, current);
+      }
+      break;
+    }
+    case SuspenseComponent: {
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
+
+      // TODO: Figure out how Suspense hydration callbacks should work
+      // with Offscreen.
+      if (includeWorkInProgressEffects && flags & Update) {
+        commitSuspenseHydrationCallbacks(finishedRoot, finishedWork);
+      }
+      break;
+    }
     case OffscreenComponent: {
-      const isHidden = finishedWork.memoizedState !== null;
+      const offscreenState: OffscreenState = finishedWork.memoizedState;
+      const isHidden = offscreenState !== null;
       if (isHidden) {
         // Nested Offscreen tree is still hidden. Don't re-appear its effects.
       } else {
-        recursivelyTraverseReappearLayoutEffects(finishedWork);
+        recursivelyTraverseReappearLayoutEffects(
+          finishedRoot,
+          finishedWork,
+          committedLanes,
+          includeWorkInProgressEffects,
+        );
       }
       break;
     }
     default: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
       break;
     }
   }
 }
 
-function recursivelyTraverseReappearLayoutEffects(parentFiber: Fiber) {
+function recursivelyTraverseReappearLayoutEffects(
+  finishedRoot: FiberRoot,
+  parentFiber: Fiber,
+  committedLanes: Lanes,
+  includeWorkInProgressEffects: boolean,
+) {
+  // This function visits both newly finished work and nodes that were re-used
+  // from a previously committed tree. We cannot check non-static flags if the
+  // node was reused.
+  const childShouldIncludeWorkInProgressEffects =
+    includeWorkInProgressEffects &&
+    (parentFiber.subtreeFlags & LayoutMask) !== NoFlags;
+
   // TODO (Offscreen) Check: flags & (RefStatic | LayoutStatic)
+  const prevDebugFiber = getCurrentDebugFiberInDEV();
   let child = parentFiber.child;
   while (child !== null) {
-    reappearLayoutEffects(child);
+    const current = child.alternate;
+    reappearLayoutEffects(
+      finishedRoot,
+      current,
+      child,
+      committedLanes,
+      childShouldIncludeWorkInProgressEffects,
+    );
     child = child.sibling;
   }
+  setCurrentDebugFiberInDEV(prevDebugFiber);
 }
 
 export function commitPassiveMountEffects(

filename: packages/react-reconciler/src/ReactFiberCommitWork.old.js
patch: @@ -225,18 +225,6 @@ const callComponentWillUnmountWithTimer = function(current, instance) {
   }
 };
 
-// Capture errors so they don't interrupt mounting.
-function safelyCallCommitHookLayoutEffectListMount(
-  current: Fiber,
-  nearestMountedAncestor: Fiber | null,
-) {
-  try {
-    commitHookEffectListMount(HookLayout, current);
-  } catch (error) {
-    captureCommitPhaseError(current, nearestMountedAncestor, error);
-  }
-}
-
 // Capture errors so they don't interrupt unmounting.
 function safelyCallComponentWillUnmount(
   current: Fiber,
@@ -250,19 +238,6 @@ function safelyCallComponentWillUnmount(
   }
 }
 
-// Capture errors so they don't interrupt mounting.
-function safelyCallComponentDidMount(
-  current: Fiber,
-  nearestMountedAncestor: Fiber | null,
-  instance: any,
-) {
-  try {
-    instance.componentDidMount();
-  } catch (error) {
-    captureCommitPhaseError(current, nearestMountedAncestor, error);
-  }
-}
-
 // Capture errors so they don't interrupt mounting.
 function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
   try {
@@ -706,7 +681,7 @@ export function commitPassiveEffectDurations(
   }
 }
 
-function commitHookLayoutEffects(finishedWork: Fiber) {
+function commitHookLayoutEffects(finishedWork: Fiber, hookFlags: HookFlags) {
   // At this point layout effects have already been destroyed (during mutation phase).
   // This is done to prevent sibling component effects from interfering with each other,
   // e.g. a destroy function in one component should never override a ref set
@@ -718,14 +693,14 @@ function commitHookLayoutEffects(finishedWork: Fiber) {
   ) {
     try {
       startLayoutEffectTimer();
-      commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
+      commitHookEffectListMount(hookFlags, finishedWork);
     } catch (error) {
       captureCommitPhaseError(finishedWork, finishedWork.return, error);
     }
     recordLayoutEffectDuration(finishedWork);
   } else {
     try {
-      commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
+      commitHookEffectListMount(hookFlags, finishedWork);
     } catch (error) {
       captureCommitPhaseError(finishedWork, finishedWork.return, error);
     }
@@ -853,7 +828,7 @@ function commitClassLayoutLifecycles(
   }
 }
 
-function commitClassCallbacks(finishedWork: Fiber, current: Fiber | null) {
+function commitClassCallbacks(finishedWork: Fiber) {
   // TODO: I think this is now always non-null by the time it reaches the
   // commit phase. Consider removing the type check.
   const updateQueue: UpdateQueue<*> | null = (finishedWork.updateQueue: any);
@@ -897,7 +872,7 @@ function commitClassCallbacks(finishedWork: Fiber, current: Fiber | null) {
   }
 }
 
-function commitHostComponentMount(finishedWork: Fiber, current: Fiber | null) {
+function commitHostComponentMount(finishedWork: Fiber) {
   const type = finishedWork.type;
   const props = finishedWork.memoizedProps;
   const instance: Instance = finishedWork.stateNode;
@@ -978,6 +953,8 @@ function commitLayoutEffectOnFiber(
   finishedWork: Fiber,
   committedLanes: Lanes,
 ): void {
+  // When updating this function, also update reappearLayoutEffects, which does
+  // most of the same things when an offscreen tree goes from hidden -> visible.
   const flags = finishedWork.flags;
   switch (finishedWork.tag) {
     case FunctionComponent:
@@ -989,9 +966,7 @@ function commitLayoutEffectOnFiber(
         committedLanes,
       );
       if (flags & Update) {
-        if (!offscreenSubtreeWasHidden) {
-          commitHookLayoutEffects(finishedWork);
-        }
+        commitHookLayoutEffects(finishedWork, HookLayout | HookHasEffect);
       }
       break;
     }
@@ -1002,19 +977,15 @@ function commitLayoutEffectOnFiber(
         committedLanes,
       );
       if (flags & Update) {
-        if (!offscreenSubtreeWasHidden) {
-          commitClassLayoutLifecycles(finishedWork, current);
-        }
+        commitClassLayoutLifecycles(finishedWork, current);
       }
 
       if (flags & Callback) {
-        commitClassCallbacks(finishedWork, current);
+        commitClassCallbacks(finishedWork);
       }
 
       if (flags & Ref) {
-        if (!offscreenSubtreeWasHidden) {
-          safelyAttachRef(finishedWork, finishedWork.return);
-        }
+        safelyAttachRef(finishedWork, finishedWork.return);
       }
       break;
     }
@@ -1063,13 +1034,11 @@ function commitLayoutEffectOnFiber(
       // These effects should only be committed when components are first mounted,
       // aka when there is no current/alternate.
       if (current === null && flags & Update) {
-        commitHostComponentMount(finishedWork, current);
+        commitHostComponentMount(finishedWork);
       }
 
       if (flags & Ref) {
-        if (!offscreenSubtreeWasHidden) {
-          safelyAttachRef(finishedWork, finishedWork.return);
-        }
+        safelyAttachRef(finishedWork, finishedWork.return);
       }
       break;
     }
@@ -1117,19 +1086,25 @@ function commitLayoutEffectOnFiber(
           offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
 
           if (offscreenSubtreeWasHidden && !prevOffscreenSubtreeWasHidden) {
-            // This is the root of a reappearing boundary. Turn its layout
-            // effects back on.
-            recursivelyTraverseReappearLayoutEffects(finishedWork);
+            // This is the root of a reappearing boundary. As we continue
+            // traversing the layout effects, we must also re-mount layout
+            // effects that were unmounted when the Offscreen subtree was
+            // hidden. So this is a superset of the normal commitLayoutEffects.
+            const includeWorkInProgressEffects =
+              (finishedWork.subtreeFlags & LayoutMask) !== NoFlags;
+            recursivelyTraverseReappearLayoutEffects(
+              finishedRoot,
+              finishedWork,
+              committedLanes,
+              includeWorkInProgressEffects,
+            );
+          } else {
+            recursivelyTraverseLayoutEffects(
+              finishedRoot,
+              finishedWork,
+              committedLanes,
+            );
           }
-
-          // TODO: We shouldn't traverse twice when reappearing layout effects.
-          // Move this into the else block of the above if statement, and modify
-          // reappearLayoutEffects to fire regular layout effects, too.
-          recursivelyTraverseLayoutEffects(
-            finishedRoot,
-            finishedWork,
-            committedLanes,
-          );
           offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
           offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
         }
@@ -2723,90 +2698,177 @@ function recursivelyTraverseDisappearLayoutEffects(parentFiber: Fiber) {
   }
 }
 
-function reappearLayoutEffects(finishedWork: Fiber) {
+function reappearLayoutEffects(
+  finishedRoot: FiberRoot,
+  current: Fiber | null,
+  finishedWork: Fiber,
+  committedLanes: Lanes,
+  // This function visits both newly finished work and nodes that were re-used
+  // from a previously committed tree. We cannot check non-static flags if the
+  // node was reused.
+  includeWorkInProgressEffects: boolean,
+) {
   // Turn on layout effects in a tree that previously disappeared.
-  // TODO (Offscreen) Check: flags & LayoutStatic
+  const flags = finishedWork.flags;
   switch (finishedWork.tag) {
     case FunctionComponent:
     case ForwardRef:
     case SimpleMemoComponent: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
-
-      // TODO: Check for LayoutStatic flag
-      if (
-        enableProfilerTimer &&
-        enableProfilerCommitHooks &&
-        finishedWork.mode & ProfileMode
-      ) {
-        try {
-          startLayoutEffectTimer();
-          safelyCallCommitHookLayoutEffectListMount(
-            finishedWork,
-            finishedWork.return,
-          );
-        } finally {
-          recordLayoutEffectDuration(finishedWork);
-        }
-      } else {
-        safelyCallCommitHookLayoutEffectListMount(
-          finishedWork,
-          finishedWork.return,
-        );
-      }
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
+      // TODO: Check flags & LayoutStatic
+      commitHookLayoutEffects(finishedWork, HookLayout);
       break;
     }
     case ClassComponent: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
 
-      const instance = finishedWork.stateNode;
       // TODO: Check for LayoutStatic flag
+      const instance = finishedWork.stateNode;
       if (typeof instance.componentDidMount === 'function') {
-        safelyCallComponentDidMount(
-          finishedWork,
-          finishedWork.return,
-          instance,
-        );
+        try {
+          instance.componentDidMount();
+        } catch (error) {
+          captureCommitPhaseError(finishedWork, finishedWork.return, error);
+        }
       }
-      // TODO: Check for RefStatic flag
-      safelyAttachRef(finishedWork, finishedWork.return);
+
+      // Commit any callbacks that would have fired while the component
+      // was hidden.
       const updateQueue: UpdateQueue<
         *,
       > | null = (finishedWork.updateQueue: any);
       if (updateQueue !== null) {
         commitHiddenCallbacks(updateQueue, instance);
       }
+
+      // If this is newly finished work, check for setState callbacks
+      if (includeWorkInProgressEffects && flags & Callback) {
+        commitClassCallbacks(finishedWork);
+      }
+
+      // TODO: Check flags & RefStatic
+      safelyAttachRef(finishedWork, finishedWork.return);
       break;
     }
+    // Unlike commitLayoutEffectsOnFiber, we don't need to handle HostRoot
+    // because this function only visits nodes that are inside an
+    // Offscreen fiber.
+    // case HostRoot: {
+    //  ...
+    // }
     case HostComponent: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
 
-      // TODO: Check for RefStatic flag
+      // Renderers may schedule work to be done after host components are mounted
+      // (eg DOM renderer may schedule auto-focus for inputs and form controls).
+      // These effects should only be committed when components are first mounted,
+      // aka when there is no current/alternate.
+      if (includeWorkInProgressEffects && current === null && flags & Update) {
+        commitHostComponentMount(finishedWork);
+      }
+
+      // TODO: Check flags & Ref
       safelyAttachRef(finishedWork, finishedWork.return);
       break;
     }
+    case Profiler: {
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
+      // TODO: Figure out how Profiler updates should work with Offscreen
+      if (includeWorkInProgressEffects && flags & Update) {
+        commitProfilerUpdate(finishedWork, current);
+      }
+      break;
+    }
+    case SuspenseComponent: {
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
+
+      // TODO: Figure out how Suspense hydration callbacks should work
+      // with Offscreen.
+      if (includeWorkInProgressEffects && flags & Update) {
+        commitSuspenseHydrationCallbacks(finishedRoot, finishedWork);
+      }
+      break;
+    }
     case OffscreenComponent: {
-      const isHidden = finishedWork.memoizedState !== null;
+      const offscreenState: OffscreenState = finishedWork.memoizedState;
+      const isHidden = offscreenState !== null;
       if (isHidden) {
         // Nested Offscreen tree is still hidden. Don't re-appear its effects.
       } else {
-        recursivelyTraverseReappearLayoutEffects(finishedWork);
+        recursivelyTraverseReappearLayoutEffects(
+          finishedRoot,
+          finishedWork,
+          committedLanes,
+          includeWorkInProgressEffects,
+        );
       }
       break;
     }
     default: {
-      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      recursivelyTraverseReappearLayoutEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        includeWorkInProgressEffects,
+      );
       break;
     }
   }
 }
 
-function recursivelyTraverseReappearLayoutEffects(parentFiber: Fiber) {
+function recursivelyTraverseReappearLayoutEffects(
+  finishedRoot: FiberRoot,
+  parentFiber: Fiber,
+  committedLanes: Lanes,
+  includeWorkInProgressEffects: boolean,
+) {
+  // This function visits both newly finished work and nodes that were re-used
+  // from a previously committed tree. We cannot check non-static flags if the
+  // node was reused.
+  const childShouldIncludeWorkInProgressEffects =
+    includeWorkInProgressEffects &&
+    (parentFiber.subtreeFlags & LayoutMask) !== NoFlags;
+
   // TODO (Offscreen) Check: flags & (RefStatic | LayoutStatic)
+  const prevDebugFiber = getCurrentDebugFiberInDEV();
   let child = parentFiber.child;
   while (child !== null) {
-    reappearLayoutEffects(child);
+    const current = child.alternate;
+    reappearLayoutEffects(
+      finishedRoot,
+      current,
+      child,
+      committedLanes,
+      childShouldIncludeWorkInProgressEffects,
+    );
     child = child.sibling;
   }
+  setCurrentDebugFiberInDEV(prevDebugFiber);
 }
 
 export function commitPassiveMountEffects(

filename: packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
patch: @@ -696,4 +696,175 @@ describe('ReactOffscreen', () => {
     ]);
     expect(root).toMatchRenderedOutput(<span prop="C" />);
   });
+
+  // @gate enableOffscreen
+  it('does not call componentDidUpdate when reappearing a hidden class component', async () => {
+    class Child extends React.Component {
+      componentDidMount() {
+        Scheduler.unstable_yieldValue('componentDidMount');
+      }
+      componentDidUpdate() {
+        Scheduler.unstable_yieldValue('componentDidUpdate');
+      }
+      componentWillUnmount() {
+        Scheduler.unstable_yieldValue('componentWillUnmount');
+      }
+      render() {
+        return 'Child';
+      }
+    }
+
+    // Initial mount
+    const root = ReactNoop.createRoot();
+    await act(async () => {
+      root.render(
+        <Offscreen mode="visible">
+          <Child />
+        </Offscreen>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['componentDidMount']);
+
+    // Hide the class component
+    await act(async () => {
+      root.render(
+        <Offscreen mode="hidden">
+          <Child />
+        </Offscreen>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['componentWillUnmount']);
+
+    // Reappear the class component. componentDidMount should fire, not
+    // componentDidUpdate.
+    await act(async () => {
+      root.render(
+        <Offscreen mode="visible">
+          <Child />
+        </Offscreen>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['componentDidMount']);
+  });
+
+  // @gate enableOffscreen
+  it(
+    'when reusing old components (hidden -> visible), layout effects fire ' +
+      'with same timing as if it were brand new',
+    async () => {
+      function Child({label}) {
+        useLayoutEffect(() => {
+          Scheduler.unstable_yieldValue('Mount ' + label);
+          return () => {
+            Scheduler.unstable_yieldValue('Unmount ' + label);
+          };
+        }, [label]);
+        return label;
+      }
+
+      // Initial mount
+      const root = ReactNoop.createRoot();
+      await act(async () => {
+        root.render(
+          <Offscreen mode="visible">
+            <Child key="B" label="B" />
+          </Offscreen>,
+        );
+      });
+      expect(Scheduler).toHaveYielded(['Mount B']);
+
+      // Hide the component
+      await act(async () => {
+        root.render(
+          <Offscreen mode="hidden">
+            <Child key="B" label="B" />
+          </Offscreen>,
+        );
+      });
+      expect(Scheduler).toHaveYielded(['Unmount B']);
+
+      // Reappear the component and also add some new siblings.
+      await act(async () => {
+        root.render(
+          <Offscreen mode="visible">
+            <Child key="A" label="A" />
+            <Child key="B" label="B" />
+            <Child key="C" label="C" />
+          </Offscreen>,
+        );
+      });
+      // B's effect should fire in between A and C even though it's been reused
+      // from a previous render. In other words, it's the same order as if all
+      // three siblings were brand new.
+      expect(Scheduler).toHaveYielded(['Mount A', 'Mount B', 'Mount C']);
+    },
+  );
+
+  // @gate enableOffscreen
+  it(
+    'when reusing old components (hidden -> visible), layout effects fire ' +
+      'with same timing as if it were brand new (includes setState callback)',
+    async () => {
+      class Child extends React.Component {
+        componentDidMount() {
+          Scheduler.unstable_yieldValue('Mount ' + this.props.label);
+        }
+        componentWillUnmount() {
+          Scheduler.unstable_yieldValue('Unmount ' + this.props.label);
+        }
+        render() {
+          return this.props.label;
+        }
+      }
+
+      // Initial mount
+      const bRef = React.createRef();
+      const root = ReactNoop.createRoot();
+      await act(async () => {
+        root.render(
+          <Offscreen mode="visible">
+            <Child key="B" ref={bRef} label="B" />
+          </Offscreen>,
+        );
+      });
+      expect(Scheduler).toHaveYielded(['Mount B']);
+
+      // We're going to schedule an update on a hidden component, so stash a
+      // reference to its setState before the ref gets detached
+      const setStateB = bRef.current.setState.bind(bRef.current);
+
+      // Hide the component
+      await act(async () => {
+        root.render(
+          <Offscreen mode="hidden">
+            <Child key="B" ref={bRef} label="B" />
+          </Offscreen>,
+        );
+      });
+      expect(Scheduler).toHaveYielded(['Unmount B']);
+
+      // Reappear the component and also add some new siblings.
+      await act(async () => {
+        setStateB(null, () => {
+          Scheduler.unstable_yieldValue('setState callback B');
+        });
+        root.render(
+          <Offscreen mode="visible">
+            <Child key="A" label="A" />
+            <Child key="B" ref={bRef} label="B" />
+            <Child key="C" label="C" />
+          </Offscreen>,
+        );
+      });
+      // B's effect should fire in between A and C even though it's been reused
+      // from a previous render. In other words, it's the same order as if all
+      // three siblings were brand new.
+      expect(Scheduler).toHaveYielded([
+        'Mount A',
+        'Mount B',
+        'setState callback B',
+        'Mount C',
+      ]);
+    },
+  );
 });