Skip to content

[Transition Tracing] Moved Transition Tracing Callback Code From Mutation to Passive Phase #24251

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
Apr 8, 2022
Merged

[Transition Tracing] Moved Transition Tracing Callback Code From Mutation to Passive Phase #24251

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
63 changes: 33 additions & 30 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2227,32 +2227,6 @@ function commitMutationEffectsOnFiber(
// because of the shared reconciliation logic below.
const flags = finishedWork.flags;

if (enableTransitionTracing) {
switch (finishedWork.tag) {
case HostRoot: {
const state = finishedWork.memoizedState;
const transitions = state.transitions;
if (transitions !== null) {
transitions.forEach(transition => {
// TODO(luna) Do we want to log TransitionStart in the startTransition callback instead?
addTransitionStartCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});

addTransitionCompleteCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});
});

clearTransitionsForLanes(root, lanes);
state.transitions = null;
}
}
}
}

if (flags & ContentReset) {
commitResetTextContent(finishedWork);
}
Expand Down Expand Up @@ -2639,34 +2613,41 @@ function reappearLayoutEffects_complete(subtreeRoot: Fiber) {
export function commitPassiveMountEffects(
root: FiberRoot,
finishedWork: Fiber,
committedLanes: Lanes,
): void {
nextEffect = finishedWork;
commitPassiveMountEffects_begin(finishedWork, root);
commitPassiveMountEffects_begin(finishedWork, root, committedLanes);
}

function commitPassiveMountEffects_begin(subtreeRoot: Fiber, root: FiberRoot) {
function commitPassiveMountEffects_begin(
subtreeRoot: Fiber,
root: FiberRoot,
committedLanes: Lanes,
) {
while (nextEffect !== null) {
const fiber = nextEffect;
const firstChild = fiber.child;
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && firstChild !== null) {
ensureCorrectReturnPointer(firstChild, fiber);
nextEffect = firstChild;
} else {
commitPassiveMountEffects_complete(subtreeRoot, root);
commitPassiveMountEffects_complete(subtreeRoot, root, committedLanes);
}
}
}

function commitPassiveMountEffects_complete(
subtreeRoot: Fiber,
root: FiberRoot,
committedLanes: Lanes,
) {
while (nextEffect !== null) {
const fiber = nextEffect;

if ((fiber.flags & Passive) !== NoFlags) {
setCurrentDebugFiberInDEV(fiber);
try {
commitPassiveMountOnFiber(root, fiber);
commitPassiveMountOnFiber(root, fiber, committedLanes);
} catch (error) {
reportUncaughtErrorInDEV(error);
captureCommitPhaseError(fiber, fiber.return, error);
Expand All @@ -2693,6 +2674,7 @@ function commitPassiveMountEffects_complete(
function commitPassiveMountOnFiber(
finishedRoot: FiberRoot,
finishedWork: Fiber,
committedLanes: Lanes,
): void {
switch (finishedWork.tag) {
case FunctionComponent:
Expand Down Expand Up @@ -2734,6 +2716,27 @@ function commitPassiveMountOnFiber(
}
}
}

if (enableTransitionTracing) {
const transitions = finishedWork.memoizedState.transitions;
if (transitions !== null) {
transitions.forEach(transition => {
// TODO(luna) Do we want to log TransitionStart in the startTransition callback instead?
addTransitionStartCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});

addTransitionCompleteCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});
});

clearTransitionsForLanes(finishedRoot, committedLanes);
finishedWork.memoizedState.transitions = null;
}
}
break;
}
case LegacyHiddenComponent:
Expand Down
63 changes: 33 additions & 30 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2227,32 +2227,6 @@ function commitMutationEffectsOnFiber(
// because of the shared reconciliation logic below.
const flags = finishedWork.flags;

if (enableTransitionTracing) {
switch (finishedWork.tag) {
case HostRoot: {
const state = finishedWork.memoizedState;
const transitions = state.transitions;
if (transitions !== null) {
transitions.forEach(transition => {
// TODO(luna) Do we want to log TransitionStart in the startTransition callback instead?
addTransitionStartCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});

addTransitionCompleteCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});
});

clearTransitionsForLanes(root, lanes);
state.transitions = null;
}
}
}
}

