Skip to content

Commit b54bb03

Browse files
committed
Don't unwind stack while suspended, when possible
Initial draft. I need to test this more. If a promise is passed to `use`, but the I/O isn't cached, we should still be able to unwrap it. This already worked in Server Components, and during SSR. For Fiber (in the browser), before this fix the state would get lost between attempts unless the promise resolved immediately in a microtask, which requires IO to be cached. This was due to an implementation quirk of Fiber where the state is reset as soon as the stack unwinds. The workaround is to suspend the entire Fiber work loop until the promise resolves. The Server Components and SSR runtimes don't require a workaround: they can maintain multiple parallel child tasks and reuse the state indefinitely across attempts. That's ideally how Fiber should work, too, but it will require larger refactor. The downside of our approach in Fiber is that it won't "warm up" the siblings while you're suspended, but to avoid waterfalls you're supposed to hoist data fetches higher in the tree regardless. But we have other ideas for how we can add this back in the future. (Though again, this doesn't affect Server Components, which already have the ideal behavior.)
1 parent c11c136 commit b54bb03

File tree

3 files changed

+386
-30
lines changed

3 files changed

+386
-30
lines changed

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 112 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@
99

1010
import {REACT_STRICT_MODE_TYPE} from 'shared/ReactSymbols';
1111

12-
import type {Wakeable} from 'shared/ReactTypes';
12+
import type {Wakeable, Thenable} from 'shared/ReactTypes';
1313
import type {Fiber, FiberRoot} from './ReactInternalTypes';
1414
import type {Lanes, Lane} from './ReactFiberLane.new';
15-
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
15+
import type {
16+
SuspenseProps,
17+
SuspenseState,
18+
} from './ReactFiberSuspenseComponent.new';
1619
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';
1720
import type {EventPriority} from './ReactEventPriorities.new';
1821
import type {
@@ -272,6 +275,10 @@ import {
272275
isThenableStateResolved,
273276
} from './ReactFiberThenable.new';
274277
import {schedulePostPaintCallback} from './ReactPostPaintCallback';
278+
import {
279+
getSuspenseHandler,
280+
isBadSuspenseFallback,
281+
} from './ReactFiberSuspenseContext.new';
275282

276283
const ceil = Math.ceil;
277284

@@ -313,7 +320,7 @@ let workInProgressRootRenderLanes: Lanes = NoLanes;
313320
opaque type SuspendedReason = 0 | 1 | 2 | 3 | 4;
314321
const NotSuspended: SuspendedReason = 0;
315322
const SuspendedOnError: SuspendedReason = 1;
316-
// const SuspendedOnData: SuspendedReason = 2;
323+
const SuspendedOnData: SuspendedReason = 2;
317324
const SuspendedOnImmediate: SuspendedReason = 3;
318325
const SuspendedAndReadyToUnwind: SuspendedReason = 4;
319326

@@ -707,6 +714,18 @@ export function scheduleUpdateOnFiber(
707714
}
708715
}
709716

717+
// Check if the work loop is currently suspended and waiting for data to
718+
// finish loading.
719+
if (
720+
workInProgressSuspendedReason === SuspendedOnData &&
721+
root === workInProgressRoot
722+
) {
723+
// The incoming update might unblock the current render. Interrupt the
724+
// current attempt and restart from the top.
725+
prepareFreshStack(root, NoLanes);
726+
markRootSuspended(root, workInProgressRootRenderLanes);
727+
}
728+
710729
// Mark that the root has a pending update.
711730
markRootUpdated(root, lane, eventTime);
712731

@@ -1131,6 +1150,17 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
11311150
if (root.callbackNode === originalCallbackNode) {
11321151
// The task node scheduled for this root is the same one that's
11331152
// currently executed. Need to return a continuation.
1153+
if (workInProgressSuspendedReason === SuspendedOnData) {
1154+
// Special case: The work loop is currently suspended and waiting for
1155+
// data to resolve. Unschedule the current task.
1156+
//
1157+
// TODO: The factoring is a little weird. Arguably this should be checked
1158+
// in ensureRootIsScheduled instead. I went back and forth, not totally
1159+
// sure yet.
1160+
root.callbackPriority = NoLane;
1161+
root.callbackNode = null;
1162+
return null;
1163+
}
11341164
return performConcurrentWorkOnRoot.bind(null, root);
11351165
}
11361166
return null;
@@ -1740,7 +1770,9 @@ function handleThrow(root, thrownValue): void {
17401770
// deprecate the old API in favor of `use`.
17411771
thrownValue = getSuspendedThenable();
17421772
workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
1743-
workInProgressSuspendedReason = SuspendedOnImmediate;
1773+
workInProgressSuspendedReason = shouldAttemptToSuspendUntilDataResolves()
1774+
? SuspendedOnData
1775+
: SuspendedOnImmediate;
17441776
} else {
17451777
// This is a regular error. If something earlier in the component already
17461778
// suspended, we must clear the thenable state to unblock the work loop.
@@ -1797,6 +1829,48 @@ function handleThrow(root, thrownValue): void {
17971829
}
17981830
}
17991831

1832+
function shouldAttemptToSuspendUntilDataResolves() {
1833+
// TODO: We should be able to move the
1834+
// renderDidSuspend/renderDidSuspendWithDelay logic into this function,
1835+
// instead of repeating it in the complete phase. Or something to that effect.
1836+
1837+
if (includesOnlyRetries(workInProgressRootRenderLanes)) {
1838+
// We can always wait during a retry.
1839+
return true;
1840+
}
1841+
1842+
// TODO: We should be able to remove the equivalent check in
1843+
// finishConcurrentRender, and rely just on this one.
1844+
if (includesOnlyTransitions(workInProgressRootRenderLanes)) {
1845+
const suspenseHandler = getSuspenseHandler();
1846+
if (suspenseHandler !== null && suspenseHandler.tag === SuspenseComponent) {
1847+
const currentSuspenseHandler = suspenseHandler.alternate;
1848+
const nextProps: SuspenseProps = suspenseHandler.memoizedProps;
1849+
if (isBadSuspenseFallback(currentSuspenseHandler, nextProps)) {
1850+
// The nearest Suspense boundary is already showing content. We should
1851+
// avoid replacing it with a fallback, and instead wait until the
1852+
// data finishes loading.
1853+
return true;
1854+
} else {
1855+
// This is not a bad fallback condition. We should show a fallback
1856+
// immediately instead of waiting for the data to resolve. This includes
1857+
// when suspending inside new trees.
1858+
return false;
1859+
}
1860+
}
1861+
1862+
// During a transition, if there is no Suspense boundary (i.e. suspending in
1863+
// the "shell" of an application), or if we're inside a hidden tree, then
1864+
// we should wait until the data finishes loading.
1865+
return true;
1866+
}
1867+
1868+
// For all other Lanes besides Transitions and Retries, we should not wait
1869+
// for the data to load.
1870+
// TODO: We should wait during Offscreen prerendering, too.
1871+
return false;
1872+
}
1873+
18001874
function pushDispatcher(container) {
18011875
prepareRendererToRender(container);
18021876
const prevDispatcher = ReactCurrentDispatcher.current;
@@ -2061,7 +2135,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
20612135
markRenderStarted(lanes);
20622136
}
20632137

2064-
do {
2138+
outer: do {
20652139
try {
20662140
if (
20672141
workInProgressSuspendedReason !== NotSuspended &&
@@ -2071,19 +2145,48 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
20712145
// replay the suspended component.
20722146
const unitOfWork = workInProgress;
20732147
const thrownValue = workInProgressThrownValue;
2074-
workInProgressSuspendedReason = NotSuspended;
2075-
workInProgressThrownValue = null;
20762148
switch (workInProgressSuspendedReason) {
20772149
case SuspendedOnError: {
20782150
// Unwind then continue with the normal work loop.
2151+
workInProgressSuspendedReason = NotSuspended;
2152+
workInProgressThrownValue = null;
20792153
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
20802154
break;
20812155
}
2156+
case SuspendedOnData: {
2157+
const didResolve =
2158+
workInProgressSuspendedThenableState !== null &&
2159+
isThenableStateResolved(workInProgressSuspendedThenableState);
2160+
if (didResolve) {
2161+
workInProgressSuspendedReason = NotSuspended;
2162+
workInProgressThrownValue = null;
2163+
replaySuspendedUnitOfWork(unitOfWork, thrownValue);
2164+
} else {
2165+
// The work loop is suspended on data. We should wait for it to
2166+
// resolve before continuing to render.
2167+
const thenable: Thenable<mixed> = (workInProgressThrownValue: any);
2168+
const onResolution = () => {
2169+
ensureRootIsScheduled(root, now());
2170+
};
2171+
thenable.then(onResolution, onResolution);
2172+
break outer;
2173+
}
2174+
break;
2175+
}
2176+
case SuspendedOnImmediate: {
2177+
// If this fiber just suspended, it's possible the data is already
2178+
// cached. Yield to the main thread to give it a chance to ping. If
2179+
// it does, we can retry immediately without unwinding the stack.
2180+
workInProgressSuspendedReason = SuspendedAndReadyToUnwind;
2181+
break outer;
2182+
}
20822183
default: {
2083-
const wasPinged =
2184+
workInProgressSuspendedReason = NotSuspended;
2185+
workInProgressThrownValue = null;
2186+
const didResolve =
20842187
workInProgressSuspendedThenableState !== null &&
20852188
isThenableStateResolved(workInProgressSuspendedThenableState);
2086-
if (wasPinged) {
2189+
if (didResolve) {
20872190
replaySuspendedUnitOfWork(unitOfWork, thrownValue);
20882191
} else {
20892192
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
@@ -2097,12 +2200,6 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
20972200
break;
20982201
} catch (thrownValue) {
20992202
handleThrow(root, thrownValue);
2100-
if (workInProgressSuspendedThenableState !== null) {
2101-
// If this fiber just suspended, it's possible the data is already
2102-
// cached. Yield to the main thread to give it a chance to ping. If
2103-
// it does, we can retry immediately without unwinding the stack.
2104-
break;
2105-
}
21062203
}
21072204
} while (true);
21082205
resetContextDependencies();

0 commit comments

Comments
 (0)