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.

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

Jump to bottom

Unify use and renderDidSuspendDelayIfPossible implementations #25922

Merged
merged 2 commits into from
Jan 4, 2023
Merged

Unify use and renderDidSuspendDelayIfPossible implementations #25922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
218d4c4
Move renderDidSuspend logic to throwException
acdlite Dec 7, 2022
7ee7066
Unify `use` and `renderDidSuspendWithDelay` impl
acdlite Dec 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 0 additions & 21 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ import {
setShallowSuspenseListContext,
ForceSuspenseFallback,
setDefaultShallowSuspenseListContext,
isBadSuspenseFallback,
} from './ReactFiberSuspenseContext';
import {popHiddenContext} from './ReactFiberHiddenContext';
import {findFirstSuspended} from './ReactFiberSuspenseComponent';
Expand All @@ -148,8 +147,6 @@ import {
upgradeHydrationErrorsToRecoverable,
} from './ReactFiberHydrationContext';
import {
renderDidSuspend,
renderDidSuspendDelayIfPossible,
renderHasNotSuspendedYet,
getRenderTargetTime,
getWorkInProgressTransitions,
Expand Down Expand Up @@ -1257,24 +1254,6 @@ function completeWork(
if (nextDidTimeout) {
const offscreenFiber: Fiber = (workInProgress.child: any);
offscreenFiber.flags |= Visibility;

// TODO: This will still suspend a synchronous tree if anything
// in the concurrent tree already suspended during this render.
// This is a known bug.
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
// TODO: Move this back to throwException because this is too late
// if this is a large tree which is common for initial loads. We
// don't know if we should restart a render or not until we get
// this marker, and this is too late.
// If this render already had a ping or lower pri updates,
// and this is the first time we know we're going to suspend we
// should be able to immediately restart from within throwException.
if (isBadSuspenseFallback(current, newProps)) {
renderDidSuspendDelayIfPossible();
} else {
renderDidSuspend();
}
}
}
}

Expand Down
150 changes: 80 additions & 70 deletions packages/react-reconciler/src/ReactFiberSuspenseContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,94 +9,86 @@

import type {Fiber} from './ReactInternalTypes';
import type {StackCursor} from './ReactFiberStack';
import type {SuspenseState, SuspenseProps} from './ReactFiberSuspenseComponent';
import type {SuspenseProps, SuspenseState} from './ReactFiberSuspenseComponent';
import type {OffscreenState} from './ReactFiberOffscreenComponent';

import {enableSuspenseAvoidThisFallback} from 'shared/ReactFeatureFlags';
import {createCursor, push, pop} from './ReactFiberStack';
import {isCurrentTreeHidden} from './ReactFiberHiddenContext';
import {SuspenseComponent, OffscreenComponent} from './ReactWorkTags';
import {OffscreenComponent} from './ReactWorkTags';

// The Suspense handler is the boundary that should capture if something
// suspends, i.e. it's the nearest `catch` block on the stack.
const suspenseHandlerStackCursor: StackCursor<Fiber | null> = createCursor(
null,
);

function shouldAvoidedBoundaryCapture(
workInProgress: Fiber,
handlerOnStack: Fiber,
props: any,
): boolean {
if (enableSuspenseAvoidThisFallback) {
// If the parent is already showing content, and we're not inside a hidden
// tree, then we should show the avoided fallback.
if (handlerOnStack.alternate !== null && !isCurrentTreeHidden()) {
return true;
}

// If the handler on the stack is also an avoided boundary, then we should
// favor this inner one.
if (
handlerOnStack.tag === SuspenseComponent &&
handlerOnStack.memoizedProps.unstable_avoidThisFallback === true
) {
return true;
}

// If this avoided boundary is dehydrated, then it should capture.
const suspenseState: SuspenseState | null = workInProgress.memoizedState;
if (suspenseState !== null && suspenseState.dehydrated !== null) {
return true;
}
}

// If none of those cases apply, then we should avoid this fallback and show
// the outer one instead.
return false;
}

