Skip to content

Unwrap sync resolved thenables without suspending #25615

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

Merged
merged 2 commits into from
Nov 2, 2022
Merged

Unwrap sync resolved thenables without suspending #25615

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35deb4d
Unwrap sync resolved thenables without suspending
acdlite Nov 2, 2022
9e09e09
Clean up code that's no longer needed
acdlite Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Clean up code that's no longer needed 
Synchronously resolved thenables are now unwrapped directly inside
`use`. So we don't need this extra logic in the work loop anymore.
  • Loading branch information
@acdlite
acdlite committed Nov 2, 2022
commit 9e09e0997955da52aca4046d11284bedf6407b49
36 changes: 10 additions & 26 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2016,37 +2016,21 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
workInProgressSuspendedReason !== NotSuspended &&
workInProgress !== null
) {
// The work loop is suspended. We need to either unwind the stack or
// replay the suspended component.
// The work loop is suspended. During a synchronous render, we don't
// yield to the main thread. Immediately unwind the stack. This will
// trigger either a fallback or an error boundary.
// TODO: For discrete and "default" updates (anything that's not
// flushSync), we want to wait for the microtasks the flush before
// unwinding. Will probably implement this using renderRootConcurrent,
// or merge renderRootSync and renderRootConcurrent into the same
// function and fork the behavior some other way.
const unitOfWork = workInProgress;
const thrownValue = workInProgressThrownValue;
workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);

// TODO: This check is only here to account for thenables that
// synchronously resolve. Otherwise we would always unwind when
// rendering with renderRootSync. (In the future, discrete updates will
// use renderRootConcurrent instead.) We should account for
// synchronously resolved thenables before hitting this path.
switch (workInProgressSuspendedReason) {
case SuspendedOnError: {
// Unwind then continue with the normal work loop.
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
break;
}
default: {
const wasPinged =
workInProgressSuspendedThenableState !== null &&
isThenableStateResolved(workInProgressSuspendedThenableState);
if (wasPinged) {
replaySuspendedUnitOfWork(unitOfWork, thrownValue);
} else {
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
}
// Continue with the normal work loop.
break;
}
}
// Continue with the normal work loop.
}
workLoopSync();
break;
Expand Down
36 changes: 10 additions & 26 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2016,37 +2016,21 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
workInProgressSuspendedReason !== NotSuspended &&
workInProgress !== null
) {
// The work loop is suspended. We need to either unwind the stack or
// replay the suspended component.
// The work loop is suspended. During a synchronous render, we don't
// yield to the main thread. Immediately unwind the stack. This will
// trigger either a fallback or an error boundary.
// TODO: For discrete and "default" updates (anything that's not
// flushSync), we want to wait for the microtasks the flush before
// unwinding. Will probably implement this using renderRootConcurrent,
// or merge renderRootSync and renderRootConcurrent into the same
// function and fork the behavior some other way.
const unitOfWork = workInProgress;
const thrownValue = workInProgressThrownValue;
workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);

// TODO: This check is only here to account for thenables that
// synchronously resolve. Otherwise we would always unwind when
// rendering with renderRootSync. (In the future, discrete updates will
// use renderRootConcurrent instead.) We should account for
// synchronously resolved thenables before hitting this path.
switch (workInProgressSuspendedReason) {
case SuspendedOnError: {
// Unwind then continue with the normal work loop.
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
break;
}
default: {
const wasPinged =
workInProgressSuspendedThenableState !== null &&
isThenableStateResolved(workInProgressSuspendedThenableState);
if (wasPinged) {
replaySuspendedUnitOfWork(unitOfWork, thrownValue);
} else {
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
}
// Continue with the normal work loop.
break;
}
}
// Continue with the normal work loop.
}
workLoopSync();
break;
Expand Down