Skip to content
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.

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

Suspending inside a hidden tree should not cause fallbacks to appear #24699

Merged
merged 2 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ export function createFiberFromOffscreen(
const primaryChildInstance: OffscreenInstance = {
isHidden: false,
pendingMarkers: null,
retryCache: null,
transitions: null,
};
fiber.stateNode = primaryChildInstance;
Expand All @@ -740,6 +741,7 @@ export function createFiberFromLegacyHidden(
isHidden: false,
pendingMarkers: null,
transitions: null,
retryCache: null,
};
fiber.stateNode = instance;
return fiber;
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiber.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ export function createFiberFromOffscreen(
const primaryChildInstance: OffscreenInstance = {
isHidden: false,
pendingMarkers: null,
retryCache: null,
transitions: null,
};
fiber.stateNode = primaryChildInstance;
Expand All @@ -740,6 +741,7 @@ export function createFiberFromLegacyHidden(
isHidden: false,
pendingMarkers: null,
transitions: null,
retryCache: null,
};
fiber.stateNode = instance;
return fiber;
Expand Down
189 changes: 143 additions & 46 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ import {
setShallowSuspenseListContext,
pushPrimaryTreeSuspenseHandler,
pushFallbackTreeSuspenseHandler,
pushOffscreenSuspenseHandler,
reuseSuspenseHandlerOnStack,
popSuspenseHandler,
} from './ReactFiberSuspenseContext.new';
import {
Expand Down Expand Up @@ -678,6 +680,52 @@ function updateOffscreenComponent(
(enableLegacyHidden && nextProps.mode === 'unstable-defer-without-hiding')
) {
// Rendering a hidden tree.

const didSuspend = (workInProgress.flags & DidCapture) !== NoFlags;
if (didSuspend) {
// Something suspended inside a hidden tree

// Include the base lanes from the last render
const nextBaseLanes =
prevState !== null
? mergeLanes(prevState.baseLanes, renderLanes)
: renderLanes;

if (current !== null) {
// Reset to the current children
let currentChild = (workInProgress.child = current.child);

// The current render suspended, but there may be other lanes with
// pending work. We can't read `childLanes` from the current Offscreen
// fiber because we reset it when it was deferred; however, we can read
// the pending lanes from the child fibers.
let currentChildLanes = NoLanes;
while (currentChild !== null) {
currentChildLanes = mergeLanes(
mergeLanes(currentChildLanes, currentChild.lanes),
currentChild.childLanes,
);
currentChild = currentChild.sibling;
}
const lanesWeJustAttempted = nextBaseLanes;
const remainingChildLanes = removeLanes(
currentChildLanes,
lanesWeJustAttempted,
);
workInProgress.childLanes = remainingChildLanes;
} else {
workInProgress.childLanes = NoLanes;
workInProgress.child = null;
}

return deferHiddenOffscreenComponent(
current,
workInProgress,
nextBaseLanes,
renderLanes,
);
}

if ((workInProgress.mode & ConcurrentMode) === NoMode) {
// In legacy sync mode, don't defer the subtree. Render it now.
// TODO: Consider how Offscreen should work with transitions in the future
Expand All @@ -694,50 +742,28 @@ function updateOffscreenComponent(
}
}
reuseHiddenContextOnStack(workInProgress);
pushOffscreenSuspenseHandler(workInProgress);
} else if (!includesSomeLane(renderLanes, (OffscreenLane: Lane))) {
// We're hidden, and we're not rendering at Offscreen. We will bail out
// and resume this tree later.
let nextBaseLanes = renderLanes;
if (prevState !== null) {
// Include the base lanes from the last render
nextBaseLanes = mergeLanes(nextBaseLanes, prevState.baseLanes);
}

// Schedule this fiber to re-render at offscreen priority. Then bailout.
// Schedule this fiber to re-render at Offscreen priority
workInProgress.lanes = workInProgress.childLanes = laneToLanes(
OffscreenLane,
);
const nextState: OffscreenState = {
baseLanes: nextBaseLanes,
// Save the cache pool so we can resume later.
cachePool: enableCache ? getOffscreenDeferredCache() : null,
};
workInProgress.memoizedState = nextState;
workInProgress.updateQueue = null;
if (enableCache) {
// push the cache pool even though we're going to bail out
// because otherwise there'd be a context mismatch
if (current !== null) {
pushTransition(workInProgress, null, null);
}
}

// We're about to bail out, but we need to push this to the stack anyway
// to avoid a push/pop misalignment.
reuseHiddenContextOnStack(workInProgress);

if (enableLazyContextPropagation && current !== null) {
// Since this tree will resume rendering in a separate render, we need
// to propagate parent contexts now so we don't lose track of which
// ones changed.
propagateParentContextChangesToDeferredTree(
current,
workInProgress,
renderLanes,
);
}
// Include the base lanes from the last render
const nextBaseLanes =
prevState !== null
? mergeLanes(prevState.baseLanes, renderLanes)
: renderLanes;

return null;
return deferHiddenOffscreenComponent(
current,
workInProgress,
nextBaseLanes,
renderLanes,
);
} else {
// This is the second render. The surrounding visible content has already
// committed. Now we resume rendering the hidden tree.
Expand All @@ -764,6 +790,7 @@ function updateOffscreenComponent(
} else {
reuseHiddenContextOnStack(workInProgress);
}
pushOffscreenSuspenseHandler(workInProgress);
}
} else {
// Rendering a visible tree.
Expand Down Expand Up @@ -791,6 +818,7 @@ function updateOffscreenComponent(

// Push the lanes that were skipped when we bailed out.
pushHiddenContext(workInProgress, prevState);
reuseSuspenseHandlerOnStack(workInProgress);

// Since we're not hidden anymore, reset the state
workInProgress.memoizedState = null;
Expand All @@ -811,13 +839,54 @@ function updateOffscreenComponent(
// We're about to bail out, but we need to push this to the stack anyway
// to avoid a push/pop misalignment.
reuseHiddenContextOnStack(workInProgress);
reuseSuspenseHandlerOnStack(workInProgress);
}
}

reconcileChildren(current, workInProgress, nextChildren, renderLanes);
return workInProgress.child;
}

function deferHiddenOffscreenComponent(
current: Fiber | null,
workInProgress: Fiber,
nextBaseLanes: Lanes,
renderLanes: Lanes,
) {
const nextState: OffscreenState = {
baseLanes: nextBaseLanes,
// Save the cache pool so we can resume later.
cachePool: enableCache ? getOffscreenDeferredCache() : null,
};
workInProgress.memoizedState = nextState;
if (enableCache) {
// push the cache pool even though we're going to bail out
// because otherwise there'd be a context mismatch
if (current !== null) {
pushTransition(workInProgress, null, null);
}
}

// We're about to bail out, but we need to push this to the stack anyway
// to avoid a push/pop misalignment.
reuseHiddenContextOnStack(workInProgress);

pushOffscreenSuspenseHandler(workInProgress);

if (enableLazyContextPropagation && current !== null) {
// Since this tree will resume rendering in a separate render, we need
// to propagate parent contexts now so we don't lose track of which
// ones changed.
propagateParentContextChangesToDeferredTree(
current,
workInProgress,
renderLanes,
);
}

return null;
}

// Note: These happen to have identical begin phases, for now. We shouldn't hold
// ourselves to this constraint, though. If the behavior diverges, we should
// fork the function.
Expand Down Expand Up @@ -2109,13 +2178,19 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
if (enableTransitionTracing) {
const currentTransitions = getPendingTransitions();
if (currentTransitions !== null) {
// If there are no transitions, we don't need to keep track of tracing markers
const parentMarkerInstances = getMarkerInstances();
const primaryChildUpdateQueue: OffscreenQueue = {
transitions: currentTransitions,
markerInstances: parentMarkerInstances,
};
primaryChildFragment.updateQueue = primaryChildUpdateQueue;
const offscreenQueue: OffscreenQueue | null = (primaryChildFragment.updateQueue: any);
if (offscreenQueue === null) {
const newOffscreenQueue: OffscreenQueue = {
transitions: currentTransitions,
markerInstances: parentMarkerInstances,
wakeables: null,
};
primaryChildFragment.updateQueue = newOffscreenQueue;
} else {
offscreenQueue.transitions = currentTransitions;
offscreenQueue.markerInstances = parentMarkerInstances;
}
}
}

Expand All @@ -2140,6 +2215,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
);
workInProgress.memoizedState = SUSPENDED_MARKER;

// TODO: Transition Tracing is not yet implemented for CPU Suspense.

// Since nothing actually suspended, there will nothing to ping this to
// get it started back up to attempt the next item. While in terms of
// priority this work has the same priority as this current render, it's
Expand Down Expand Up @@ -2201,11 +2278,31 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
const currentTransitions = getPendingTransitions();
if (currentTransitions !== null) {
const parentMarkerInstances = getMarkerInstances();
const primaryChildUpdateQueue: OffscreenQueue = {
transitions: currentTransitions,
markerInstances: parentMarkerInstances,
};
primaryChildFragment.updateQueue = primaryChildUpdateQueue;
const offscreenQueue: OffscreenQueue | null = (primaryChildFragment.updateQueue: any);
const currentOffscreenQueue: OffscreenQueue | null = (current.updateQueue: any);
if (offscreenQueue === null) {
const newOffscreenQueue: OffscreenQueue = {
transitions: currentTransitions,
markerInstances: parentMarkerInstances,
wakeables: null,
};
primaryChildFragment.updateQueue = newOffscreenQueue;
} else if (offscreenQueue === currentOffscreenQueue) {
// If the work-in-progress queue is the same object as current, we
// can't modify it without cloning it first.
const newOffscreenQueue: OffscreenQueue = {
transitions: currentTransitions,
markerInstances: parentMarkerInstances,
wakeables:
currentOffscreenQueue !== null
? currentOffscreenQueue.wakeables
: null,
};
primaryChildFragment.updateQueue = newOffscreenQueue;
} else {
offscreenQueue.transitions = currentTransitions;
offscreenQueue.markerInstances = parentMarkerInstances;
}
}
}
primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2130,6 +2130,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
const primaryChildUpdateQueue: OffscreenQueue = {
transitions: currentTransitions,
markerInstances: parentMarkerInstances,
wakeables: null,
};
primaryChildFragment.updateQueue = primaryChildUpdateQueue;
}
Expand Down Expand Up @@ -2216,6 +2217,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
const primaryChildUpdateQueue: OffscreenQueue = {
transitions: currentTransitions,
markerInstances: parentMarkerInstances,
wakeables: null,
};
primaryChildFragment.updateQueue = primaryChildUpdateQueue;
}
Expand Down
Loading