export function isBadSuspenseFallback(
current: Fiber | null,
nextProps: SuspenseProps,
): boolean {
// Check if this is a "bad" fallback state or a good one. A bad fallback state
// is one that we only show as a last resort; if this is a transition, we'll
// block it from displaying, and wait for more data to arrive.
if (current !== null) {
const prevState: SuspenseState = current.memoizedState;
const isShowingFallback = prevState !== null;
if (!isShowingFallback && !isCurrentTreeHidden()) {
// It's bad to switch to a fallback if content is already visible
return true;
}
}

if (
enableSuspenseAvoidThisFallback &&
nextProps.unstable_avoidThisFallback === true
) {
// Experimental: Some fallbacks are always bad
return true;
}

return false;
// Represents the outermost boundary that is not visible in the current tree.
// Everything above this is the "shell". When this is null, it means we're
// rendering in the shell of the app. If it's non-null, it means we're rendering
// deeper than the shell, inside a new tree that wasn't already visible.
//
// The main way we use this concept is to determine whether showing a fallback
// would result in a desirable or undesirable loading state. Activing a fallback
// in the shell is considered an undersirable loading state, because it would
// mean hiding visible (albeit stale) content in the current tree — we prefer to
// show the stale content, rather than switch to a fallback. But showing a
// fallback in a new tree is fine, because there's no stale content to
// prefer instead.
let shellBoundary: Fiber | null = null;

export function getShellBoundary(): Fiber | null {
return shellBoundary;
}

export function pushPrimaryTreeSuspenseHandler(handler: Fiber): void {
const props = handler.pendingProps;
const handlerOnStack = suspenseHandlerStackCursor.current;
// TODO: Pass as argument
const current = handler.alternate;
const props: SuspenseProps = handler.pendingProps;

// Experimental feature: Some Suspense boundaries are marked as having an
// undesirable fallback state. These have special behavior where we only
// activate the fallback if there's no other boundary on the stack that we can
// use instead.
if (
enableSuspenseAvoidThisFallback &&
props.unstable_avoidThisFallback === true &&
handlerOnStack !== null &&
!shouldAvoidedBoundaryCapture(handler, handlerOnStack, props)
// If an avoided boundary is already visible, it behaves identically to
// a regular Suspense boundary.
(current === null || isCurrentTreeHidden())
) {
// This boundary should not capture if something suspends. Reuse the
// existing handler on the stack.
push(suspenseHandlerStackCursor, handlerOnStack, handler);
} else {
// Push this handler onto the stack.
push(suspenseHandlerStackCursor, handler, handler);
if (shellBoundary === null) {
// We're rendering in the shell. There's no parent Suspense boundary that
// can provide a desirable fallback state. We'll use this boundary.
push(suspenseHandlerStackCursor, handler, handler);

// However, because this is not a desirable fallback, the children are
// still considered part of the shell. So we intentionally don't assign
// to `shellBoundary`.
} else {
// There's already a parent Suspense boundary that can provide a desirable
// fallback state. Prefer that one.
const handlerOnStack = suspenseHandlerStackCursor.current;
push(suspenseHandlerStackCursor, handlerOnStack, handler);
}
return;
}

// TODO: If the parent Suspense handler already suspended, there's no reason
// to push a nested Suspense handler, because it will get replaced by the
// outer fallback, anyway. Consider this as a future optimization.
push(suspenseHandlerStackCursor, handler, handler);
if (shellBoundary === null) {
if (current === null || isCurrentTreeHidden()) {
// This boundary is not visible in the current UI.
shellBoundary = handler;
} else {
const prevState: SuspenseState = current.memoizedState;
if (prevState !== null) {
// This boundary is showing a fallback in the current UI.
shellBoundary = handler;
}
}
}
}

Expand All @@ -110,6 +102,20 @@ export function pushFallbackTreeSuspenseHandler(fiber: Fiber): void {
export function pushOffscreenSuspenseHandler(fiber: Fiber): void {
if (fiber.tag === OffscreenComponent) {
push(suspenseHandlerStackCursor, fiber, fiber);
if (shellBoundary !== null) {
// A parent boundary is showing a fallback, so we've already rendered
// deeper than the shell.
} else {
const current = fiber.alternate;
if (current !== null) {
const prevState: OffscreenState = current.memoizedState;
if (prevState !== null) {
// This is the first boundary in the stack that's already showing
// a fallback. So everything outside is considered the shell.
shellBoundary = fiber;
}
}
}
} else {
// This is a LegacyHidden component.
reuseSuspenseHandlerOnStack(fiber);
Expand All @@ -126,6 +132,10 @@ export function getSuspenseHandler(): Fiber | null {

export function popSuspenseHandler(fiber: Fiber): void {
pop(suspenseHandlerStackCursor, fiber);
if (shellBoundary === fiber) {
// Popping back into the shell.
shellBoundary = null;
}
}

// SuspenseList context
Expand Down
43 changes: 41 additions & 2 deletions packages/react-reconciler/src/ReactFiberThrow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ import {
enqueueUpdate,
} from './ReactFiberClassUpdateQueue';
import {markFailedErrorBoundaryForHotReloading} from './ReactFiberHotReloading';
import {getSuspenseHandler} from './ReactFiberSuspenseContext';
import {
getShellBoundary,
getSuspenseHandler,
} from './ReactFiberSuspenseContext';
import {
renderDidError,
renderDidSuspendDelayIfPossible,
Expand All @@ -58,6 +61,7 @@ import {
isAlreadyFailedLegacyErrorBoundary,
attachPingListener,
restorePendingUpdaters,
renderDidSuspend,
} from './ReactFiberWorkLoop';
import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext';
import {logCapturedError} from './ReactFiberErrorLogger';
Expand Down Expand Up @@ -349,11 +353,46 @@ function throwException(
}
}

// Schedule the nearest Suspense to re-render the timed out view.
// Mark the nearest Suspense boundary to switch to rendering a fallback.
const suspenseBoundary = getSuspenseHandler();
if (suspenseBoundary !== null) {
switch (suspenseBoundary.tag) {
case SuspenseComponent: {
// If this suspense boundary is not already showing a fallback, mark
// the in-progress render as suspended. We try to perform this logic
// as soon as soon as possible during the render phase, so the work
// loop can know things like whether it's OK to switch to other tasks,
// or whether it can wait for data to resolve before continuing.
// TODO: Most of these checks are already performed when entering a
// Suspense boundary. We should track the information on the stack so
// we don't have to recompute it on demand. This would also allow us
// to unify with `use` which needs to perform this logic even sooner,
// before `throwException` is called.
if (sourceFiber.mode & ConcurrentMode) {
if (getShellBoundary() === null) {
// Suspended in the "shell" of the app. This is an undesirable
// loading state. We should avoid committing this tree.
renderDidSuspendDelayIfPossible();
} else {
// If we suspended deeper than the shell, we don't need to delay
// the commmit. However, we still call renderDidSuspend if this is
// a new boundary, to tell the work loop that a new fallback has
// appeared during this render.
// TODO: Theoretically we should be able to delete this branch.
// It's currently used for two things: 1) to throttle the
// appearance of successive loading states, and 2) in
// SuspenseList, to determine whether the children include any
// pending fallbacks. For 1, we should apply throttling to all
// retries, not just ones that render an additional fallback. For
// 2, we should check subtreeFlags instead. Then we can delete
// this branch.
const current = suspenseBoundary.alternate;
if (current === null) {
renderDidSuspend();
}
}
}

suspenseBoundary.flags &= ~ForceClientRender;
markSuspenseBoundaryShouldCapture(
suspenseBoundary,
Expand Down