From bf2cfea2db14742367e14e839b405dea44062690 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 14 Mar 2025 13:06:34 -0400 Subject: [PATCH 1/2] Clarify that the BeforeMutationTransitionMask is also used in the AfterMutation pass Also add ContentReset which is also a type of mutation but it's usually accompanied by an Update too. --- packages/react-reconciler/src/ReactFiberCommitWork.js | 6 +++--- packages/react-reconciler/src/ReactFiberFlags.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 2663c1e087542..a49e76ef9455b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -100,7 +100,7 @@ import { Hydrating, Passive, BeforeMutationMask, - BeforeMutationTransitionMask, + BeforeAndAfterMutationTransitionMask, MutationMask, LayoutMask, PassiveMask, @@ -311,7 +311,7 @@ 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 - ? BeforeMutationTransitionMask + ? BeforeAndAfterMutationTransitionMask : BeforeMutationMask; while (nextEffect !== null) { const fiber = nextEffect; @@ -2440,7 +2440,7 @@ function recursivelyTraverseAfterMutationEffects( lanes: Lanes, ) { // We need to visit the same nodes that we visited in the before mutation phase. - if (parentFiber.subtreeFlags & BeforeMutationTransitionMask) { + if (parentFiber.subtreeFlags & BeforeAndAfterMutationTransitionMask) { let child = parentFiber.child; while (child !== null) { commitAfterMutationEffectsOnFiber(child, root, lanes); diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index b6b2efd1996a8..8b26c66859363 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -108,8 +108,8 @@ export const BeforeMutationMask: number = // For View Transition support we use the snapshot phase to scan the tree for potentially // affected ViewTransition components. -export const BeforeMutationTransitionMask: number = - Snapshot | Update | Placement | ChildDeletion | Visibility; +export const BeforeAndAfterMutationTransitionMask: number = + Snapshot | Update | Placement | ChildDeletion | Visibility | ContentReset; export const MutationMask = Placement | From 12fbbd9bd1ccd722177b8c894578d11f84851ef6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 14 Mar 2025 13:49:34 -0400 Subject: [PATCH 2/2] Remove subtreeFlag check for the Update path --- .../src/ReactFiberCommitWork.js | 102 ++++++++---------- 1 file changed, 44 insertions(+), 58 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index a49e76ef9455b..579dd21fb46c6 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -487,16 +487,8 @@ function commitBeforeMutationEffectsOnFiber( if (current === null) { // This is a new mount. We should have handled this as part of the // Placement effect or it is deeper inside a entering transition. - } else if ( - (finishedWork.subtreeFlags & - (Placement | - Update | - ChildDeletion | - ContentReset | - Visibility)) !== - NoFlags - ) { - // Something mutated within this subtree. This might need to cause + } else { + // Something may have mutated within this subtree. This might need to cause // a cross-fade of this parent. We first assign old names to the // previous tree in the before mutation phase in case we need to. // TODO: This walks the tree that we might continue walking anyway. @@ -2525,63 +2517,57 @@ function commitAfterMutationEffectsOnFiber( break; } case ViewTransitionComponent: { - if ( - (finishedWork.subtreeFlags & - (Placement | Update | ChildDeletion | ContentReset | Visibility)) !== - NoFlags - ) { - const wasMutated = (finishedWork.flags & Update) !== NoFlags; + const wasMutated = (finishedWork.flags & Update) !== NoFlags; - const prevContextChanged = viewTransitionContextChanged; - const prevCancelableChildren = pushViewTransitionCancelableScope(); - viewTransitionContextChanged = false; - recursivelyTraverseAfterMutationEffects(root, finishedWork, lanes); + const prevContextChanged = viewTransitionContextChanged; + const prevCancelableChildren = pushViewTransitionCancelableScope(); + viewTransitionContextChanged = false; + recursivelyTraverseAfterMutationEffects(root, finishedWork, lanes); - if (viewTransitionContextChanged) { - finishedWork.flags |= Update; - } + if (viewTransitionContextChanged) { + finishedWork.flags |= Update; + } - const inViewport = measureUpdateViewTransition(current, finishedWork); + const inViewport = measureUpdateViewTransition(current, finishedWork); - if ((finishedWork.flags & Update) === NoFlags || !inViewport) { - // If this boundary didn't update, then we may be able to cancel its children. - // We bubble them up to the parent set to be determined later if we can cancel. - // Similarly, if old and new state was outside the viewport, we can skip it - // even if it did update. - if (prevCancelableChildren === null) { - // Bubbling up this whole set to the parent. - } else { - // Merge with parent set. - // $FlowFixMe[method-unbinding] - prevCancelableChildren.push.apply( - prevCancelableChildren, - viewTransitionCancelableChildren, - ); - popViewTransitionCancelableScope(prevCancelableChildren); - } - // TODO: If this doesn't end up canceled, because a parent animates, - // then we should probably issue an event since this instance is part of it. + if ((finishedWork.flags & Update) === NoFlags || !inViewport) { + // If this boundary didn't update, then we may be able to cancel its children. + // We bubble them up to the parent set to be determined later if we can cancel. + // Similarly, if old and new state was outside the viewport, we can skip it + // even if it did update. + if (prevCancelableChildren === null) { + // Bubbling up this whole set to the parent. } else { - const props: ViewTransitionProps = finishedWork.memoizedProps; - scheduleViewTransitionEvent( - finishedWork, - wasMutated || viewTransitionContextChanged - ? props.onUpdate - : props.onLayout, + // Merge with parent set. + // $FlowFixMe[method-unbinding] + prevCancelableChildren.push.apply( + prevCancelableChildren, + viewTransitionCancelableChildren, ); - - // If this boundary did update, we cannot cancel its children so those are dropped. popViewTransitionCancelableScope(prevCancelableChildren); } + // TODO: If this doesn't end up canceled, because a parent animates, + // then we should probably issue an event since this instance is part of it. + } else { + const props: ViewTransitionProps = finishedWork.memoizedProps; + scheduleViewTransitionEvent( + finishedWork, + wasMutated || viewTransitionContextChanged + ? props.onUpdate + : props.onLayout, + ); - if ((finishedWork.flags & AffectedParentLayout) !== NoFlags) { - // This boundary changed size in a way that may have caused its parent to - // relayout. We need to bubble this information up to the parent. - viewTransitionContextChanged = true; - } else { - // Otherwise, we restore it to whatever the parent had found so far. - viewTransitionContextChanged = prevContextChanged; - } + // If this boundary did update, we cannot cancel its children so those are dropped. + popViewTransitionCancelableScope(prevCancelableChildren); + } + + if ((finishedWork.flags & AffectedParentLayout) !== NoFlags) { + // This boundary changed size in a way that may have caused its parent to + // relayout. We need to bubble this information up to the parent. + viewTransitionContextChanged = true; + } else { + // Otherwise, we restore it to whatever the parent had found so far. + viewTransitionContextChanged = prevContextChanged; } break; }