Skip to content

Commit

Permalink
Treat blocked modules or models as a special status
Browse files Browse the repository at this point in the history
We currently loop over all chunks at the end to error them if they're
still pending. We shouldn't do this if they're pending because they're
blocked on an external resource like a module because the module might not
resolve before the Flight connection closes and that's not an error.

In an alternative solution I had a set that tracked pending chunks and
removed one at a time. While the loop at the end is faster it's more
work as we go.

I figured the extra status might also help debugging.

For modules we can probably assume no forward references, and the first
async module we can just use the promise as the chunk.

So we could probably get away with this only on models that are blocked by
modules.
  • Loading branch information
sebmarkbage committed Sep 14, 2022
1 parent c84f7d2 commit f6c0d7a
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 19 deletions.
59 changes: 43 additions & 16 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export type JSONValue =
| $ReadOnlyArray<JSONValue>;

const PENDING = 'pending';
const BLOCKED = 'blocked';
const RESOLVED_MODEL = 'resolved_model';
const RESOLVED_MODULE = 'resolved_module';
const INITIALIZED = 'fulfilled';
Expand All @@ -50,6 +51,13 @@ type PendingChunk<T> = {
_response: Response,
then(resolve: (T) => mixed, reject: (mixed) => mixed): void,
};
type BlockedChunk<T> = {
status: 'blocked',
value: null | Array<(T) => mixed>,
reason: null | Array<(mixed) => mixed>,
_response: Response,
then(resolve: (T) => mixed, reject: (mixed) => mixed): void,
};
type ResolvedModelChunk<T> = {
status: 'resolved_model',
value: UninitializedModel,
Expand Down Expand Up @@ -80,6 +88,7 @@ type ErroredChunk<T> = {
};
type SomeChunk<T> =
| PendingChunk<T>
| BlockedChunk<T>
| ResolvedModelChunk<T>
| ResolvedModuleChunk<T>
| InitializedChunk<T>
Expand Down Expand Up @@ -115,6 +124,7 @@ Chunk.prototype.then = function<T>(
resolve(chunk.value);
break;
case PENDING:
case BLOCKED:
if (resolve) {
if (chunk.value === null) {
chunk.value = [];
Expand Down Expand Up @@ -159,8 +169,9 @@ function readChunk<T>(chunk: SomeChunk<T>): T {
case INITIALIZED:
return chunk.value;
case PENDING:
case BLOCKED:
// eslint-disable-next-line no-throw-literal
throw (chunk: Thenable<T>);
throw ((chunk: any): Thenable<T>);
default:
throw chunk.reason;
}
Expand All @@ -177,6 +188,11 @@ function createPendingChunk<T>(response: Response): PendingChunk<T> {
return new Chunk(PENDING, null, null, response);
}

function createBlockedChunk<T>(response: Response): BlockedChunk<T> {
// $FlowFixMe Flow doesn't support functions as constructors
return new Chunk(BLOCKED, null, null, response);
}

function createErrorChunk<T>(
response: Response,
error: Error,
Expand Down Expand Up @@ -210,6 +226,7 @@ function wakeChunkIfInitialized<T>(
wakeChunk(resolveListeners, chunk.value);
break;
case PENDING:
case BLOCKED:
chunk.value = resolveListeners;
chunk.reason = rejectListeners;
break;
Expand All @@ -222,7 +239,7 @@ function wakeChunkIfInitialized<T>(
}

function triggerErrorOnChunk<T>(chunk: SomeChunk<T>, error: mixed): void {
if (chunk.status !== PENDING) {
if (chunk.status !== PENDING && chunk.status !== BLOCKED) {
// We already resolved. We didn't expect to see this.
return;
}
Expand Down Expand Up @@ -278,7 +295,7 @@ function resolveModuleChunk<T>(
chunk: SomeChunk<T>,
value: ModuleReference<T>,
): void {
if (chunk.status !== PENDING) {
if (chunk.status !== PENDING && chunk.status !== BLOCKED) {
// We already resolved. We didn't expect to see this.
return;
}
Expand Down Expand Up @@ -308,11 +325,11 @@ function initializeModelChunk<T>(chunk: ResolvedModelChunk<T>): void {
) {
initializingChunkBlockedModel.value = value;
// We discovered new dependencies on modules that are not yet resolved.
// We have to return to the PENDING state until they're resolved.
const pendingChunk: PendingChunk<T> = (chunk: any);
pendingChunk.status = PENDING;
pendingChunk.value = null;
pendingChunk.reason = null;
// We have to go the BLOCKED state until they're resolved.
const blockedChunk: BlockedChunk<T> = (chunk: any);
blockedChunk.status = BLOCKED;
blockedChunk.value = null;
blockedChunk.reason = null;
} else {
const initializedChunk: InitializedChunk<T> = (chunk: any);
initializedChunk.status = INITIALIZED;
Expand Down Expand Up @@ -348,7 +365,9 @@ export function reportGlobalError(response: Response, error: Error): void {
// If this chunk was already resolved or errored, it won't
// trigger an error but if it wasn't then we need to
// because we won't be getting any new data to resolve it.
triggerErrorOnChunk(chunk, error);
if (chunk.status === PENDING) {
triggerErrorOnChunk(chunk, error);
}
});
}

Expand Down Expand Up @@ -433,7 +452,7 @@ function createModelResolver<T>(
parentObject[key] = value;
blocked.deps--;
if (blocked.deps === 0) {
if (chunk.status !== PENDING) {
if (chunk.status !== BLOCKED) {
return;
}
const resolveListeners = chunk.value;
Expand Down Expand Up @@ -480,6 +499,7 @@ export function parseModelString(
case INITIALIZED:
return chunk.value;
case PENDING:
case BLOCKED:
const parentChunk = initializingChunk;
chunk.then(
createModelResolver(parentChunk, parentObject, key),
Expand Down Expand Up @@ -573,21 +593,28 @@ export function resolveModule(
// that we'll need them.
const promise = preloadModule(moduleReference);
if (promise) {
let pendingChunk;
let blockedChunk: BlockedChunk<any>;
if (!chunk) {
pendingChunk = createPendingChunk(response);
chunks.set(id, pendingChunk);
// Technically, we should just treat promise as the chunk in this
// case. Because it'll just behave as any other promise.
blockedChunk = createBlockedChunk(response);
chunks.set(id, blockedChunk);
} else {
pendingChunk = chunk;
// This can't actually happen because we don't have any forward
// references to modules.
blockedChunk = (chunk: any);
blockedChunk.status = BLOCKED;
}
promise.then(
() => resolveModuleChunk(pendingChunk, moduleReference),
error => triggerErrorOnChunk(pendingChunk, error),
() => resolveModuleChunk(blockedChunk, moduleReference),
error => triggerErrorOnChunk(blockedChunk, error),
);
} else {
if (!chunk) {
chunks.set(id, createResolvedModuleChunk(response, moduleReference));
} else {
// This can't actually happen because we don't have any forward
// references to modules.
resolveModuleChunk(chunk, moduleReference);
}
}
Expand Down
9 changes: 6 additions & 3 deletions packages/react-reconciler/src/ReactFiberWakeable.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,12 @@ export function trackSuspendedWakeable(wakeable: Wakeable) {
suspendedThenable = null;
break;
default: {
// TODO: 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.
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.
break;
}
const pendingThenable: PendingThenable<mixed> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
Expand Down

0 comments on commit f6c0d7a

Please sign in to comment.