Skip to content

Commit 438ce50

Browse files
acdliteacdlite
committed
Yield to main thread whenever a fiber suspends
When a fiber suspends, we should yield to the main thread in case the data is already cached, to unblock a potential ping event. By itself, this commit isn't useful because we don't do anything special in the case where to do receive an immediate ping event. I've split this out only to demonstrate that it doesn't break any existing behavior. See the next commit for full context and motivation.
1 parent 2eac58a commit 438ce50

File tree

6 files changed

+66
-40
lines changed

6 files changed

+66
-40
lines changed

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ function throwException(
357357
sourceFiber: Fiber,
358358
value: mixed,
359359
rootRenderLanes: Lanes,
360-
) {
360+
): Wakeable | null {
361361
// The source fiber did not complete.
362362
sourceFiber.flags |= Incomplete;
363363

@@ -459,7 +459,7 @@ function throwException(
459459
if (suspenseBoundary.mode & ConcurrentMode) {
460460
attachPingListener(root, wakeable, rootRenderLanes);
461461
}
462-
return;
462+
return wakeable;
463463
} else {
464464
// No boundary was found. Unless this is a sync update, this is OK.
465465
// We can suspend and wait for more data to arrive.
@@ -474,7 +474,7 @@ function throwException(
474474
// This case also applies to initial hydration.
475475
attachPingListener(root, wakeable, rootRenderLanes);
476476
renderDidSuspendDelayIfPossible();
477-
return;
477+
return wakeable;
478478
}
479479

480480
// This is a sync/discrete update. We treat this case like an error
@@ -517,7 +517,7 @@ function throwException(
517517
// Even though the user may not be affected by this error, we should
518518
// still log it so it can be fixed.
519519
queueHydrationError(createCapturedValueAtFiber(value, sourceFiber));
520-
return;
520+
return null;
521521
}
522522
} else {
523523
// Otherwise, fall through to the error path.
@@ -540,7 +540,7 @@ function throwException(
540540
workInProgress.lanes = mergeLanes(workInProgress.lanes, lane);
541541
const update = createRootErrorUpdate(workInProgress, errorInfo, lane);
542542
enqueueCapturedUpdate(workInProgress, update);
543-
return;
543+
return null;
544544
}
545545
case ClassComponent:
546546
// Capture and retry
@@ -564,14 +564,15 @@ function throwException(
564564
lane,
565565
);
566566
enqueueCapturedUpdate(workInProgress, update);
567-
return;
567+
return null;
568568
}
569569
break;
570570
default:
571571
break;
572572
}
573573
workInProgress = workInProgress.return;
574574
} while (workInProgress !== null);
575+
return null;
575576
}
576577

577578
export {throwException, createRootErrorUpdate, createClassErrorUpdate};

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ function throwException(
357357
sourceFiber: Fiber,
358358
value: mixed,
359359
rootRenderLanes: Lanes,
360-
) {
360+
): Wakeable | null {
361361
// The source fiber did not complete.
362362
sourceFiber.flags |= Incomplete;
363363

@@ -459,7 +459,7 @@ function throwException(
459459
if (suspenseBoundary.mode & ConcurrentMode) {
460460
attachPingListener(root, wakeable, rootRenderLanes);
461461
}
462-
return;
462+
return wakeable;
463463
} else {
464464
// No boundary was found. Unless this is a sync update, this is OK.
465465
// We can suspend and wait for more data to arrive.
@@ -474,7 +474,7 @@ function throwException(
474474
// This case also applies to initial hydration.
475475
attachPingListener(root, wakeable, rootRenderLanes);
476476
renderDidSuspendDelayIfPossible();
477-
return;
477+
return wakeable;
478478
}
479479

480480
// This is a sync/discrete update. We treat this case like an error
@@ -517,7 +517,7 @@ function throwException(
517517
// Even though the user may not be affected by this error, we should
518518
// still log it so it can be fixed.
519519
queueHydrationError(createCapturedValueAtFiber(value, sourceFiber));
520-
return;
520+
return null;
521521
}
522522
} else {
523523
// Otherwise, fall through to the error path.
@@ -540,7 +540,7 @@ function throwException(
540540
workInProgress.lanes = mergeLanes(workInProgress.lanes, lane);
541541
const update = createRootErrorUpdate(workInProgress, errorInfo, lane);
542542
enqueueCapturedUpdate(workInProgress, update);
543-
return;
543+
return null;
544544
}
545545
case ClassComponent:
546546
// Capture and retry
@@ -564,14 +564,15 @@ function throwException(
564564
lane,
565565
);
566566
enqueueCapturedUpdate(workInProgress, update);
567-
return;
567+
return null;
568568
}
569569
break;
570570
default:
571571
break;
572572
}
573573
workInProgress = workInProgress.return;
574574
} while (workInProgress !== null);
575+
return null;
575576
}
576577

