[Fiber] Track Appearing Named ViewTransition in the accumulateSuspenseyCommit Phase (#32254)

When a named ViewTransition component unmounts in one place and mounts in a different place we need to match these up so we know a pair has been created. Since the unmounts are tracked in the snapshot phase we need some way to track the mounts before that. Originally the way I did that is by reusing the render phase since there was no other phase in the commit before that. However, that's not quite correct. Just because something is visited in render doesn't mean it'll commit. E.g. if that tree ends up suspending or erroring. Which would lead to a false positive on match. The unmount shouldn't animate in that case. (Un)fortunately we have already added a traversal before the snapshot phase for tracking suspensey CSS. The `accumulateSuspenseyCommit` phase. This needs to find new mounts of Suspensey CSS or if there was a reappearing Offscreen boundary it needs to find any Suspensey CSS already inside that tree. This is exactly the same traversal we need to find newly appearing View Transition components. So we can just reuse that.
2025-11-01 09:12:30 +00:00 · 2025-01-30 12:13:36 -05:00
parent f02ba2fcc5
commit 4b3728f05e
5 changed files with 66 additions and 122 deletions
@@ -97,6 +97,7 @@ import {
  Passive,
  DidDefer,
  ViewTransitionNamedStatic,
+  ViewTransitionNamedMount,
  LayoutStatic,
 } from './ReactFiberFlags';
 import {
@@ -266,7 +267,6 @@ import {
  markSkippedUpdateLanes,
  getWorkInProgressRoot,
  peekDeferredLane,
-  trackAppearingViewTransition,
 } from './ReactFiberWorkLoop';
 import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates';
 import {pushCacheProvider, CacheContext} from './ReactFiberCacheComponent';
@@ -3243,12 +3243,10 @@ function updateViewTransition(
  if (pendingProps.name != null && pendingProps.name !== 'auto') {
    // Explicitly named boundary. We track it so that we can pair it up with another explicit
    // boundary if we get deleted.
-    workInProgress.flags |= ViewTransitionNamedStatic;
-    if (current === null) {
-      // This is a new mount. We track it in case we end up having a deletion with the same name.
-      // TODO: A problem with this strategy is that this subtree might not actually end up mounted.
-      trackAppearingViewTransition(instance, pendingProps.name);
-    }
+    workInProgress.flags |=
+      current === null
+        ? ViewTransitionNamedMount | ViewTransitionNamedStatic
+        : ViewTransitionNamedStatic;
  } else {
    // Assign an auto generated name using the useId algorthim if an explicit one is not provided.
    // We don't need the name yet but we do it here to allow hydration state to be used.
@@ -276,6 +276,10 @@ export let shouldFireAfterActiveInstanceBlur: boolean = false;

 export let shouldStartViewTransition: boolean = false;

+// This tracks named ViewTransition components found in the accumulateSuspenseyCommit
+// phase that might need to find deleted pairs in the beforeMutation phase.
+let appearingViewTransitions: Map<string, ViewTransitionState> | null = null;
+
 // Used during the commit phase to track whether a parent ViewTransition component
 // might have been affected by any mutations / relayouts below.
 let viewTransitionContextChanged: boolean = false;
@@ -288,7 +292,6 @@ export function commitBeforeMutationEffects(
  root: FiberRoot,
  firstChild: Fiber,
  committedLanes: Lanes,
-  appearingViewTransitions: Map<string, ViewTransitionState> | null,
 ): void {
  focusedInstanceHandle = prepareForCommit(root.containerInfo);
  shouldFireAfterActiveInstanceBlur = false;
@@ -299,19 +302,15 @@ export function commitBeforeMutationEffects(
    includesOnlyViewTransitionEligibleLanes(committedLanes);

  nextEffect = firstChild;
-  commitBeforeMutationEffects_begin(
-    isViewTransitionEligible,
-    appearingViewTransitions,
-  );
+  commitBeforeMutationEffects_begin(isViewTransitionEligible);

  // We no longer need to track the active instance fiber
  focusedInstanceHandle = null;
+  // We've found any matched pairs and can now reset.
+  appearingViewTransitions = null;
 }

-function commitBeforeMutationEffects_begin(
-  isViewTransitionEligible: boolean,
-  appearingViewTransitions: Map<string, ViewTransitionState> | null,
-) {
+function commitBeforeMutationEffects_begin(isViewTransitionEligible: boolean) {
  // If this commit is eligible for a View Transition we look into all mutated subtrees.
  // TODO: We could optimize this by marking these with the Snapshot subtree flag in the render phase.
  const subtreeMask = isViewTransitionEligible
@@ -331,7 +330,6 @@ function commitBeforeMutationEffects_begin(
          commitBeforeMutationEffectsDeletion(
            deletion,
            isViewTransitionEligible,
-            appearingViewTransitions,
          );
        }
      }
@@ -364,7 +362,7 @@ function commitBeforeMutationEffects_begin(
            isViewTransitionEligible
          ) {
            // Was previously mounted as visible but is now hidden.
-            commitExitViewTransitions(current, appearingViewTransitions);
+            commitExitViewTransitions(current);
          }
          // Skip before mutation effects of the children because they're hidden.
          commitBeforeMutationEffects_complete(isViewTransitionEligible);
@@ -528,7 +526,6 @@ function commitBeforeMutationEffectsOnFiber(
 function commitBeforeMutationEffectsDeletion(
  deletion: Fiber,
  isViewTransitionEligible: boolean,
-  appearingViewTransitions: Map<string, ViewTransitionState> | null,
 ) {
  if (enableCreateEventHandleAPI) {
    // TODO (effects) It would be nice to avoid calling doesFiberContain()
@@ -541,7 +538,7 @@ function commitBeforeMutationEffectsDeletion(
    }
  }
  if (isViewTransitionEligible) {
-    commitExitViewTransitions(deletion, appearingViewTransitions);
+    commitExitViewTransitions(deletion);
  }
 }

@@ -745,14 +742,15 @@ function commitEnterViewTransitions(placement: Fiber): void {
  }
 }

-function commitDeletedPairViewTransitions(
-  deletion: Fiber,
-  appearingViewTransitions: Map<string, ViewTransitionState>,
-): void {
-  if (appearingViewTransitions.size === 0) {
+function commitDeletedPairViewTransitions(deletion: Fiber): void {
+  if (
+    appearingViewTransitions === null ||
+    appearingViewTransitions.size === 0
+  ) {
    // We've found all.
    return;
  }
+  const pairs = appearingViewTransitions;
  if ((deletion.subtreeFlags & ViewTransitionNamedStatic) === NoFlags) {
    // This has no named view transitions in its subtree.
    return;
@@ -769,7 +767,7 @@ function commitDeletedPairViewTransitions(
        const props: ViewTransitionProps = child.memoizedProps;
        const name = props.name;
        if (name != null && name !== 'auto') {
-          const pair = appearingViewTransitions.get(name);
+          const pair = pairs.get(name);
          if (pair !== undefined) {
            const className: ?string = getViewTransitionClassName(
              props.className,
@@ -802,23 +800,20 @@ function commitDeletedPairViewTransitions(
            }
            // Delete the entry so that we know when we've found all of them
            // and can stop searching (size reaches zero).
-            appearingViewTransitions.delete(name);
-            if (appearingViewTransitions.size === 0) {
+            pairs.delete(name);
+            if (pairs.size === 0) {
              break;
            }
          }
        }
      }
-      commitDeletedPairViewTransitions(child, appearingViewTransitions);
+      commitDeletedPairViewTransitions(child);
    }
    child = child.sibling;
  }
 }

-function commitExitViewTransitions(
-  deletion: Fiber,
-  appearingViewTransitions: Map<string, ViewTransitionState> | null,
-): void {
+function commitExitViewTransitions(deletion: Fiber): void {
  if (deletion.tag === ViewTransitionComponent) {
    const props: ViewTransitionProps = deletion.memoizedProps;
    const name = getViewTransitionName(props, deletion.stateNode);
@@ -863,17 +858,17 @@ function commitExitViewTransitions(
    }
    if (appearingViewTransitions !== null) {
      // Look for more pairs deeper in the tree.
-      commitDeletedPairViewTransitions(deletion, appearingViewTransitions);
+      commitDeletedPairViewTransitions(deletion);
    }
  } else if ((deletion.subtreeFlags & ViewTransitionStatic) !== NoFlags) {
    let child = deletion.child;
    while (child !== null) {
-      commitExitViewTransitions(child, appearingViewTransitions);
+      commitExitViewTransitions(child);
      child = child.sibling;
    }
  } else {
    if (appearingViewTransitions !== null) {
-      commitDeletedPairViewTransitions(deletion, appearingViewTransitions);
+      commitDeletedPairViewTransitions(deletion);
    }
  }
 }
@@ -4813,8 +4808,13 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
 // already in the "current" tree. Because their visibility has changed, the
 // browser may not have prerendered them yet. So we check the MaySuspendCommit
 // flag instead.
+//
+// Note that MaySuspendCommit and ShouldSuspendCommit also includes named
+// ViewTransitions so that we know to also visit those to collect appearing
+// pairs.
 let suspenseyCommitFlag = ShouldSuspendCommit;
 export function accumulateSuspenseyCommit(finishedWork: Fiber): void {
+  appearingViewTransitions = null;
  accumulateSuspenseyCommitOnFiber(finishedWork);
 }

@@ -4893,6 +4893,29 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) {
      }
      break;
    }
+    case ViewTransitionComponent: {
+      if (enableViewTransition) {
+        if ((fiber.flags & suspenseyCommitFlag) !== NoFlags) {
+          const props: ViewTransitionProps = fiber.memoizedProps;
+          const name: ?string | 'auto' = props.name;
+          if (name != null && name !== 'auto') {
+            // This is a named ViewTransition being mounted or reappearing. Let's add it to
+            // the map so we can match it with deletions later.
+            if (appearingViewTransitions === null) {
+              appearingViewTransitions = new Map();
+            }
+            // Reset the pair in case we didn't end up restoring the instance in previous commits.
+            // This shouldn't really happen anymore but just in case. We could maybe add an invariant.
+            const instance: ViewTransitionState = fiber.stateNode;
+            instance.paired = null;
+            appearingViewTransitions.set(name, instance);
+          }
+        }
+        recursivelyAccumulateSuspenseyCommit(fiber);
+        break;
+      }
+      // Fallthrough
+    }
    default: {
      recursivelyAccumulateSuspenseyCommit(fiber);
    }
@@ -28,10 +28,6 @@ import type {
  OffscreenState,
  OffscreenQueue,
 } from './ReactFiberActivityComponent';
-import type {
-  ViewTransitionProps,
-  ViewTransitionState,
-} from './ReactFiberViewTransitionComponent';
 import {isOffscreenManual} from './ReactFiberActivityComponent';
 import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent';
 import type {Cache} from './ReactFiberCacheComponent';
@@ -99,7 +95,6 @@ import {
  ShouldSuspendCommit,
  Cloned,
  ViewTransitionStatic,
-  ViewTransitionNamedStatic,
 } from './ReactFiberFlags';

 import {
@@ -164,7 +159,6 @@ import {
  getWorkInProgressTransitions,
  shouldRemainOnPreviousScreen,
  markSpawnedRetryLane,
-  trackAppearingViewTransition,
 } from './ReactFiberWorkLoop';
 import {
  OffscreenLane,
@@ -947,34 +941,6 @@ function completeDehydratedSuspenseBoundary(
  }
 }

-function trackReappearingViewTransitions(workInProgress: Fiber): void {
-  if ((workInProgress.subtreeFlags & ViewTransitionNamedStatic) === NoFlags) {
-    // This has no named view transitions in its subtree.
-    return;
-  }
-  // This needs to search for any explicitly named reappearing View Transitions,
-  // whether they were updated in this transition or unchanged from before.
-  let child = workInProgress.child;
-  while (child !== null) {
-    if (child.tag === OffscreenComponent && child.memoizedState === null) {
-      // This tree is currently hidden so we skip it.
-    } else {
-      if (
-        child.tag === ViewTransitionComponent &&
-        (child.flags & ViewTransitionNamedStatic) !== NoFlags
-      ) {
-        const props: ViewTransitionProps = child.memoizedProps;
-        if (props.name != null && props.name !== 'auto') {
-          const instance: ViewTransitionState = child.stateNode;
-          trackAppearingViewTransition(instance, props.name);
-        }
-      }
-      trackReappearingViewTransitions(child);
-    }
-    child = child.sibling;
-  }
-}
-
 function completeWork(
  current: Fiber | null,
  workInProgress: Fiber,
@@ -1796,14 +1762,6 @@ function completeWork(
          const prevIsHidden = prevState !== null;
          if (prevIsHidden !== nextIsHidden) {
            workInProgress.flags |= Visibility;
-            if (enableViewTransition && !nextIsHidden) {
-              // If we're revealing a new tree, we need to find any named
-              // ViewTransitions inside it that might have a deleted pair.
-              // We do this in the complete phase in case the tree has
-              // changed during the reveal but we have to do it before we
-              // find the first deleted pair in the before mutation phase.
-              trackReappearingViewTransitions(workInProgress);
-            }
          }
        } else {
          // On initial mount, we only need a Visibility effect if the tree
@@ -44,6 +44,7 @@ export const StoreConsistency = /*             */ 0b0000000000000000100000000000
 // possible, because we're about to run out of bits.
 export const ScheduleRetry = StoreConsistency;
 export const ShouldSuspendCommit = Visibility;
+export const ViewTransitionNamedMount = ShouldSuspendCommit;
 export const DidDefer = ContentReset;
 export const FormReset = Snapshot;
 export const AffectedParentLayout = ContentReset;
@@ -74,8 +75,10 @@ export const PassiveStatic = /*                */ 0b0000000100000000000000000000
 export const MaySuspendCommit = /*             */ 0b0000001000000000000000000000000;
 // ViewTransitionNamedStatic tracks explicitly name ViewTransition components deeply
 // that might need to be visited during clean up. This is similar to SnapshotStatic
-// if there was any other use for it.
-export const ViewTransitionNamedStatic = /*    */ SnapshotStatic;
+// if there was any other use for it. It also needs to run in the same phase as
+// MaySuspendCommit tracking.
+export const ViewTransitionNamedStatic =
+  /*    */ SnapshotStatic | MaySuspendCommit;
 // ViewTransitionStatic tracks whether there are an ViewTransition components from
 // the nearest HostComponent down. It resets at every HostComponent level.
 export const ViewTransitionStatic = /*         */ 0b0000010000000000000000000000000;
@@ -432,12 +432,6 @@ let workInProgressRootConcurrentErrors: Array<CapturedValue<mixed>> | null =
 // We will log them once the tree commits.
 let workInProgressRootRecoverableErrors: Array<CapturedValue<mixed>> | null =
  null;
-// This tracks named ViewTransition components that might need to find deleted
-// pairs in the snapshot phase.
-let workInProgressAppearingViewTransitions: Map<
-  string,
-  ViewTransitionState,
-> | null = null;

 // Tracks when an update occurs during the render phase.
 let workInProgressRootDidIncludeRecursiveRenderUpdate: boolean = false;
@@ -1318,7 +1312,6 @@ function finishConcurrentRender(
      lanes,
      workInProgressRootRecoverableErrors,
      workInProgressTransitions,
-      workInProgressAppearingViewTransitions,
      workInProgressRootDidIncludeRecursiveRenderUpdate,
      workInProgressDeferredLane,
      workInProgressRootInterleavedUpdatedLanes,
@@ -1368,7 +1361,6 @@ function finishConcurrentRender(
            finishedWork,
            workInProgressRootRecoverableErrors,
            workInProgressTransitions,
-            workInProgressAppearingViewTransitions,
            workInProgressRootDidIncludeRecursiveRenderUpdate,
            lanes,
            workInProgressDeferredLane,
@@ -1390,7 +1382,6 @@ function finishConcurrentRender(
      finishedWork,
      workInProgressRootRecoverableErrors,
      workInProgressTransitions,
-      workInProgressAppearingViewTransitions,
      workInProgressRootDidIncludeRecursiveRenderUpdate,
      lanes,
      workInProgressDeferredLane,
@@ -1410,7 +1401,6 @@ function commitRootWhenReady(
  finishedWork: Fiber,
  recoverableErrors: Array<CapturedValue<mixed>> | null,
  transitions: Array<Transition> | null,
-  appearingViewTransitions: Map<string, ViewTransitionState> | null,
  didIncludeRenderPhaseUpdate: boolean,
  lanes: Lanes,
  spawnedLane: Lane,
@@ -1442,9 +1432,9 @@ function commitRootWhenReady(
    // the suspensey resources. The renderer is responsible for accumulating
    // all the load events. This all happens in a single synchronous
    // transaction, so it track state in its own module scope.
-    if (maySuspendCommit) {
-      accumulateSuspenseyCommit(finishedWork);
-    }
+    // This will also track any newly added or appearing ViewTransition
+    // components for the purposes of forming pairs.
+    accumulateSuspenseyCommit(finishedWork);
    if (isViewTransitionEligible) {
      suspendOnActiveViewTransition(root.containerInfo);
    }
@@ -1468,7 +1458,6 @@ function commitRootWhenReady(
          lanes,
          recoverableErrors,
          transitions,
-          appearingViewTransitions,
          didIncludeRenderPhaseUpdate,
          spawnedLane,
          updatedLanes,
@@ -1492,7 +1481,6 @@ function commitRootWhenReady(
    lanes,
    recoverableErrors,
    transitions,
-    appearingViewTransitions,
    didIncludeRenderPhaseUpdate,
    spawnedLane,
    updatedLanes,
@@ -1962,7 +1950,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
  workInProgressRootConcurrentErrors = null;
  workInProgressRootRecoverableErrors = null;
  workInProgressRootDidIncludeRecursiveRenderUpdate = false;
-  workInProgressAppearingViewTransitions = null;

  // Get the lanes that are entangled with whatever we're about to render. We
  // track these separately so we can distinguish the priority of the render
@@ -2302,25 +2289,6 @@ export function renderHasNotSuspendedYet(): boolean {
  return workInProgressRootExitStatus === RootInProgress;
 }

-export function trackAppearingViewTransition(
-  instance: ViewTransitionState,
-  name: string,
-): void {
-  if (workInProgressAppearingViewTransitions === null) {
-    if (
-      !includesOnlyViewTransitionEligibleLanes(workInProgressRootRenderLanes)
-    ) {
-      return;
-    }
-    workInProgressAppearingViewTransitions = new Map();
-  }
-  // Reset the pair in case we didn't end up restoring the instance in previous commits.
-  // This could happen since we don't actually commit all tracked instances if they end
-  // up in a non-committed subtree.
-  instance.paired = null;
-  workInProgressAppearingViewTransitions.set(name, instance);
-}
-
 // TODO: Over time, this function and renderRootConcurrent have become more
 // and more similar. Not sure it makes sense to maintain forked paths. Consider
 // unifying them again.
@@ -3230,7 +3198,6 @@ function commitRoot(
  lanes: Lanes,
  recoverableErrors: null | Array<CapturedValue<mixed>>,
  transitions: Array<Transition> | null,
-  appearingViewTransitions: Map<string, ViewTransitionState> | null,
  didIncludeRenderPhaseUpdate: boolean,
  spawnedLane: Lane,
  updatedLanes: Lanes,
@@ -3460,12 +3427,7 @@ function commitRoot(
      // The first phase a "before mutation" phase. We use this phase to read the
      // state of the host tree right before we mutate it. This is where
      // getSnapshotBeforeUpdate is called.
-      commitBeforeMutationEffects(
-        root,
-        finishedWork,
-        lanes,
-        appearingViewTransitions,
-      );
+      commitBeforeMutationEffects(root, finishedWork, lanes);
    } finally {
      // Reset the priority to the previous non-sync value.
      executionContext = prevExecutionContext;