Skip to content

Commit fd9cfa4

Browse files
authored
Execute layout phase before after mutation phase inside view transition (facebook#32029)
This allows mutations and scrolling in the layout phase to be counted towards the mutation. This would maybe not be the case for gestures but it is useful for fire-and-forget. This also avoids the issue that if you resolve navigation in useLayoutEffect that it ends up dead locked. It also means that useLayoutEffect does not observe the scroll restoration and in fact, the scroll restoration would win over any manual scrolling in layout effects. For better or worse, this is more in line with how things worked before and how it works in popstate. So it's less of a breaking change. This does mean that we can't unify the after mutation phase with the layout phase though. To do this we need split out flushSpawnedWork from the flushLayoutEffect call. Spawned work from setState inside the layout phase is done outside and not counted towards the transition. They're sync updates and so are not eligible for their own View Transitions. It's also tricky to support this since it's unclear what things like exits in that update would mean. This work will still be able to mutate the live DOM but it's just not eligible to trigger new transitions or adjust the target of those. One difference between popstate is that this spawned work is after scroll restoration. So any scrolling spawned from a second pass would now win over scroll restoration. Another consequence of this change is that you can't safely animate pseudo elements in useLayoutEffect. We'll introduce a better event for that anyway.
1 parent 800c9db commit fd9cfa4

File tree

5 files changed

+52
-32
lines changed

5 files changed

+52
-32
lines changed

fixtures/view-transition/src/components/App.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React, {
22
startTransition,
3-
useInsertionEffect,
3+
useLayoutEffect,
44
useEffect,
55
useState,
66
} from 'react';
@@ -68,7 +68,7 @@ export default function App({assets, initialURL}) {
6868
}
6969
}, []);
7070
const pendingNav = routerState.pendingNav;
71-
useInsertionEffect(() => {
71+
useLayoutEffect(() => {
7272
pendingNav();
7373
}, [pendingNav]);
7474
return (

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

+11-6
Original file line numberDiff line numberDiff line change
@@ -1201,8 +1201,9 @@ export function hasInstanceAffectedParent(
12011201
export function startViewTransition(
12021202
rootContainer: Container,
12031203
mutationCallback: () => void,
1204-
afterMutationCallback: () => void,
12051204
layoutCallback: () => void,
1205+
afterMutationCallback: () => void,
1206+
spawnedWorkCallback: () => void,
12061207
passiveCallback: () => mixed,
12071208
): boolean {
12081209
const ownerDocument: Document =
@@ -1213,11 +1214,15 @@ export function startViewTransition(
12131214
// $FlowFixMe[prop-missing]
12141215
const transition = ownerDocument.startViewTransition({
12151216
update() {
1216-
mutationCallback();
1217-
// TODO: Wait for fonts.
1217+
// Note: We read the existence of a pending navigation before we apply the
1218+
// mutations. That way we're not waiting on a navigation that we spawned
1219+
// from this update. Only navigations that started before this commit.
12181220
const ownerWindow = ownerDocument.defaultView;
12191221
const pendingNavigation =
12201222
ownerWindow.navigation && ownerWindow.navigation.transition;
1223+
mutationCallback();
1224+
// TODO: Wait for fonts.
1225+
layoutCallback();
12211226
if (pendingNavigation) {
12221227
return pendingNavigation.finished.then(
12231228
afterMutationCallback,
@@ -1241,13 +1246,13 @@ export function startViewTransition(
12411246
console.error(
12421247
'A ViewTransition timed out because a Navigation stalled. ' +
12431248
'This can happen if a Navigation is blocked on React itself. ' +
1244-
"Such as if it's resolved inside useLayoutEffect. " +
1245-
'This can be solved by moving the resolution to useInsertionEffect.',
1249+
"Such as if it's resolved inside useEffect. " +
1250+
'This can be solved by moving the resolution to useLayoutEffect.',
12461251
);
12471252
}
12481253
});
12491254
}
1250-
transition.ready.then(layoutCallback, layoutCallback);
1255+
transition.ready.then(spawnedWorkCallback, spawnedWorkCallback);
12511256
transition.finished.then(() => {
12521257
// $FlowFixMe[prop-missing]
12531258
ownerDocument.__reactViewTransition = null;

packages/react-native-renderer/src/ReactFiberConfigNative.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -583,8 +583,9 @@ export function hasInstanceAffectedParent(
583583
export function startViewTransition(
584584
rootContainer: Container,
585585
mutationCallback: () => void,
586-
afterMutationCallback: () => void,
587586
layoutCallback: () => void,
587+
afterMutationCallback: () => void,
588+
spawnedWorkCallback: () => void,
588589
passiveCallback: () => mixed,
589590
): boolean {
590591
return false;

packages/react-reconciler/src/ReactFiberWorkLoop.js

+35-22
Original file line numberDiff line numberDiff line change
@@ -637,10 +637,11 @@ const THROTTLED_COMMIT = 2;
637637

638638
const NO_PENDING_EFFECTS = 0;
639639
const PENDING_MUTATION_PHASE = 1;
640-
const PENDING_AFTER_MUTATION_PHASE = 2;
641-
const PENDING_LAYOUT_PHASE = 3;
642-
const PENDING_PASSIVE_PHASE = 4;
643-
let pendingEffectsStatus: 0 | 1 | 2 | 3 | 4 = 0;
640+
const PENDING_LAYOUT_PHASE = 2;
641+
const PENDING_AFTER_MUTATION_PHASE = 3;
642+
const PENDING_SPAWNED_WORK = 4;
643+
const PENDING_PASSIVE_PHASE = 5;
644+
let pendingEffectsStatus: 0 | 1 | 2 | 3 | 4 | 5 = 0;
644645
let pendingEffectsRoot: FiberRoot = (null: any);
645646
let pendingFinishedWork: Fiber = (null: any);
646647
let pendingEffectsLanes: Lanes = NoLanes;
@@ -3432,19 +3433,17 @@ function commitRoot(
34323433
startViewTransition(
34333434
root.containerInfo,
34343435
flushMutationEffects,
3435-
flushAfterMutationEffects,
34363436
flushLayoutEffects,
3437-
// TODO: This flushes passive effects at the end of the transition but
3438-
// we also schedule work to flush them separately which we really shouldn't.
3439-
// We use flushPendingEffects instead of
3437+
flushAfterMutationEffects,
3438+
flushSpawnedWork,
34403439
flushPassiveEffects,
34413440
);
34423441
if (!startedViewTransition) {
34433442
// Flush synchronously.
34443443
flushMutationEffects();
3445-
// Skip flushAfterMutationEffects
3446-
pendingEffectsStatus = PENDING_LAYOUT_PHASE;
34473444
flushLayoutEffects();
3445+
// Skip flushAfterMutationEffects
3446+
flushSpawnedWork();
34483447
}
34493448
}
34503449

@@ -3457,7 +3456,7 @@ function flushAfterMutationEffects(): void {
34573456
const finishedWork = pendingFinishedWork;
34583457
const lanes = pendingEffectsLanes;
34593458
commitAfterMutationEffects(root, finishedWork, lanes);
3460-
pendingEffectsStatus = PENDING_LAYOUT_PHASE;
3459+
pendingEffectsStatus = PENDING_SPAWNED_WORK;
34613460
}
34623461

34633462
function flushMutationEffects(): void {
@@ -3503,27 +3502,18 @@ function flushMutationEffects(): void {
35033502
// componentWillUnmount, but before the layout phase, so that the finished
35043503
// work is current during componentDidMount/Update.
35053504
root.current = finishedWork;
3506-
pendingEffectsStatus = PENDING_AFTER_MUTATION_PHASE;
3505+
pendingEffectsStatus = PENDING_LAYOUT_PHASE;
35073506
}
35083507

35093508
function flushLayoutEffects(): void {
3510-
if (
3511-
pendingEffectsStatus !== PENDING_LAYOUT_PHASE &&
3512-
// If a startViewTransition times out, we might flush this earlier than
3513-
// after mutation phase. In that case, we just skip the after mutation phase.
3514-
pendingEffectsStatus !== PENDING_AFTER_MUTATION_PHASE
3515-
) {
3509+
if (pendingEffectsStatus !== PENDING_LAYOUT_PHASE) {
35163510
return;
35173511
}
35183512
pendingEffectsStatus = NO_PENDING_EFFECTS;
35193513

35203514
const root = pendingEffectsRoot;
35213515
const finishedWork = pendingFinishedWork;
35223516
const lanes = pendingEffectsLanes;
3523-
const completedRenderEndTime = pendingEffectsRenderEndTime;
3524-
const recoverableErrors = pendingRecoverableErrors;
3525-
const didIncludeRenderPhaseUpdate = pendingDidIncludeRenderPhaseUpdate;
3526-
const suspendedCommitReason = pendingSuspendedCommitReason;
35273517

35283518
const subtreeHasLayoutEffects =
35293519
(finishedWork.subtreeFlags & LayoutMask) !== NoFlags;
@@ -3554,11 +3544,32 @@ function flushLayoutEffects(): void {
35543544
ReactSharedInternals.T = prevTransition;
35553545
}
35563546
}
3547+
pendingEffectsStatus = PENDING_AFTER_MUTATION_PHASE;
3548+
}
3549+
3550+
function flushSpawnedWork(): void {
3551+
if (
3552+
pendingEffectsStatus !== PENDING_SPAWNED_WORK &&
3553+
// If a startViewTransition times out, we might flush this earlier than
3554+
// after mutation phase. In that case, we just skip the after mutation phase.
3555+
pendingEffectsStatus !== PENDING_AFTER_MUTATION_PHASE
3556+
) {
3557+
return;
3558+
}
3559+
pendingEffectsStatus = NO_PENDING_EFFECTS;
35573560

35583561
// Tell Scheduler to yield at the end of the frame, so the browser has an
35593562
// opportunity to paint.
35603563
requestPaint();
35613564

3565+
const root = pendingEffectsRoot;
3566+
const finishedWork = pendingFinishedWork;
3567+
const lanes = pendingEffectsLanes;
3568+
const completedRenderEndTime = pendingEffectsRenderEndTime;
3569+
const recoverableErrors = pendingRecoverableErrors;
3570+
const didIncludeRenderPhaseUpdate = pendingDidIncludeRenderPhaseUpdate;
3571+
const suspendedCommitReason = pendingSuspendedCommitReason;
3572+
35623573
if (enableProfilerTimer && enableComponentPerformanceTrack) {
35633574
recordCommitEndTime();
35643575
logCommitPhase(
@@ -3795,6 +3806,8 @@ export function flushPendingEffects(wasDelayedCommit?: boolean): boolean {
37953806
// Returns whether passive effects were flushed.
37963807
flushMutationEffects();
37973808
flushLayoutEffects();
3809+
// Skip flushAfterMutation if we're forcing this early.
3810+
flushSpawnedWork();
37983811
return flushPassiveEffects(wasDelayedCommit);
37993812
}
38003813

packages/react-test-renderer/src/ReactFiberConfigTestHost.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,9 @@ export function hasInstanceAffectedParent(
365365
export function startViewTransition(
366366
rootContainer: Container,
367367
mutationCallback: () => void,
368-
afterMutationCallback: () => void,
369368
layoutCallback: () => void,
369+
afterMutationCallback: () => void,
370+
spawnedWorkCallback: () => void,
370371
passiveCallback: () => mixed,
371372
): boolean {
372373
return false;

0 commit comments

Comments
 (0)