Skip to content

Commit ff42f0c

Browse files
committed
Revert "Unify use and renderDidSuspendDelayIfPossible implementations (facebook#25922)"
This reverts commit c2d6552.
1 parent 935fba5 commit ff42f0c

File tree

6 files changed

+132
-631
lines changed

6 files changed

+132
-631
lines changed

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ import {
126126
setShallowSuspenseListContext,
127127
ForceSuspenseFallback,
128128
setDefaultShallowSuspenseListContext,
129+
isBadSuspenseFallback,
129130
} from './ReactFiberSuspenseContext';
130131
import {popHiddenContext} from './ReactFiberHiddenContext';
131132
import {findFirstSuspended} from './ReactFiberSuspenseComponent';
@@ -147,6 +148,8 @@ import {
147148
upgradeHydrationErrorsToRecoverable,
148149
} from './ReactFiberHydrationContext';
149150
import {
151+
renderDidSuspend,
152+
renderDidSuspendDelayIfPossible,
150153
renderHasNotSuspendedYet,
151154
getRenderTargetTime,
152155
getWorkInProgressTransitions,
@@ -1224,6 +1227,24 @@ function completeWork(
12241227
if (nextDidTimeout) {
12251228
const offscreenFiber: Fiber = (workInProgress.child: any);
12261229
offscreenFiber.flags |= Visibility;
1230+
1231+
// TODO: This will still suspend a synchronous tree if anything
1232+
// in the concurrent tree already suspended during this render.
1233+
// This is a known bug.
1234+
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
1235+
// TODO: Move this back to throwException because this is too late
1236+
// if this is a large tree which is common for initial loads. We
1237+
// don't know if we should restart a render or not until we get
1238+
// this marker, and this is too late.
1239+
// If this render already had a ping or lower pri updates,
1240+
// and this is the first time we know we're going to suspend we
1241+
// should be able to immediately restart from within throwException.
1242+
if (isBadSuspenseFallback(current, newProps)) {
1243+
renderDidSuspendDelayIfPossible();
1244+
} else {
1245+
renderDidSuspend();
1246+
}
1247+
}
12271248
}
12281249
}
12291250

packages/react-reconciler/src/ReactFiberSuspenseContext.js

Lines changed: 70 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -9,85 +9,93 @@
99

1010
import type {Fiber} from './ReactInternalTypes';
1111
import type {StackCursor} from './ReactFiberStack';
12-
import type {SuspenseProps, SuspenseState} from './ReactFiberSuspenseComponent';
13-
import type {OffscreenState} from './ReactFiberOffscreenComponent';
12+
import type {SuspenseState, SuspenseProps} from './ReactFiberSuspenseComponent';
1413

1514
import {enableSuspenseAvoidThisFallback} from 'shared/ReactFeatureFlags';
1615
import {createCursor, push, pop} from './ReactFiberStack';
1716
import {isCurrentTreeHidden} from './ReactFiberHiddenContext';
18-
import {OffscreenComponent} from './ReactWorkTags';
17+
import {SuspenseComponent, OffscreenComponent} from './ReactWorkTags';
1918

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