577578
export {throwException, createRootErrorUpdate, createClassErrorUpdate};

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,7 +1573,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
15731573
return rootWorkInProgress;
15741574
}
15751575

1576-
function handleError(root, thrownValue): void {
1576+
function handleError(root, thrownValue): Wakeable | null {
15771577
do {
15781578
let erroredWork = workInProgress;
15791579
try {
@@ -1599,7 +1599,7 @@ function handleError(root, thrownValue): void {
15991599
// intentionally not calling those, we need set it here.
16001600
// TODO: Consider calling `unwindWork` to pop the contexts.
16011601
workInProgress = null;
1602-
return;
1602+
return null;
16031603
}
16041604

16051605
if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
@@ -1632,7 +1632,7 @@ function handleError(root, thrownValue): void {
16321632
}
16331633
}
16341634

1635-
throwException(
1635+
const maybeWakeable = throwException(
16361636
root,
16371637
erroredWork.return,
16381638
erroredWork,
@@ -1644,6 +1644,9 @@ function handleError(root, thrownValue): void {
16441644
// happens because of Suspense, but it also applies to errors. Think of it
16451645
// as suspending the execution of the work loop.
16461646
workInProgressIsSuspended = true;
1647+
1648+
// Return to the normal work loop.
1649+
return maybeWakeable;
16471650
} catch (yetAnotherThrownValue) {
16481651
// Something in the return path also threw.
16491652
thrownValue = yetAnotherThrownValue;
@@ -1657,8 +1660,6 @@ function handleError(root, thrownValue): void {
16571660
}
16581661
continue;
16591662
}
1660-
// Return to the normal work loop.
1661-
return;
16621663
} while (true);
16631664
}
16641665

@@ -1878,7 +1879,13 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
18781879
workLoopConcurrent();
18791880
break;
18801881
} catch (thrownValue) {
1881-
handleError(root, thrownValue);
1882+
const maybeWakeable = handleError(root, thrownValue);
1883+
if (maybeWakeable !== null) {
1884+
// If this fiber just suspended, it's possible the data is already
1885+
// cached. Yield to the the main thread to give it a chance to ping. If
1886+
// it does, we can retry immediately without unwinding the stack.
1887+
break;
1888+
}
18821889
}
18831890
} while (true);
18841891
resetContextDependencies();

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,7 +1573,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
15731573
return rootWorkInProgress;
15741574
}
15751575

