Skip to content

Commit 796fff5

Browse files
acdliteacdlite
authored
Allow suspending outside a Suspense boundary (facebook#23267)
(If the update is wrapped in startTransition) Currently you're not allowed to suspend outside of a Suspense boundary. We throw an error: > A React component suspended while rendering, but no fallback UI was specified We treat this case like an error because discrete renders are expected to finish synchronously to maintain consistency with external state. However, during a concurrent transition (startTransition), what we can do instead is treat this case like a refresh transition: suspend the commit without showing a fallback. The behavior is roughly as if there were a built-in Suspense boundary at the root of the app with unstable_avoidThisFallback enabled. Conceptually it's very similar because during hydration you're already showing server-rendered UI; there's no need to replace that with a fallback when something suspends.
1 parent 64223fe commit 796fff5

File tree

7 files changed

+414
-182
lines changed

7 files changed

+414
-182
lines changed

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

Lines changed: 60 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import {
6262
} from './ReactFiberSuspenseContext.new';
6363
import {
6464
renderDidError,
65+
renderDidSuspendDelayIfPossible,
6566
onUncaughtError,
6667
markLegacyErrorBoundaryAsFailed,
6768
isAlreadyFailedLegacyErrorBoundary,
@@ -78,6 +79,7 @@ import {
7879
includesSomeLane,
7980
mergeLanes,
8081
pickArbitraryLane,
82+
includesOnlyTransitions,
8183
} from './ReactFiberLane.new';
8284
import {
8385
getIsHydrating,
@@ -165,12 +167,7 @@ function createClassErrorUpdate(
165167
return update;
166168
}
167169

168-
function attachWakeableListeners(
169-
suspenseBoundary: Fiber,
170-
root: FiberRoot,
171-
wakeable: Wakeable,
172-
lanes: Lanes,
173-
) {
170+
function attachPingListener(root: FiberRoot, wakeable: Wakeable, lanes: Lanes) {
174171
// Attach a ping listener
175172
//
176173
// The data might resolve before we have a chance to commit the fallback. Or,
@@ -183,34 +180,39 @@ function attachWakeableListeners(
183180
//
184181
// We only need to do this in concurrent mode. Legacy Suspense always
185182
// commits fallbacks synchronously, so there are no pings.
186-
if (suspenseBoundary.mode & ConcurrentMode) {
187-
let pingCache = root.pingCache;
188-
let threadIDs;
189-
if (pingCache === null) {
190-
pingCache = root.pingCache = new PossiblyWeakMap();
183+
let pingCache = root.pingCache;
184+
let threadIDs;
185+
if (pingCache === null) {
186+
pingCache = root.pingCache = new PossiblyWeakMap();
187+
threadIDs = new Set();
188+
pingCache.set(wakeable, threadIDs);
189+
} else {
190+
threadIDs = pingCache.get(wakeable);
191+
if (threadIDs === undefined) {
191192
threadIDs = new Set();
192193
pingCache.set(wakeable, threadIDs);
193-
} else {
194-
threadIDs = pingCache.get(wakeable);
195-
if (threadIDs === undefined) {
196-
threadIDs = new Set();
197-
pingCache.set(wakeable, threadIDs);
198-
}
199194
}
200-
if (!threadIDs.has(lanes)) {
201-
// Memoize using the thread ID to prevent redundant listeners.
202-
threadIDs.add(lanes);
203-
const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes);
204-
if (enableUpdaterTracking) {
205-
if (isDevToolsPresent) {
206-
// If we have pending work still, restore the original updaters
207-
restorePendingUpdaters(root, lanes);
208-
}
195+
}
196+
if (!threadIDs.has(lanes)) {
197+
// Memoize using the thread ID to prevent redundant listeners.
198+
threadIDs.add(lanes);
199+
const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes);
200+
if (enableUpdaterTracking) {
201+
if (isDevToolsPresent) {
202+
// If we have pending work still, restore the original updaters
203+
restorePendingUpdaters(root, lanes);
209204
}
210-
wakeable.then(ping, ping);
211205
}
206+
wakeable.then(ping, ping);
212207
}
208+
}
213209

210+
function attachRetryListener(
211+
suspenseBoundary: Fiber,
212+
root: FiberRoot,
213+
wakeable: Wakeable,
214+
lanes: Lanes,
215+
) {
214216
// Retry listener
215217
//
216218
// If the fallback does commit, we need to attach a different type of
@@ -470,24 +472,47 @@ function throwException(
470472
root,
471473
rootRenderLanes,
472474
);
473-
attachWakeableListeners(
474-
suspenseBoundary,
475-
root,
476-
wakeable,
477-
rootRenderLanes,
478-
);
475+
// We only attach ping listeners in concurrent mode. Legacy Suspense always
476+
// commits fallbacks synchronously, so there are no pings.
477+
if (suspenseBoundary.mode & ConcurrentMode) {
478+
attachPingListener(root, wakeable, rootRenderLanes);
479+
}
480+
attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes);
479481
return;
480482
} else {
481-
// No boundary was found. Fallthrough to error mode.
483+
// No boundary was found. If we're inside startTransition, this is OK.
484+
// We can suspend and wait for more data to arrive.
485+
486+
if (includesOnlyTransitions(rootRenderLanes)) {
487+
// This is a transition. Suspend. Since we're not activating a Suspense
488+
// boundary, this will unwind all the way to the root without performing
489+
// a second pass to render a fallback. (This is arguably how refresh
490+
// transitions should work, too, since we're not going to commit the
491+
// fallbacks anyway.)
492+
attachPingListener(root, wakeable, rootRenderLanes);
493+
renderDidSuspendDelayIfPossible();
494+
return;
495+
}
496+
497+
// We're not in a transition. We treat this case like an error because
498+
// discrete renders are expected to finish synchronously to maintain
499+
// consistency with external state.
500+
// TODO: This will error during non-transition concurrent renders, too.
501+
// But maybe it shouldn't?
502+
482503
// TODO: We should never call getComponentNameFromFiber in production.
483504
// Log a warning or something to prevent us from accidentally bundling it.
484-
value = new Error(
505+
const uncaughtSuspenseError = new Error(
485506
(getComponentNameFromFiber(sourceFiber) || 'A React component') +
486507
' suspended while rendering, but no fallback UI was specified.\n' +
487508
'\n' +
488509
'Add a <Suspense fallback=...> component higher in the tree to ' +
489510
'provide a loading indicator or placeholder to display.',
490511
);
512+
513+
// If we're outside a transition, fall through to the regular error path.
514+
// The error will be caught by the nearest suspense boundary.
515+
value = uncaughtSuspenseError;
491516
}
492517
} else {
493518
// This is a regular error, not a Suspense wakeable.

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

Lines changed: 60 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import {
6262
} from './ReactFiberSuspenseContext.old';
6363
import {
6464
renderDidError,
65+
renderDidSuspendDelayIfPossible,
6566
onUncaughtError,
6667
markLegacyErrorBoundaryAsFailed,
6768
isAlreadyFailedLegacyErrorBoundary,
@@ -78,6 +79,7 @@ import {
7879
includesSomeLane,
7980
mergeLanes,
8081
pickArbitraryLane,
82+
includesOnlyTransitions,
8183
} from './ReactFiberLane.old';
8284
import {
8385
getIsHydrating,
@@ -165,12 +167,7 @@ function createClassErrorUpdate(
165167
return update;
166168
}
167169

168-
function attachWakeableListeners(
169-
suspenseBoundary: Fiber,
170-
root: FiberRoot,
171-
wakeable: Wakeable,
172-
lanes: Lanes,
173-
) {
170+
function attachPingListener(root: FiberRoot, wakeable: Wakeable, lanes: Lanes) {
174171
// Attach a ping listener
175172
//
176173
// The data might resolve before we have a chance to commit the fallback. Or,
@@ -183,34 +180,39 @@ function attachWakeableListeners(
183180
//
184181
// We only need to do this in concurrent mode. Legacy Suspense always
185182
// commits fallbacks synchronously, so there are no pings.
186-
if (suspenseBoundary.mode & ConcurrentMode) {
187-
let pingCache = root.pingCache;
188-
let threadIDs;
189-
if (pingCache === null) {
190-
pingCache = root.pingCache = new PossiblyWeakMap();
183+
let pingCache = root.pingCache;
184+
let threadIDs;
185+
if (pingCache === null) {
186+
pingCache = root.pingCache = new PossiblyWeakMap();
187+
threadIDs = new Set();
188+
pingCache.set(wakeable, threadIDs);
189+
} else {
190+
threadIDs = pingCache.get(wakeable);
191+
if (threadIDs === undefined) {
191192
threadIDs = new Set();
192193
pingCache.set(wakeable, threadIDs);
193-
} else {
194-
threadIDs = pingCache.get(wakeable);
195-
if (threadIDs === undefined) {
196-
threadIDs = new Set();
197-
pingCache.set(wakeable, threadIDs);
198-
}
199194
}
200-
if (!threadIDs.has(lanes)) {
201-
// Memoize using the thread ID to prevent redundant listeners.
202-
threadIDs.add(lanes);
203-
const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes);
204-
if (enableUpdaterTracking) {
205-
if (isDevToolsPresent) {
206-
// If we have pending work still, restore the original updaters
207-
restorePendingUpdaters(root, lanes);
208-
}
195+
}
196+
if (!threadIDs.has(lanes)) {
197+
// Memoize using the thread ID to prevent redundant listeners.
198+
threadIDs.add(lanes);
199+
const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes);
200+
if (enableUpdaterTracking) {
201+
if (isDevToolsPresent) {
202+
// If we have pending work still, restore the original updaters
203+
restorePendingUpdaters(root, lanes);
209204
}
210-
wakeable.then(ping, ping);
211205
}
206+
wakeable.then(ping, ping);
212207
}
208+
}
213209

210+
function attachRetryListener(
211+
suspenseBoundary: Fiber,
212+
root: FiberRoot,
213+
wakeable: Wakeable,
214+
lanes: Lanes,
215+
) {
214216
// Retry listener
215217
//
216218
// If the fallback does commit, we need to attach a different type of
@@ -470,24 +472,47 @@ function throwException(
470472
root,
471473
rootRenderLanes,
472474
);
473-
attachWakeableListeners(
474-
suspenseBoundary,
475-
root,
476-
wakeable,
477-
rootRenderLanes,
478-
);
475+
// We only attach ping listeners in concurrent mode. Legacy Suspense always
476+
// commits fallbacks synchronously, so there are no pings.
477+
if (suspenseBoundary.mode & ConcurrentMode) {
478+
attachPingListener(root, wakeable, rootRenderLanes);
479+
}
480+
attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes);
479481
return;
480482
} else {
481-
// No boundary was found. Fallthrough to error mode.
483+
// No boundary was found. If we're inside startTransition, this is OK.
484+
// We can suspend and wait for more data to arrive.
485+
486+
if (includesOnlyTransitions(rootRenderLanes)) {
487+
// This is a transition. Suspend. Since we're not activating a Suspense
488+
// boundary, this will unwind all the way to the root without performing
489+
// a second pass to render a fallback. (This is arguably how refresh
490+
// transitions should work, too, since we're not going to commit the
491+
// fallbacks anyway.)
492+
attachPingListener(root, wakeable, rootRenderLanes);
493+
renderDidSuspendDelayIfPossible();
494+
return;
495+
}
496+
497+
// We're not in a transition. We treat this case like an error because
498+
// discrete renders are expected to finish synchronously to maintain
499+
// consistency with external state.
500+
// TODO: This will error during non-transition concurrent renders, too.
501+
// But maybe it shouldn't?
502+
482503
// TODO: We should never call getComponentNameFromFiber in production.
483504
// Log a warning or something to prevent us from accidentally bundling it.
484-
value = new Error(
505+
const uncaughtSuspenseError = new Error(
485506
(getComponentNameFromFiber(sourceFiber) || 'A React component') +
486507
' suspended while rendering, but no fallback UI was specified.\n' +
487508
'\n' +
488509
'Add a <Suspense fallback=...> component higher in the tree to ' +
489510
'provide a loading indicator or placeholder to display.',
490511
);
512+
513+
// If we're outside a transition, fall through to the regular error path.
514+
// The error will be caught by the nearest suspense boundary.
515+
value = uncaughtSuspenseError;
491516
}
492517
} else {
493518
// This is a regular error, not a Suspense wakeable.

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,17 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
8989
popTopLevelLegacyContextObject(workInProgress);
9090
resetMutableSourceWorkInProgressVersions();
9191
const flags = workInProgress.flags;
92-
93-
if ((flags & DidCapture) !== NoFlags) {
94-
throw new Error(
95-
'The root failed to unmount after an error. This is likely a bug in ' +
96-
'React. Please file an issue.',
97-
);
92+
if (
93+
(flags & ShouldCapture) !== NoFlags &&
94+
(flags & DidCapture) === NoFlags
95+
) {
96+
// There was an error during render that wasn't captured by a suspense
97+
// boundary. Do a second pass on the root to unmount the children.
98+
workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
99+
return workInProgress;
98100
}
99-
100-
workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
101-
return workInProgress;
101+
// We unwound to the root without completing it. Exit.
102+
return null;
102103
}
103104
case HostComponent: {
104105
// TODO: popHydrationState

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,17 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
8989
popTopLevelLegacyContextObject(workInProgress);
9090
resetMutableSourceWorkInProgressVersions();
9191
const flags = workInProgress.flags;
92-
93-
if ((flags & DidCapture) !== NoFlags) {
94-
throw new Error(
95-
'The root failed to unmount after an error. This is likely a bug in ' +
96-
'React. Please file an issue.',
97-
);
92+
if (
93+
(flags & ShouldCapture) !== NoFlags &&
94+
(flags & DidCapture) === NoFlags
95+
) {
96+
// There was an error during render that wasn't captured by a suspense
97+
// boundary. Do a second pass on the root to unmount the children.
98+
workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
99+
return workInProgress;
98100
}
99-
100-
workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
101-
return workInProgress;
101+
// We unwound to the root without completing it. Exit.
102+
return null;
102103
}
103104
case HostComponent: {
104105
// TODO: popHydrationState

0 commit comments

Comments
 (0)