Skip to content
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.

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

Move suspended render logic to ensureRootIsScheduled #26328

Merged
merged 1 commit into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 27 additions & 18 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ const SuspendedOnError: SuspendedReason = 1;
const SuspendedOnData: SuspendedReason = 2;
const SuspendedOnImmediate: SuspendedReason = 3;
const SuspendedOnDeprecatedThrowPromise: SuspendedReason = 4;
const SuspendedAndReadyToUnwind: SuspendedReason = 5;
const SuspendedAndReadyToContinue: SuspendedReason = 5;
const SuspendedOnHydration: SuspendedReason = 6;

// When this is true, the work-in-progress fiber just suspended (or errored) and
Expand Down Expand Up @@ -892,6 +892,18 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
return;
}

// If this root is currently suspended and waiting for data to resolve, don't
// schedule a task to render it. We'll either wait for a ping, or wait to
// receive an update.
if (
workInProgressSuspendedReason === SuspendedOnData &&
workInProgressRoot === root
) {
root.callbackPriority = NoLane;
root.callbackNode = null;
return;
}

// We use the highest priority lane to represent the priority of the callback.
const newCallbackPriority = getHighestPriorityLane(nextLanes);

Expand Down Expand Up @@ -1153,20 +1165,6 @@ function performConcurrentWorkOnRoot(
if (root.callbackNode === originalCallbackNode) {
// The task node scheduled for this root is the same one that's
// currently executed. Need to return a continuation.
if (
workInProgressSuspendedReason === SuspendedOnData &&
workInProgressRoot === root
) {
// Special case: The work loop is currently suspended and waiting for
// data to resolve. Unschedule the current task.
//
// TODO: The factoring is a little weird. Arguably this should be checked
// in ensureRootIsScheduled instead. I went back and forth, not totally
// sure yet.
root.callbackPriority = NoLane;
root.callbackNode = null;
return null;
}
return performConcurrentWorkOnRoot.bind(null, root);
}
return null;
Expand Down Expand Up @@ -1858,7 +1856,7 @@ function handleThrow(root: FiberRoot, thrownValue: any): void {
case SuspendedOnData:
case SuspendedOnImmediate:
case SuspendedOnDeprecatedThrowPromise:
case SuspendedAndReadyToUnwind: {
case SuspendedAndReadyToContinue: {
const wakeable: Wakeable = (thrownValue: any);
markComponentSuspended(
erroredWork,
Expand Down Expand Up @@ -2216,6 +2214,17 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
// `status` field, but if the promise already has a status, we won't
// have added a listener until right here.
const onResolution = () => {
// Check if the root is still suspended on this promise.
if (
workInProgressSuspendedReason === SuspendedOnData &&
workInProgressRoot === root
) {
// Mark the root as ready to continue rendering.
workInProgressSuspendedReason = SuspendedAndReadyToContinue;
}
// Ensure the root is scheduled. We should do this even if we're
// currently working on a different root, so that we resume
// rendering later.
ensureRootIsScheduled(root, now());
};
thenable.then(onResolution, onResolution);
Expand All @@ -2225,10 +2234,10 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
// If this fiber just suspended, it's possible the data is already
// cached. Yield to the main thread to give it a chance to ping. If
// it does, we can retry immediately without unwinding the stack.
workInProgressSuspendedReason = SuspendedAndReadyToUnwind;
workInProgressSuspendedReason = SuspendedAndReadyToContinue;
break outer;
}
case SuspendedAndReadyToUnwind: {
case SuspendedAndReadyToContinue: {
const thenable: Thenable<mixed> = (thrownValue: any);
if (isThenableResolved(thenable)) {
// The data resolved. Try rendering the component again.
Expand Down
41 changes: 41 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactThenable-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,47 @@ describe('ReactThenable', () => {
assertLog(['Something different']);
});

// @gate enableUseHook
test('when waiting for data to resolve, an update on a different root does not cause work to be dropped', async () => {
const getCachedAsyncText = cache(getAsyncText);

function App() {
return <Text text={use(getCachedAsyncText('Hi'))} />;
}

const root1 = ReactNoop.createRoot();
await act(async () => {
root1.render(<Suspense fallback={<Text text="Loading..." />} />);
});

// Start a transition on one root. It will suspend.
await act(async () => {
startTransition(() => {
root1.render(
<Suspense fallback={<Text text="Loading..." />}>
<App />
</Suspense>,
);
});
});
assertLog(['Async text requested [Hi]']);

// While we're waiting for the first root's data to resolve, a second
// root renders.
const root2 = ReactNoop.createRoot();
await act(async () => {
root2.render('Do re mi');
});
expect(root2).toMatchRenderedOutput('Do re mi');

// Once the first root's data is ready, we should finish its transition.
await act(async () => {
await resolveTextRequests('Hi');
});
assertLog(['Hi']);
expect(root1).toMatchRenderedOutput('Hi');
});

// @gate enableUseHook
test('while suspended, hooks cannot be called (i.e. current dispatcher is unset correctly)', async () => {
function App() {
Expand Down