25-
// Represents the outermost boundary that is not visible in the current tree.
26-
// Everything above this is the "shell". When this is null, it means we're
27-
// rendering in the shell of the app. If it's non-null, it means we're rendering
28-
// deeper than the shell, inside a new tree that wasn't already visible.
29-
//
30-
// The main way we use this concept is to determine whether showing a fallback
31-
// would result in a desirable or undesirable loading state. Activing a fallback
32-
// in the shell is considered an undersirable loading state, because it would
33-
// mean hiding visible (albeit stale) content in the current tree — we prefer to
34-
// show the stale content, rather than switch to a fallback. But showing a
35-
// fallback in a new tree is fine, because there's no stale content to
36-
// prefer instead.
37-
let shellBoundary: Fiber | null = null;
38-
39-
export function getShellBoundary(): Fiber | null {
40-
return shellBoundary;
24+
function shouldAvoidedBoundaryCapture(
25+
workInProgress: Fiber,
26+
handlerOnStack: Fiber,
27+
props: any,
28+
): boolean {
29+
if (enableSuspenseAvoidThisFallback) {
30+
// If the parent is already showing content, and we're not inside a hidden
31+
// tree, then we should show the avoided fallback.
32+
if (handlerOnStack.alternate !== null && !isCurrentTreeHidden()) {
33+
return true;
34+
}
35+
36+
// If the handler on the stack is also an avoided boundary, then we should
37+
// favor this inner one.
38+
if (
39+
handlerOnStack.tag === SuspenseComponent &&
40+
handlerOnStack.memoizedProps.unstable_avoidThisFallback === true
41+
) {
42+
return true;
43+
}
44+
45+
// If this avoided boundary is dehydrated, then it should capture.
46+
const suspenseState: SuspenseState | null = workInProgress.memoizedState;
47+
if (suspenseState !== null && suspenseState.dehydrated !== null) {
48+
return true;
49+
}
50+
}
51+
52+
// If none of those cases apply, then we should avoid this fallback and show
53+
// the outer one instead.
54+
return false;
4155
}
4256

43-
export function pushPrimaryTreeSuspenseHandler(handler: Fiber): void {
44-
// TODO: Pass as argument
45-
const current = handler.alternate;
46-
const props: SuspenseProps = handler.pendingProps;
47-
48-
// Experimental feature: Some Suspense boundaries are marked as having an
49-
// undesirable fallback state. These have special behavior where we only
50-
// activate the fallback if there's no other boundary on the stack that we can
51-
// use instead.
57+
export function isBadSuspenseFallback(
58+
current: Fiber | null,
59+
nextProps: SuspenseProps,
60+
): boolean {
61+
// Check if this is a "bad" fallback state or a good one. A bad fallback state
62+
// is one that we only show as a last resort; if this is a transition, we'll
63+
// block it from displaying, and wait for more data to arrive.
64+
if (current !== null) {
65+
const prevState: SuspenseState = current.memoizedState;
66+
const isShowingFallback = prevState !== null;
67+
if (!isShowingFallback && !isCurrentTreeHidden()) {
68+
// It's bad to switch to a fallback if content is already visible
69+
return true;
70+
}
71+
}
72+
5273
if (
5374
enableSuspenseAvoidThisFallback &&
54-
props.unstable_avoidThisFallback === true &&
55-
// If an avoided boundary is already visible, it behaves identically to
56-
// a regular Suspense boundary.
57-
(current === null || isCurrentTreeHidden())
75+
nextProps.unstable_avoidThisFallback === true
5876
) {
59-
if (shellBoundary === null) {
60-
// We're rendering in the shell. There's no parent Suspense boundary that
61-
// can provide a desirable fallback state. We'll use this boundary.
62-
push(suspenseHandlerStackCursor, handler, handler);
63-
64-
// However, because this is not a desirable fallback, the children are
65-
// still considered part of the shell. So we intentionally don't assign
66-
// to `shellBoundary`.
67-
} else {
68-
// There's already a parent Suspense boundary that can provide a desirable
69-
// fallback state. Prefer that one.
70-
const handlerOnStack = suspenseHandlerStackCursor.current;
71-
push(suspenseHandlerStackCursor, handlerOnStack, handler);
72-
}
73-
return;
77+
// Experimental: Some fallbacks are always bad
78+
return true;
7479
}
7580

76-
// TODO: If the parent Suspense handler already suspended, there's no reason
77-
// to push a nested Suspense handler, because it will get replaced by the
78-
// outer fallback, anyway. Consider this as a future optimization.
79-
push(suspenseHandlerStackCursor, handler, handler);
80-
if (shellBoundary === null) {
81-
if (current === null || isCurrentTreeHidden()) {
82-
// This boundary is not visible in the current UI.
83-
shellBoundary = handler;
84-
} else {
85-
const prevState: SuspenseState = current.memoizedState;
86-
if (prevState !== null) {
87-
// This boundary is showing a fallback in the current UI.
88-
shellBoundary = handler;
89-
}
90-
}
81+
return false;
82+
}
83+
84+
export function pushPrimaryTreeSuspenseHandler(handler: Fiber): void {
85+
const props = handler.pendingProps;
86+
const handlerOnStack = suspenseHandlerStackCursor.current;
87+
if (
88+
enableSuspenseAvoidThisFallback &&
89+
props.unstable_avoidThisFallback === true &&
90+
handlerOnStack !== null &&
91+
!shouldAvoidedBoundaryCapture(handler, handlerOnStack, props)
92+
) {
93+
// This boundary should not capture if something suspends. Reuse the
94+
// existing handler on the stack.
95+
push(suspenseHandlerStackCursor, handlerOnStack, handler);
96+
} else {
97+
// Push this handler onto the stack.
98+
push(suspenseHandlerStackCursor, handler, handler);
9199
}
92100
}
93101

@@ -101,20 +109,6 @@ export function pushFallbackTreeSuspenseHandler(fiber: Fiber): void {
101109
export function pushOffscreenSuspenseHandler(fiber: Fiber): void {
102110
if (fiber.tag === OffscreenComponent) {
103111
push(suspenseHandlerStackCursor, fiber, fiber);
104-
if (shellBoundary !== null) {
105-
// A parent boundary is showing a fallback, so we've already rendered
106-
// deeper than the shell.
107-
} else {
108-
const current = fiber.alternate;
109-
if (current !== null) {
110-
const prevState: OffscreenState = current.memoizedState;
111-
if (prevState !== null) {
112-
// This is the first boundary in the stack that's already showing
113-
// a fallback. So everything outside is considered the shell.
114-
shellBoundary = fiber;
115-
}
116-
}
117-
}
118112
} else {
119113
// This is a LegacyHidden component.
120114
reuseSuspenseHandlerOnStack(fiber);
@@ -131,10 +125,6 @@ export function getSuspenseHandler(): Fiber | null {
131125

132126
export function popSuspenseHandler(fiber: Fiber): void {
133127
pop(suspenseHandlerStackCursor, fiber);
134-
if (shellBoundary === fiber) {
135-
// Popping back into the shell.
136-
shellBoundary = null;
137-
}
138128
}
139129

140130
// SuspenseList context

packages/react-reconciler/src/ReactFiberThrow.js

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,7 @@ import {
4949
enqueueUpdate,
5050
} from './ReactFiberClassUpdateQueue';
5151
import {markFailedErrorBoundaryForHotReloading} from './ReactFiberHotReloading';
52-
import {
53-
getShellBoundary,
54-
getSuspenseHandler,
55-
} from './ReactFiberSuspenseContext';
52+
import {getSuspenseHandler} from './ReactFiberSuspenseContext';
5653
import {
5754
renderDidError,
5855
renderDidSuspendDelayIfPossible,
@@ -61,7 +58,6 @@ import {
6158
isAlreadyFailedLegacyErrorBoundary,
6259
attachPingListener,
6360
restorePendingUpdaters,
64-
renderDidSuspend,
6561
} from './ReactFiberWorkLoop';
6662
import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext';
6763
import {logCapturedError} from './ReactFiberErrorLogger';
@@ -353,46 +349,11 @@ function throwException(
353349
}
354350
}
355351

356-
// Mark the nearest Suspense boundary to switch to rendering a fallback.
352+
// Schedule the nearest Suspense to re-render the timed out view.
357353
const suspenseBoundary = getSuspenseHandler();
358354
if (suspenseBoundary !== null) {
359355
switch (suspenseBoundary.tag) {
360356
case SuspenseComponent: {
361-
// If this suspense boundary is not already showing a fallback, mark
362-
// the in-progress render as suspended. We try to perform this logic
363-
// as soon as soon as possible during the render phase, so the work
364-
// loop can know things like whether it's OK to switch to other tasks,
365-
// or whether it can wait for data to resolve before continuing.
366-
// TODO: Most of these checks are already performed when entering a
367-
// Suspense boundary. We should track the information on the stack so
368-
// we don't have to recompute it on demand. This would also allow us
369-
// to unify with `use` which needs to perform this logic even sooner,
370-
// before `throwException` is called.
371-
if (sourceFiber.mode & ConcurrentMode) {
372-
if (getShellBoundary() === null) {
373-
// Suspended in the "shell" of the app. This is an undesirable
374-
// loading state. We should avoid committing this tree.
375-
renderDidSuspendDelayIfPossible();
376-
} else {
377-
// If we suspended deeper than the shell, we don't need to delay
378-
// the commmit. However, we still call renderDidSuspend if this is
379-
// a new boundary, to tell the work loop that a new fallback has
380-
// appeared during this render.
381-
// TODO: Theoretically we should be able to delete this branch.
382-
// It's currently used for two things: 1) to throttle the
383-
// appearance of successive loading states, and 2) in
384-
// SuspenseList, to determine whether the children include any
385-
// pending fallbacks. For 1, we should apply throttling to all
386-
// retries, not just ones that render an additional fallback. For
387-
// 2, we should check subtreeFlags instead. Then we can delete
388-
// this branch.
389-
const current = suspenseBoundary.alternate;
390-
if (current === null) {
391-
renderDidSuspend();
392-
}
393-
}
394-
}
395-
396357
suspenseBoundary.flags &= ~ForceClientRender;
397358
markSuspenseBoundaryShouldCapture(
398359
suspenseBoundary,

0 commit comments

Comments
 (0)