Skip to content

Mark the root as animating if any Portal mutates or resizes #32772

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 1 commit into from
Mar 31, 2025
+54 −1
Merged

Mark the root as animating if any Portal mutates or resizes #32772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions fixtures/view-transition/src/components/Page.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,11 @@
border: 0px;
border-radius: 5px;
padding: 10px;
}

.portal {
position: fixed;
top: 10px;
left: 360px;
border: 1px solid #ccc;
}
20 changes: 20 additions & 0 deletions fixtures/view-transition/src/components/Page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import React, {
useEffect,
useState,
useId,
startTransition,
} from 'react';
import {createPortal} from 'react-dom';

import SwipeRecognizer from './SwipeRecognizer';

Expand Down Expand Up @@ -79,6 +81,23 @@ export default function Page({url, navigate}) {
// });
}, [show]);

const [showModal, setShowModal] = useState(false);
const portal = showModal ? (
createPortal(
<div className="portal">
Portal: {!show ? 'A' : 'B'}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, this would've been thought of as React as being inside the <ViewTransition> in the page and so it would've considered not leaking to the root and so the root animation gets cancelled. But since this is in the root it now doesn't animate this transition. Now we're more pessimistic about that.

By wrapping this <div> in <ViewTransition> the root no longer needs to animate because the mutation is contained within this ViewTransition.

<ViewTransition>
<div>{!show ? 'A' : 'B'}</div>
</ViewTransition>
</div>,
document.body
)
) : (
<button onClick={() => startTransition(() => setShowModal(true))}>
Show Modal
</button>
);

const exclamation = (
<ViewTransition name="exclamation" onShare={onTransition}>
<span>!</span>
Expand Down Expand Up @@ -153,6 +172,7 @@ export default function Page({url, navigate}) {
<p>content</p>
<p>out</p>
<p>of</p>
{portal}
<p>the</p>
<p>viewport</p>
{show ? <Component /> : null}
Expand Down
28 changes: 27 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ export let shouldFireAfterActiveInstanceBlur: boolean = false;
// 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;
let rootViewTransitionAffected: boolean = false;

export function commitBeforeMutationEffects(
root: FiberRoot,
Expand Down Expand Up @@ -1750,6 +1751,8 @@ export function commitMutationEffects(
inProgressLanes = committedLanes;
inProgressRoot = root;

rootViewTransitionAffected = false;

resetComponentEffectTimers();

commitMutationEffectsOnFiber(finishedWork, root, committedLanes);
Expand Down Expand Up @@ -2068,6 +2071,7 @@ function commitMutationEffectsOnFiber(
break;
}
case HostPortal: {
const prevMutationContext = pushMutationContext();
if (supportsResources) {
const previousHoistableRoot = currentHoistableRoot;
currentHoistableRoot = getHoistableRoot(
Expand All @@ -2080,6 +2084,14 @@ function commitMutationEffectsOnFiber(
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork, lanes);
}
if (viewTransitionMutationContext) {
// A Portal doesn't necessarily exist within the context of this subtree.
// Ideally we would track which React ViewTransition component nests the container
// but that's costly. Instead, we treat each Portal as if it's a new React root.
// Therefore any leaked mutation means that the root should animate.
rootViewTransitionAffected = true;
}
popMutationContext(prevMutationContext);

if (flags & Update) {
if (supportsPersistence) {
Expand Down Expand Up @@ -2432,7 +2444,7 @@ function commitAfterMutationEffectsOnFiber(
viewTransitionContextChanged = false;
pushViewTransitionCancelableScope();
recursivelyTraverseAfterMutationEffects(root, finishedWork, lanes);
if (!viewTransitionContextChanged) {
if (!viewTransitionContextChanged && !rootViewTransitionAffected) {
// If we didn't leak any resizing out to the root, we don't have to transition
// the root itself. This means that we can now safely cancel any cancellations
// that bubbled all the way up.
Expand All @@ -2456,6 +2468,20 @@ function commitAfterMutationEffectsOnFiber(
recursivelyTraverseAfterMutationEffects(root, finishedWork, lanes);
break;
}
case HostPortal: {
const prevContextChanged = viewTransitionContextChanged;
viewTransitionContextChanged = false;
recursivelyTraverseAfterMutationEffects(root, finishedWork, lanes);
if (viewTransitionContextChanged) {
// A Portal doesn't necessarily exist within the context of this subtree.
// Ideally we would track which React ViewTransition component nests the container
// but that's costly. Instead, we treat each Portal as if it's a new React root.
// Therefore any leaked resize of a child could affect the root so the root should animate.
rootViewTransitionAffected = true;
}
viewTransitionContextChanged = prevContextChanged;
break;
}
case OffscreenComponent: {
const isModernRoot =
disableLegacyMode || (finishedWork.mode & ConcurrentMode) !== NoMode;
Expand Down
Loading