Skip to content

Commit d343c39

Browse files
authored
Remove Gesture warning when cloning the root (facebook#35566)
Currently we always clone the root when a gesture transition happens. The was to add an optimization where if a Transition could be isolated to an absolutely positioned subtree then we could just clone that subtree or just do a plain insertion if it was simple an Enter. That way when switching between two absolutely positioned pages the shell wouldn't need to be cloned. In that case `detectMutationOrInsertClones` would return false. However, currently it always return true because we don't yet have that optimization. The idea was to warn when the root required cloning to ensure that you optimize it intentionally since it's easy to accidentally update more than necessary. However, since this is not yet actionable I'm removing this warning for now. Instead, I add a warning for particularly bad cases where you really shouldn't clone like iframe and video. They may not be very actionable without the optimization since you can't scope it down to a subtree without the optimization. So if they're above the gesture then they're always cloned atm. However, it might also be that it's unnecessary to keep them mounted if they could be removed or hidden with Activity.
1 parent 1ecd99c commit d343c39

File tree

2 files changed

+22
-13
lines changed

2 files changed

+22
-13
lines changed

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,10 +607,32 @@ export function createInstance(
607607
return domElement;
608608
}
609609

610+
let didWarnForClone = false;
611+
610612
export function cloneMutableInstance(
611613
instance: Instance,
612614
keepChildren: boolean,
613615
): Instance {
616+
if (__DEV__) {
617+
// Warn for problematic
618+
const tagName = instance.tagName;
619+
switch (tagName) {
620+
case 'VIDEO':
621+
case 'IFRAME':
622+
if (!didWarnForClone) {
623+
didWarnForClone = true;
624+
// TODO: Once we have the ability to avoid cloning the root, suggest an absolutely
625+
// positioned ViewTransition instead as the solution.
626+
console.warn(
627+
'startGestureTransition() required cloning a <%s> element since it exists in ' +
628+
'both states of the gesture. This can be problematic since it will load it twice ' +
629+
'Try removing or hiding it with <Activity mode="offscreen"> in the optimistic state.',
630+
tagName.toLowerCase(),
631+
);
632+
}
633+
break;
634+
}
635+
}
614636
return instance.cloneNode(keepChildren);
615637
}
616638

packages/react-reconciler/src/ReactFiberApplyGesture.js

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,6 @@ import {
9797
import {trackAnimatingTask} from './ReactProfilerTimer';
9898
import {scheduleGestureTransitionEvent} from './ReactFiberWorkLoop';
9999

100-
let didWarnForRootClone = false;
101-
102100
// Used during the apply phase to track whether a parent ViewTransition component
103101
// might have been affected by any mutations / relayouts below.
104102
let viewTransitionContextChanged: boolean = false;
@@ -1033,17 +1031,6 @@ export function insertDestinationClones(
10331031
// we cancel the root view transition name.
10341032
const needsClone = detectMutationOrInsertClones(finishedWork);
10351033
if (needsClone) {
1036-
if (__DEV__) {
1037-
if (!didWarnForRootClone) {
1038-
didWarnForRootClone = true;
1039-
console.warn(
1040-
'startGestureTransition() caused something to mutate or relayout the root. ' +
1041-
'This currently requires a clone of the whole document. Make sure to ' +
1042-
'add a <ViewTransition> directly around an absolutely positioned DOM node ' +
1043-
'to minimize the impact of any changes caused by the Gesture Transition.',
1044-
);
1045-
}
1046-
}
10471034
// Clone the whole root
10481035
const rootClone = cloneRootViewTransitionContainer(root.containerInfo);
10491036
root.gestureClone = rootClone;

0 commit comments

Comments
 (0)