if (flags & ContentReset) {
commitResetTextContent(finishedWork);
}
Expand Down Expand Up @@ -2639,34 +2613,41 @@ function reappearLayoutEffects_complete(subtreeRoot: Fiber) {
export function commitPassiveMountEffects(
root: FiberRoot,
finishedWork: Fiber,
committedLanes: Lanes,
): void {
nextEffect = finishedWork;
commitPassiveMountEffects_begin(finishedWork, root);
commitPassiveMountEffects_begin(finishedWork, root, committedLanes);
}

function commitPassiveMountEffects_begin(subtreeRoot: Fiber, root: FiberRoot) {
function commitPassiveMountEffects_begin(
subtreeRoot: Fiber,
root: FiberRoot,
committedLanes: Lanes,
) {
while (nextEffect !== null) {
const fiber = nextEffect;
const firstChild = fiber.child;
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && firstChild !== null) {
ensureCorrectReturnPointer(firstChild, fiber);
nextEffect = firstChild;
} else {
commitPassiveMountEffects_complete(subtreeRoot, root);
commitPassiveMountEffects_complete(subtreeRoot, root, committedLanes);
}
}
}

function commitPassiveMountEffects_complete(
subtreeRoot: Fiber,
root: FiberRoot,
committedLanes: Lanes,
) {
while (nextEffect !== null) {
const fiber = nextEffect;

if ((fiber.flags & Passive) !== NoFlags) {
setCurrentDebugFiberInDEV(fiber);
try {
commitPassiveMountOnFiber(root, fiber);
commitPassiveMountOnFiber(root, fiber, committedLanes);
} catch (error) {
reportUncaughtErrorInDEV(error);
captureCommitPhaseError(fiber, fiber.return, error);
Expand All @@ -2693,6 +2674,7 @@ function commitPassiveMountEffects_complete(
function commitPassiveMountOnFiber(
finishedRoot: FiberRoot,
finishedWork: Fiber,
committedLanes: Lanes,
): void {
switch (finishedWork.tag) {
case FunctionComponent:
Expand Down Expand Up @@ -2734,6 +2716,27 @@ function commitPassiveMountOnFiber(
}
}
}

if (enableTransitionTracing) {
const transitions = finishedWork.memoizedState.transitions;
if (transitions !== null) {
transitions.forEach(transition => {
// TODO(luna) Do we want to log TransitionStart in the startTransition callback instead?
addTransitionStartCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});

addTransitionCompleteCallbackToPendingTransition({
transitionName: transition.name,
startTime: transition.startTime,
});
});

clearTransitionsForLanes(finishedRoot, committedLanes);
finishedWork.memoizedState.transitions = null;
}
}
break;
}
case LegacyHiddenComponent:
Expand Down
26 changes: 26 additions & 0 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ import {
popRenderLanes,
getRenderTargetTime,
subtreeRenderLanes,
getWorkInProgressTransitions,
} from './ReactFiberWorkLoop.new';
import {
OffscreenLane,
Expand Down Expand Up @@ -862,6 +863,17 @@ function completeWork(
}
case HostRoot: {
const fiberRoot = (workInProgress.stateNode: FiberRoot);

if (enableTransitionTracing) {
const transitions = getWorkInProgressTransitions();
Copy link
Collaborator

@acdlite acdlite Apr 7, 2022

Choose a reason for hiding this comment

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

Reminder for me: This will change in next PR

// We set the Passive flag here because if there are new transitions,
// we will need to schedule callbacks and process the transitions,
// which we do in the passive phase
if (transitions !== null) {
workInProgress.flags |= Passive;
}
}

if (enableCache) {
popRootTransition(fiberRoot, renderLanes);

Expand Down Expand Up @@ -918,6 +930,14 @@ function completeWork(
}
updateHostContainer(current, workInProgress);
bubbleProperties(workInProgress);
if (enableTransitionTracing) {
if ((workInProgress.subtreeFlags & Visibility) !== NoFlags) {
// If any of our suspense children toggle visibility, this means that
// the pending boundaries array needs to be updated, which we only
// do in the passive phase.
workInProgress.flags |= Passive;
Copy link
Collaborator

@acdlite acdlite Apr 7, 2022

Choose a reason for hiding this comment

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

What's this for?

Copy link
Contributor Author

@lunaruan lunaruan Apr 7, 2022

Choose a reason for hiding this comment

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

If a suspense boundary toggles in visibility, we should also schedule a passive effect even if there are no new transitions because this might mean we completed the interaction (or the pending boundaries array changes at least)

Copy link
Collaborator

@acdlite acdlite Apr 7, 2022

Choose a reason for hiding this comment

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

Ok makes sense, thanks for the added inline comment!

}
}
return null;
}
case HostComponent: {
Expand Down Expand Up @@ -1187,6 +1207,12 @@ function completeWork(
const offscreenFiber: Fiber = (workInProgress.child: any);
offscreenFiber.flags |= Visibility;

// If the suspended state of the boundary changes, we need to schedule
// a passive effect, which is when we process the transitions
if (enableTransitionTracing) {
offscreenFiber.flags |= Passive;
}

// TODO: This will still suspend a synchronous tree if anything
// in the concurrent tree already suspended during this render.
// This is a known bug.
Expand Down
26 changes: 26 additions & 0 deletions packages/react-reconciler/src/ReactFiberCompleteWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ import {
popRenderLanes,
getRenderTargetTime,
subtreeRenderLanes,
getWorkInProgressTransitions,
} from './ReactFiberWorkLoop.old';
import {
OffscreenLane,
Expand Down Expand Up @@ -862,6 +863,17 @@ function completeWork(
}
case HostRoot: {
const fiberRoot = (workInProgress.stateNode: FiberRoot);

if (enableTransitionTracing) {
const transitions = getWorkInProgressTransitions();
// We set the Passive flag here because if there are new transitions,
// we will need to schedule callbacks and process the transitions,
// which we do in the passive phase
if (transitions !== null) {
workInProgress.flags |= Passive;
}
}

if (enableCache) {
popRootTransition(fiberRoot, renderLanes);

Expand Down Expand Up @@ -918,6 +930,14 @@ function completeWork(
}
updateHostContainer(current, workInProgress);
bubbleProperties(workInProgress);
if (enableTransitionTracing) {
if ((workInProgress.subtreeFlags & Visibility) !== NoFlags) {
// If any of our suspense children toggle visibility, this means that
// the pending boundaries array needs to be updated, which we only
// do in the passive phase.
workInProgress.flags |= Passive;
}
}
return null;
}
case HostComponent: {
Expand Down Expand Up @@ -1187,6 +1207,12 @@ function completeWork(
const offscreenFiber: Fiber = (workInProgress.child: any);
offscreenFiber.flags |= Visibility;

// If the suspended state of the boundary changes, we need to schedule
// a passive effect, which is when we process the transitions
if (enableTransitionTracing) {
offscreenFiber.flags |= Passive;
}

// TODO: This will still suspend a synchronous tree if anything
// in the concurrent tree already suspended during this render.
// This is a known bug.
Expand Down
Loading