Skip to content

Remove Mutation Check Around commit/measureUpdateViewTransition #32617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 14, 2025
Merged

Remove Mutation Check Around commit/measureUpdateViewTransition #32617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf2cfea
Clarify that the BeforeMutationTransitionMask is also used in the Aft…
sebmarkbage Mar 14, 2025
12fbbd9
Remove subtreeFlag check for the Update path
sebmarkbage Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 47 additions & 61 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ import {
Hydrating,
Passive,
BeforeMutationMask,
BeforeMutationTransitionMask,
BeforeAndAfterMutationTransitionMask,
MutationMask,
LayoutMask,
PassiveMask,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -2440,7 +2432,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);
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
Loading