1576-
function handleError(root, thrownValue): void {
1576+
function handleError(root, thrownValue): Wakeable | null {
15771577
do {
15781578
let erroredWork = workInProgress;
15791579
try {
@@ -1599,7 +1599,7 @@ function handleError(root, thrownValue): void {
15991599
// intentionally not calling those, we need set it here.
16001600
// TODO: Consider calling `unwindWork` to pop the contexts.
16011601
workInProgress = null;
1602-
return;
1602+
return null;
16031603
}
16041604

16051605
if (enableProfilerTimer && erroredWork.mode & ProfileMode) {
@@ -1632,7 +1632,7 @@ function handleError(root, thrownValue): void {
16321632
}
16331633
}
16341634

1635-
throwException(
1635+
const maybeWakeable = throwException(
16361636
root,
16371637
erroredWork.return,
16381638
erroredWork,
@@ -1644,6 +1644,9 @@ function handleError(root, thrownValue): void {
16441644
// happens because of Suspense, but it also applies to errors. Think of it
16451645
// as suspending the execution of the work loop.
16461646
workInProgressIsSuspended = true;
1647+
1648+
// Return to the normal work loop.
1649+
return maybeWakeable;
16471650
} catch (yetAnotherThrownValue) {
16481651
// Something in the return path also threw.
16491652
thrownValue = yetAnotherThrownValue;
@@ -1657,8 +1660,6 @@ function handleError(root, thrownValue): void {
16571660
}
16581661
continue;
16591662
}
1660-
// Return to the normal work loop.
1661-
return;
16621663
} while (true);
16631664
}
16641665

@@ -1878,7 +1879,13 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
18781879
workLoopConcurrent();
18791880
break;
18801881
} catch (thrownValue) {
1881-
handleError(root, thrownValue);
1882+
const maybeWakeable = handleError(root, thrownValue);
1883+
if (maybeWakeable !== null) {
1884+
// If this fiber just suspended, it's possible the data is already
1885+
// cached. Yield to the the main thread to give it a chance to ping. If
1886+
// it does, we can retry immediately without unwinding the stack.
1887+
break;
1888+
}
18821889
}
18831890
} while (true);
18841891
resetContextDependencies();

‎packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -485,22 +485,33 @@ describe('ReactOffscreen', () => {
485485
// In the same render, also hide the offscreen tree.
486486
root.render(<App show={false} />);
487487

488-
expect(Scheduler).toFlushUntilNextPaint([
489-
// The outer update will commit, but the inner update is deferred until
490-
// a later render.
491-
'Outer: 1',
492-
493-
// Something suspended. This means we won't commit immediately; there
494-
// will be an async gap between render and commit. In this test, we will
495-
// use this property to schedule a concurrent update. The fact that
496-
// we're using Suspense to schedule a concurrent update is not directly
497-
// relevant to the test — we could also use time slicing, but I've
498-
// chosen to use Suspense the because implementation details of time
499-
// slicing are more volatile.
500-
'Suspend! [Async: 1]',
488+
if (gate(flags => flags.enableSyncDefaultUpdates)) {
489+
expect(Scheduler).toFlushUntilNextPaint([
490+
// The outer update will commit, but the inner update is deferred until
491+
// a later render.
492+
'Outer: 1',
493+
494+
// Something suspended. This means we won't commit immediately; there
495+
// will be an async gap between render and commit. In this test, we will
496+
// use this property to schedule a concurrent update. The fact that
497+
// we're using Suspense to schedule a concurrent update is not directly
498+
// relevant to the test — we could also use time slicing, but I've
499+
// chosen to use Suspense the because implementation details of time
500+
// slicing are more volatile.
501+
'Suspend! [Async: 1]',
502+
503+
'Loading...',
504+
]);
505+
} else {
506+
// When default updates are time sliced, React yields before preparing
507+
// the fallback.
508+
expect(Scheduler).toFlushUntilNextPaint([
509+
'Outer: 1',
510+
'Suspend! [Async: 1]',
511+
]);
512+
expect(Scheduler).toFlushUntilNextPaint(['Loading...']);
513+
}
501514

502-
'Loading...',
503-
]);
504515
// Assert that we haven't committed quite yet
505516
expect(root).toMatchRenderedOutput(
506517
<>

‎packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3859,7 +3859,6 @@ describe('ReactSuspenseWithNoopRenderer', () => {
38593859
'Suspend! [A2]',
38603860
'Loading...',
38613861
'Suspend! [B2]',
3862-
'Loading...',
38633862
]);
38643863
expect(root).toMatchRenderedOutput(
38653864
<>

0 commit comments

Comments
 (0)