Skip to content

Commit 8e2f9b0

Browse files
lunaruanlunaruan
authored
move passive flag (#24339)
Previously, we were only adding the passive flag when we add the Visibility flag, which is only set when we go from primary to fallback. Now, we add the passive flag BOTH when we go from primary to fallback and from fallback to primary. An alternate solution is to add the passive flag in the same place as the visibility flag in the offscreen complete phase (rather than the suspense complete phase), but this feature is currently only for suspense, and offscreen can be used in different ways, so for now we add the passive flag only in the suspense component's complete phase. We might want to revisit this later when we think about how offscreen should work with transition tracing.
1 parent 55a21ef commit 8e2f9b0

File tree

2 files changed

+98
-90
lines changed

2 files changed

+98
-90
lines changed

‎packages/react-reconciler/src/ReactFiberCompleteWork.new.js

+49-45
Original file line numberDiff line numberDiff line change
@@ -1192,55 +1192,59 @@ function completeWork(
11921192
}
11931193

11941194
// If the suspended state of the boundary changes, we need to schedule
1195-
// an effect to toggle the subtree's visibility. When we switch from
1196-
// fallback -> primary, the inner Offscreen fiber schedules this effect
1197-
// as part of its normal complete phase. But when we switch from
1198-
// primary -> fallback, the inner Offscreen fiber does not have a complete
1199-
// phase. So we need to schedule its effect here.
1200-
//
1201-
// We also use this flag to connect/disconnect the effects, but the same
1202-
// logic applies: when re-connecting, the Offscreen fiber's complete
1203-
// phase will handle scheduling the effect. It's only when the fallback
1204-
// is active that we have to do anything special.
1205-
if (nextDidTimeout && !prevDidTimeout) {
1206-
const offscreenFiber: Fiber = (workInProgress.child: any);
1207-
offscreenFiber.flags |= Visibility;
1208-
1209-
// If the suspended state of the boundary changes, we need to schedule
1210-
// a passive effect, which is when we process the transitions
1195+
// a passive effect, which is when we process the transitions
1196+
if (nextDidTimeout !== prevDidTimeout) {
12111197
if (enableTransitionTracing) {
1198+
const offscreenFiber: Fiber = (workInProgress.child: any);
12121199
offscreenFiber.flags |= Passive;
12131200
}
12141201

1215-
// TODO: This will still suspend a synchronous tree if anything
1216-
// in the concurrent tree already suspended during this render.
1217-
// This is a known bug.
1218-
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
1219-
// TODO: Move this back to throwException because this is too late
1220-
// if this is a large tree which is common for initial loads. We
1221-
// don't know if we should restart a render or not until we get
1222-
// this marker, and this is too late.
1223-
// If this render already had a ping or lower pri updates,
1224-
// and this is the first time we know we're going to suspend we
1225-
// should be able to immediately restart from within throwException.
1226-
const hasInvisibleChildContext =
1227-
current === null &&
1228-
(workInProgress.memoizedProps.unstable_avoidThisFallback !== true ||
1229-
!enableSuspenseAvoidThisFallback);
1230-
if (
1231-
hasInvisibleChildContext ||
1232-
hasSuspenseContext(
1233-
suspenseStackCursor.current,
1234-
(InvisibleParentSuspenseContext: SuspenseContext),
1235-
)
1236-
) {
1237-
// If this was in an invisible tree or a new render, then showing
1238-
// this boundary is ok.
1239-
renderDidSuspend();
1240-
} else {
1241-
// Otherwise, we're going to have to hide content so we should
1242-
// suspend for longer if possible.
1243-
renderDidSuspendDelayIfPossible();
1202+
// If the suspended state of the boundary changes, we need to schedule
1203+
// an effect to toggle the subtree's visibility. When we switch from
1204+
// fallback -> primary, the inner Offscreen fiber schedules this effect
1205+
// as part of its normal complete phase. But when we switch from
1206+
// primary -> fallback, the inner Offscreen fiber does not have a complete
1207+
// phase. So we need to schedule its effect here.
1208+
//
1209+
// We also use this flag to connect/disconnect the effects, but the same
1210+
// logic applies: when re-connecting, the Offscreen fiber's complete
1211+
// phase will handle scheduling the effect. It's only when the fallback
1212+
// is active that we have to do anything special.
1213+
if (nextDidTimeout) {
1214+
const offscreenFiber: Fiber = (workInProgress.child: any);
1215+
offscreenFiber.flags |= Visibility;
1216+
1217+
// TODO: This will still suspend a synchronous tree if anything
1218+
// in the concurrent tree already suspended during this render.
1219+
// This is a known bug.
1220+
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
1221+
// TODO: Move this back to throwException because this is too late
1222+
// if this is a large tree which is common for initial loads. We
1223+
// don't know if we should restart a render or not until we get
1224+
// this marker, and this is too late.
1225+
// If this render already had a ping or lower pri updates,
1226+
// and this is the first time we know we're going to suspend we
1227+
// should be able to immediately restart from within throwException.
1228+
const hasInvisibleChildContext =
1229+
current === null &&
1230+
(workInProgress.memoizedProps.unstable_avoidThisFallback !==
1231+
true ||
1232+
!enableSuspenseAvoidThisFallback);
1233+
if (
1234+
hasInvisibleChildContext ||
1235+
hasSuspenseContext(
1236+
suspenseStackCursor.current,
1237+
(InvisibleParentSuspenseContext: SuspenseContext),
1238+
)
1239+
) {
1240+
// If this was in an invisible tree or a new render, then showing
1241+
// this boundary is ok.
1242+
renderDidSuspend();
1243+
} else {
1244+
// Otherwise, we're going to have to hide content so we should
1245+
// suspend for longer if possible.
1246+
renderDidSuspendDelayIfPossible();
1247+
}
12441248
}
12451249
}
12461250
}

‎packages/react-reconciler/src/ReactFiberCompleteWork.old.js

+49-45
Original file line numberDiff line numberDiff line change
@@ -1192,55 +1192,59 @@ function completeWork(
11921192
}
11931193

11941194
// If the suspended state of the boundary changes, we need to schedule
1195-
// an effect to toggle the subtree's visibility. When we switch from
1196-
// fallback -> primary, the inner Offscreen fiber schedules this effect
1197-
// as part of its normal complete phase. But when we switch from
1198-
// primary -> fallback, the inner Offscreen fiber does not have a complete
1199-
// phase. So we need to schedule its effect here.
1200-
//
1201-
// We also use this flag to connect/disconnect the effects, but the same
1202-
// logic applies: when re-connecting, the Offscreen fiber's complete
1203-
// phase will handle scheduling the effect. It's only when the fallback
1204-
// is active that we have to do anything special.
1205-
if (nextDidTimeout && !prevDidTimeout) {
1206-
const offscreenFiber: Fiber = (workInProgress.child: any);
1207-
offscreenFiber.flags |= Visibility;
1208-
1209-
// If the suspended state of the boundary changes, we need to schedule
1210-
// a passive effect, which is when we process the transitions
1195+
// a passive effect, which is when we process the transitions
1196+
if (nextDidTimeout !== prevDidTimeout) {
12111197
if (enableTransitionTracing) {
1198+
const offscreenFiber: Fiber = (workInProgress.child: any);
12121199
offscreenFiber.flags |= Passive;
12131200
}
12141201

1215-
// TODO: This will still suspend a synchronous tree if anything
1216-
// in the concurrent tree already suspended during this render.
1217-
// This is a known bug.
1218-
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
1219-
// TODO: Move this back to throwException because this is too late
1220-
// if this is a large tree which is common for initial loads. We
1221-
// don't know if we should restart a render or not until we get
1222-
// this marker, and this is too late.
1223-
// If this render already had a ping or lower pri updates,
1224-
// and this is the first time we know we're going to suspend we
1225-
// should be able to immediately restart from within throwException.
1226-
const hasInvisibleChildContext =
1227-
current === null &&
1228-
(workInProgress.memoizedProps.unstable_avoidThisFallback !== true ||
1229-
!enableSuspenseAvoidThisFallback);
1230-
if (
1231-
hasInvisibleChildContext ||
1232-
hasSuspenseContext(
1233-
suspenseStackCursor.current,
1234-
(InvisibleParentSuspenseContext: SuspenseContext),
1235-
)
1236-
) {
1237-
// If this was in an invisible tree or a new render, then showing
1238-
// this boundary is ok.
1239-
renderDidSuspend();
1240-
} else {
1241-
// Otherwise, we're going to have to hide content so we should
1242-
// suspend for longer if possible.
1243-
renderDidSuspendDelayIfPossible();
1202+
// If the suspended state of the boundary changes, we need to schedule
1203+
// an effect to toggle the subtree's visibility. When we switch from
1204+
// fallback -> primary, the inner Offscreen fiber schedules this effect
1205+
// as part of its normal complete phase. But when we switch from
1206+
// primary -> fallback, the inner Offscreen fiber does not have a complete
1207+
// phase. So we need to schedule its effect here.
1208+
//
1209+
// We also use this flag to connect/disconnect the effects, but the same
1210+
// logic applies: when re-connecting, the Offscreen fiber's complete
1211+
// phase will handle scheduling the effect. It's only when the fallback
1212+
// is active that we have to do anything special.
1213+
if (nextDidTimeout) {
1214+
const offscreenFiber: Fiber = (workInProgress.child: any);
1215+
offscreenFiber.flags |= Visibility;
1216+
1217+
// TODO: This will still suspend a synchronous tree if anything
1218+
// in the concurrent tree already suspended during this render.
1219+
// This is a known bug.
1220+
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
1221+
// TODO: Move this back to throwException because this is too late
1222+
// if this is a large tree which is common for initial loads. We
1223+
// don't know if we should restart a render or not until we get
1224+
// this marker, and this is too late.
1225+
// If this render already had a ping or lower pri updates,
1226+
// and this is the first time we know we're going to suspend we
1227+
// should be able to immediately restart from within throwException.
1228+
const hasInvisibleChildContext =
1229+
current === null &&
1230+
(workInProgress.memoizedProps.unstable_avoidThisFallback !==
1231+
true ||
1232+
!enableSuspenseAvoidThisFallback);
1233+
if (
1234+
hasInvisibleChildContext ||
1235+
hasSuspenseContext(
1236+
suspenseStackCursor.current,
1237+
(InvisibleParentSuspenseContext: SuspenseContext),
1238+
)
1239+
) {
1240+
// If this was in an invisible tree or a new render, then showing
1241+
// this boundary is ok.
1242+
renderDidSuspend();
1243+
} else {
1244+
// Otherwise, we're going to have to hide content so we should
1245+
// suspend for longer if possible.
1246+
renderDidSuspendDelayIfPossible();
1247+
}
12441248
}
12451249
}
12461250
}

0 commit comments

Comments
 (0)