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.

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

Jump to bottom

Revert yieldy behavior for non-use Suspense (in Flight, too) #25541

Closed
wants to merge 1 commit into from
Closed

Revert yieldy behavior for non-use Suspense (in Flight, too) #25541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 39 additions & 10 deletions packages/react-server/src/ReactFlightServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import type {
ServerContextJSONValue,
Wakeable,
Thenable,
PendingThenable,
FulfilledThenable,
RejectedThenable,
} from 'shared/ReactTypes';
import type {LazyComponent} from 'react/src/ReactLazy';

Expand Down Expand Up @@ -66,7 +69,6 @@ import {
getActiveContext,
rootContextSnapshot,
} from './ReactFlightNewContext';
import {trackSuspendedWakeable} from './ReactFlightThenable';

import {
REACT_ELEMENT_TYPE,
Expand Down Expand Up @@ -224,10 +226,44 @@ function readThenable<T>(thenable: Thenable<T>): T {
}

function createLazyWrapperAroundWakeable(wakeable: Wakeable) {
trackSuspendedWakeable(wakeable);
// This is a temporary fork of the `use` implementation until we accept
// promises everywhere.
const thenable: Thenable<mixed> = (wakeable: any);
switch (thenable.status) {
case 'fulfilled':
case 'rejected':
break;
default: {
if (typeof thenable.status === 'string') {
// Only instrument the thenable if the status if not defined. If
// it's defined, but an unknown value, assume it's been instrumented by
// some custom userspace implementation. We treat it as "pending".
break;
}
const pendingThenable: PendingThenable<mixed> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
fulfilledValue => {
if (thenable.status === 'pending') {
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
fulfilledThenable.status = 'fulfilled';
fulfilledThenable.value = fulfilledValue;
}
},
(error: mixed) => {
if (thenable.status === 'pending') {
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
rejectedThenable.status = 'rejected';
rejectedThenable.reason = error;
}
},
);
break;
}
}
const lazyType: LazyComponent<any, Thenable<any>> = {
$$typeof: REACT_LAZY_TYPE,
_payload: (wakeable: any),
_payload: thenable,
_init: readThenable,
};
return lazyType;
Expand Down Expand Up @@ -818,11 +854,7 @@ export function resolveModelToJSON(
);
const ping = newTask.ping;
x.then(ping, ping);

const wakeable: Wakeable = x;
trackSuspendedWakeable(wakeable);
newTask.thenableState = getThenableStateAfterSuspending();

return serializeByRefID(newTask.id);
} else {
// Something errored. We'll still send everything we have up until this point.
Expand Down Expand Up @@ -1146,9 +1178,6 @@ function retryTask(request: Request, task: Task): void {
// Something suspended again, let's pick it back up later.
const ping = task.ping;
x.then(ping, ping);

const wakeable: Wakeable = x;
trackSuspendedWakeable(wakeable);
task.thenableState = getThenableStateAfterSuspending();
return;
} else {
Expand Down
29 changes: 6 additions & 23 deletions packages/react-server/src/ReactFlightThenable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// instead of "Wakeable". Or some other more appropriate name.

import type {
Wakeable,
Thenable,
PendingThenable,
FulfilledThenable,
Expand All @@ -30,14 +29,12 @@ export function createThenableState(): ThenableState {
return [];
}

// TODO: Unify this with trackSuspendedThenable. It needs to support not only
// `use`, but async components, too.
export function trackSuspendedWakeable(wakeable: Wakeable) {
// If this wakeable isn't already a thenable, turn it into one now. Then,
// when we resume the work loop, we can check if its status is
// still pending.
// TODO: Get rid of the Wakeable type? It's superseded by UntrackedThenable.
const thenable: Thenable<mixed> = (wakeable: any);
export function trackUsedThenable<T>(
thenableState: ThenableState,
thenable: Thenable<T>,
index: number,
) {
thenableState[index] = thenable;

// We use an expando to track the status and result of a thenable so that we
// can synchronously unwrap the value. Think of this as an extension of the
Expand Down Expand Up @@ -84,20 +81,6 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
}
}

export function trackUsedThenable<T>(
thenableState: ThenableState,
thenable: Thenable<T>,
index: number,
) {
// This is only a separate function from trackSuspendedWakeable for symmetry
// with Fiber.
// TODO: Disallow throwing a thenable directly. It must go through `use` (or
// some equivalent for internal Suspense implementations). We can't do this in
// Fiber yet because it's a breaking change but we can do it in Server
// Components because Server Components aren't released yet.
thenableState[index] = thenable;
}

export function getPreviouslyUsedThenableAtIndex<T>(
thenableState: ThenableState | null,
index: number,
Expand Down