Repository: facebook/react
Pull request: Use recursion to traverse during "disappear" and "reappear" layout phases

This converts the "disappear" and "reappear" layout phases to iterate over the effects recursively instead of iteratively. This makes it easier to track contextual information, like whether a fiber is inside a hidden tree.

We already made this change for several other phases, like mutation and layout mount. See https://github.com/facebook/react/commit/481dece5808d1207acd66a6a0365efee729cf0f7 for more context.

Changed files:
filename: packages/react-reconciler/src/ReactFiberCommitWork.new.js
patch: @@ -1132,11 +1132,12 @@ function commitLayoutEffectOnFiber(
           if (offscreenSubtreeWasHidden && !prevOffscreenSubtreeWasHidden) {
             // This is the root of a reappearing boundary. Turn its layout
             // effects back on.
-            // TODO: Convert this to use recursion
-            nextEffect = finishedWork;
-            reappearLayoutEffects_begin(finishedWork);
+            recursivelyTraverseReappearLayoutEffects(finishedWork);
           }
 
+          // 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,
@@ -1165,48 +1166,6 @@ function commitLayoutEffectOnFiber(
   }
 }
 
-function reappearLayoutEffectsOnFiber(node: Fiber) {
-  // Turn on layout effects in a tree that previously disappeared.
-  // TODO (Offscreen) Check: flags & LayoutStatic
-  switch (node.tag) {
-    case FunctionComponent:
-    case ForwardRef:
-    case SimpleMemoComponent: {
-      if (
-        enableProfilerTimer &&
-        enableProfilerCommitHooks &&
-        node.mode & ProfileMode
-      ) {
-        try {
-          startLayoutEffectTimer();
-          safelyCallCommitHookLayoutEffectListMount(node, node.return);
-        } finally {
-          recordLayoutEffectDuration(node);
-        }
-      } else {
-        safelyCallCommitHookLayoutEffectListMount(node, node.return);
-      }
-      break;
-    }
-    case ClassComponent: {
-      const instance = node.stateNode;
-      if (typeof instance.componentDidMount === 'function') {
-        safelyCallComponentDidMount(node, node.return, instance);
-      }
-      safelyAttachRef(node, node.return);
-      const updateQueue: UpdateQueue<*> | null = (node.updateQueue: any);
-      if (updateQueue !== null) {
-        commitHiddenCallbacks(updateQueue, instance);
-      }
-      break;
-    }
-    case HostComponent: {
-      safelyAttachRef(node, node.return);
-      break;
-    }
-  }
-}
-
 function commitTransitionProgress(offscreenFiber: Fiber) {
   if (enableTransitionTracing) {
     // This function adds suspense boundaries to the root
@@ -2773,60 +2732,89 @@ function recursivelyTraverseDisappearLayoutEffects(parentFiber: Fiber) {
   }
 }
 
-function reappearLayoutEffects_begin(subtreeRoot: Fiber) {
-  while (nextEffect !== null) {
-    const fiber = nextEffect;
-    const firstChild = fiber.child;
+function reappearLayoutEffects(finishedWork: Fiber) {
+  // Turn on layout effects in a tree that previously disappeared.
+  // TODO (Offscreen) Check: flags & LayoutStatic
+  switch (finishedWork.tag) {
+    case FunctionComponent:
+    case ForwardRef:
+    case SimpleMemoComponent: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
 
-    if (fiber.tag === OffscreenComponent) {
-      const isHidden = fiber.memoizedState !== null;
-      if (isHidden) {
-        // Nested Offscreen tree is still hidden. Don't re-appear its effects.
-        reappearLayoutEffects_complete(subtreeRoot);
-        continue;
+      // 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,
+        );
       }
+      break;
     }
+    case ClassComponent: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
 
-    // TODO (Offscreen) Check: subtreeFlags & LayoutStatic
-    if (firstChild !== null) {
-      // This node may have been reused from a previous render, so we can't
-      // assume its return pointer is correct.
-      firstChild.return = fiber;
-      nextEffect = firstChild;
-    } else {
-      reappearLayoutEffects_complete(subtreeRoot);
+      const instance = finishedWork.stateNode;
+      // TODO: Check for LayoutStatic flag
+      if (typeof instance.componentDidMount === 'function') {
+        safelyCallComponentDidMount(
+          finishedWork,
+          finishedWork.return,
+          instance,
+        );
+      }
+      // TODO: Check for RefStatic flag
+      safelyAttachRef(finishedWork, finishedWork.return);
+      const updateQueue: UpdateQueue<
+        *,
+      > | null = (finishedWork.updateQueue: any);
+      if (updateQueue !== null) {
+        commitHiddenCallbacks(updateQueue, instance);
+      }
+      break;
     }
-  }
-}
-
-function reappearLayoutEffects_complete(subtreeRoot: Fiber) {
-  while (nextEffect !== null) {
-    const fiber = nextEffect;
+    case HostComponent: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
 
-    // TODO (Offscreen) Check: flags & LayoutStatic
-    setCurrentDebugFiberInDEV(fiber);
-    try {
-      reappearLayoutEffectsOnFiber(fiber);
-    } catch (error) {
-      captureCommitPhaseError(fiber, fiber.return, error);
+      // TODO: Check for RefStatic flag
+      safelyAttachRef(finishedWork, finishedWork.return);
+      break;
     }
-    resetCurrentDebugFiberInDEV();
-
-    if (fiber === subtreeRoot) {
-      nextEffect = null;
-      return;
+    case OffscreenComponent: {
+      const isHidden = finishedWork.memoizedState !== null;
+      if (isHidden) {
+        // Nested Offscreen tree is still hidden. Don't re-appear its effects.
+      } else {
+        recursivelyTraverseReappearLayoutEffects(finishedWork);
+      }
+      break;
     }
-
-    const sibling = fiber.sibling;
-    if (sibling !== null) {
-      // This node may have been reused from a previous render, so we can't
-      // assume its return pointer is correct.
-      sibling.return = fiber.return;
-      nextEffect = sibling;
-      return;
+    default: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      break;
     }
+  }
+}
 
-    nextEffect = fiber.return;
+function recursivelyTraverseReappearLayoutEffects(parentFiber: Fiber) {
+  // TODO (Offscreen) Check: flags & (RefStatic | LayoutStatic)
+  let child = parentFiber.child;
+  while (child !== null) {
+    reappearLayoutEffects(child);
+    child = child.sibling;
   }
 }
 

filename: packages/react-reconciler/src/ReactFiberCommitWork.old.js
patch: @@ -1132,11 +1132,12 @@ function commitLayoutEffectOnFiber(
           if (offscreenSubtreeWasHidden && !prevOffscreenSubtreeWasHidden) {
             // This is the root of a reappearing boundary. Turn its layout
             // effects back on.
-            // TODO: Convert this to use recursion
-            nextEffect = finishedWork;
-            reappearLayoutEffects_begin(finishedWork);
+            recursivelyTraverseReappearLayoutEffects(finishedWork);
           }
 
+          // 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,
@@ -1165,48 +1166,6 @@ function commitLayoutEffectOnFiber(
   }
 }
 
-function reappearLayoutEffectsOnFiber(node: Fiber) {
-  // Turn on layout effects in a tree that previously disappeared.
-  // TODO (Offscreen) Check: flags & LayoutStatic
-  switch (node.tag) {
-    case FunctionComponent:
-    case ForwardRef:
-    case SimpleMemoComponent: {
-      if (
-        enableProfilerTimer &&
-        enableProfilerCommitHooks &&
-        node.mode & ProfileMode
-      ) {
-        try {
-          startLayoutEffectTimer();
-          safelyCallCommitHookLayoutEffectListMount(node, node.return);
-        } finally {
-          recordLayoutEffectDuration(node);
-        }
-      } else {
-        safelyCallCommitHookLayoutEffectListMount(node, node.return);
-      }
-      break;
-    }
-    case ClassComponent: {
-      const instance = node.stateNode;
-      if (typeof instance.componentDidMount === 'function') {
-        safelyCallComponentDidMount(node, node.return, instance);
-      }
-      safelyAttachRef(node, node.return);
-      const updateQueue: UpdateQueue<*> | null = (node.updateQueue: any);
-      if (updateQueue !== null) {
-        commitHiddenCallbacks(updateQueue, instance);
-      }
-      break;
-    }
-    case HostComponent: {
-      safelyAttachRef(node, node.return);
-      break;
-    }
-  }
-}
-
 function commitTransitionProgress(offscreenFiber: Fiber) {
   if (enableTransitionTracing) {
     // This function adds suspense boundaries to the root
@@ -2773,60 +2732,89 @@ function recursivelyTraverseDisappearLayoutEffects(parentFiber: Fiber) {
   }
 }
 
-function reappearLayoutEffects_begin(subtreeRoot: Fiber) {
-  while (nextEffect !== null) {
-    const fiber = nextEffect;
-    const firstChild = fiber.child;
+function reappearLayoutEffects(finishedWork: Fiber) {
+  // Turn on layout effects in a tree that previously disappeared.
+  // TODO (Offscreen) Check: flags & LayoutStatic
+  switch (finishedWork.tag) {
+    case FunctionComponent:
+    case ForwardRef:
+    case SimpleMemoComponent: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
 
-    if (fiber.tag === OffscreenComponent) {
-      const isHidden = fiber.memoizedState !== null;
-      if (isHidden) {
-        // Nested Offscreen tree is still hidden. Don't re-appear its effects.
-        reappearLayoutEffects_complete(subtreeRoot);
-        continue;
+      // 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,
+        );
       }
+      break;
     }
+    case ClassComponent: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
 
-    // TODO (Offscreen) Check: subtreeFlags & LayoutStatic
-    if (firstChild !== null) {
-      // This node may have been reused from a previous render, so we can't
-      // assume its return pointer is correct.
-      firstChild.return = fiber;
-      nextEffect = firstChild;
-    } else {
-      reappearLayoutEffects_complete(subtreeRoot);
+      const instance = finishedWork.stateNode;
+      // TODO: Check for LayoutStatic flag
+      if (typeof instance.componentDidMount === 'function') {
+        safelyCallComponentDidMount(
+          finishedWork,
+          finishedWork.return,
+          instance,
+        );
+      }
+      // TODO: Check for RefStatic flag
+      safelyAttachRef(finishedWork, finishedWork.return);
+      const updateQueue: UpdateQueue<
+        *,
+      > | null = (finishedWork.updateQueue: any);
+      if (updateQueue !== null) {
+        commitHiddenCallbacks(updateQueue, instance);
+      }
+      break;
     }
-  }
-}
-
-function reappearLayoutEffects_complete(subtreeRoot: Fiber) {
-  while (nextEffect !== null) {
-    const fiber = nextEffect;
+    case HostComponent: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
 
-    // TODO (Offscreen) Check: flags & LayoutStatic
-    setCurrentDebugFiberInDEV(fiber);
-    try {
-      reappearLayoutEffectsOnFiber(fiber);
-    } catch (error) {
-      captureCommitPhaseError(fiber, fiber.return, error);
+      // TODO: Check for RefStatic flag
+      safelyAttachRef(finishedWork, finishedWork.return);
+      break;
     }
-    resetCurrentDebugFiberInDEV();
-
-    if (fiber === subtreeRoot) {
-      nextEffect = null;
-      return;
+    case OffscreenComponent: {
+      const isHidden = finishedWork.memoizedState !== null;
+      if (isHidden) {
+        // Nested Offscreen tree is still hidden. Don't re-appear its effects.
+      } else {
+        recursivelyTraverseReappearLayoutEffects(finishedWork);
+      }
+      break;
     }
-
-    const sibling = fiber.sibling;
-    if (sibling !== null) {
-      // This node may have been reused from a previous render, so we can't
-      // assume its return pointer is correct.
-      sibling.return = fiber.return;
-      nextEffect = sibling;
-      return;
+    default: {
+      recursivelyTraverseReappearLayoutEffects(finishedWork);
+      break;
     }
+  }
+}
 
-    nextEffect = fiber.return;
+function recursivelyTraverseReappearLayoutEffects(parentFiber: Fiber) {
+  // TODO (Offscreen) Check: flags & (RefStatic | LayoutStatic)
+  let child = parentFiber.child;
+  while (child !== null) {
+    reappearLayoutEffects(child);
+    child = child.sibling;
   }
 }