Skip to content

[Flight] Remove back pointers to the Response from the Chunks #33620

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
Jun 23, 2025
Merged

[Flight] Remove back pointers to the Response from the Chunks #33620

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6808516
Only store Response on the ResolvedModelChunk
sebmarkbage Jun 23, 2025
70dcc16
Delete _response field and just use the reason instead
sebmarkbage Jun 23, 2025
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
73 changes: 34 additions & 39 deletions packages/react-client/src/ReactFlightClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ type PendingChunk<T> = {
status: 'pending',
value: null | Array<(T) => mixed>,
reason: null | Array<(mixed) => mixed>,
_response: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (T) => mixed, reject?: (mixed) => mixed): void,
Expand All @@ -174,16 +173,14 @@ type BlockedChunk<T> = {
status: 'blocked',
value: null | Array<(T) => mixed>,
reason: null | Array<(mixed) => mixed>,
_response: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (T) => mixed, reject?: (mixed) => mixed): void,
};
type ResolvedModelChunk<T> = {
status: 'resolved_model',
value: UninitializedModel,
reason: null,
_response: Response,
reason: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (T) => mixed, reject?: (mixed) => mixed): void,
Expand All @@ -192,7 +189,6 @@ type ResolvedModuleChunk<T> = {
status: 'resolved_module',
value: ClientReference<T>,
reason: null,
_response: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (T) => mixed, reject?: (mixed) => mixed): void,
Expand All @@ -201,7 +197,6 @@ type InitializedChunk<T> = {
status: 'fulfilled',
value: T,
reason: null | FlightStreamController,
_response: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (T) => mixed, reject?: (mixed) => mixed): void,
Expand All @@ -212,7 +207,6 @@ type InitializedStreamChunk<
status: 'fulfilled',
value: T,
reason: FlightStreamController,
_response: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (ReadableStream) => mixed, reject?: (mixed) => mixed): void,
Expand All @@ -221,7 +215,6 @@ type ErroredChunk<T> = {
status: 'rejected',
value: null,
reason: mixed,
_response: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (T) => mixed, reject?: (mixed) => mixed): void,
Expand All @@ -230,7 +223,6 @@ type HaltedChunk<T> = {
status: 'halted',
value: null,
reason: null,
_response: Response,
_children: Array<SomeChunk<any>> | ProfilingResult, // Profiling-only
_debugInfo?: null | ReactDebugInfo, // DEV-only
then(resolve: (T) => mixed, reject?: (mixed) => mixed): void,
Expand All @@ -245,16 +237,10 @@ type SomeChunk<T> =
| HaltedChunk<T>;

// $FlowFixMe[missing-this-annot]
function ReactPromise(
status: any,
value: any,
reason: any,
response: Response,
) {
function ReactPromise(status: any, value: any, reason: any) {
this.status = status;
this.value = value;
this.reason = reason;
this._response = response;
if (enableProfilerTimer && enableComponentPerformanceTrack) {
this._children = [];
}
Expand Down Expand Up @@ -401,20 +387,20 @@ export function getRoot<T>(response: Response): Thenable<T> {

function createPendingChunk<T>(response: Response): PendingChunk<T> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(PENDING, null, null, response);
return new ReactPromise(PENDING, null, null);
}

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

function createErrorChunk<T>(
response: Response,
error: mixed,
): ErroredChunk<T> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(ERRORED, null, error, response);
return new ReactPromise(ERRORED, null, error);
}

function wakeChunk<T>(listeners: Array<(T) => mixed>, value: T): void {
Expand Down Expand Up @@ -486,31 +472,31 @@ function createResolvedModelChunk<T>(
value: UninitializedModel,
): ResolvedModelChunk<T> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(RESOLVED_MODEL, value, null, response);
return new ReactPromise(RESOLVED_MODEL, value, response);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment like we have for createInitializedStreamChunk would help avoid confusion:

// We use the reason field to stash the controller since we already have that
// field. It's a bit of a hack but efficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all over every access in this file and indeed every Fiber field too. At some point you just got to add a doc for the whole repo.

}

function createResolvedModuleChunk<T>(
response: Response,
value: ClientReference<T>,
): ResolvedModuleChunk<T> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(RESOLVED_MODULE, value, null, response);
return new ReactPromise(RESOLVED_MODULE, value, null);
}

function createInitializedTextChunk(
response: Response,
value: string,
): InitializedChunk<string> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(INITIALIZED, value, null, response);
return new ReactPromise(INITIALIZED, value, null);
}

function createInitializedBufferChunk(
response: Response,
value: $ArrayBufferView | ArrayBuffer,
): InitializedChunk<Uint8Array> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(INITIALIZED, value, null, response);
return new ReactPromise(INITIALIZED, value, null);
}

function createInitializedIteratorResultChunk<T>(
Expand All @@ -519,12 +505,7 @@ function createInitializedIteratorResultChunk<T>(
done: boolean,
): InitializedChunk<IteratorResult<T, T>> {
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(
INITIALIZED,
{done: done, value: value},
null,
response,
);
return new ReactPromise(INITIALIZED, {done: done, value: value}, null);
}

function createInitializedStreamChunk<
Expand All @@ -537,7 +518,7 @@ function createInitializedStreamChunk<
// We use the reason field to stash the controller since we already have that
// field. It's a bit of a hack but efficient.
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(INITIALIZED, value, controller, response);
return new ReactPromise(INITIALIZED, value, controller);
}

function createResolvedIteratorResultChunk<T>(
Expand All @@ -549,21 +530,23 @@ function createResolvedIteratorResultChunk<T>(
const iteratorResultJSON =
(done ? '{"done":true,"value":' : '{"done":false,"value":') + value + '}';
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new ReactPromise(RESOLVED_MODEL, iteratorResultJSON, null, response);
return new ReactPromise(RESOLVED_MODEL, iteratorResultJSON, response);
}

function resolveIteratorResultChunk<T>(
response: Response,
chunk: SomeChunk<IteratorResult<T, T>>,
value: UninitializedModel,
done: boolean,
): void {
// To reuse code as much code as possible we add the wrapper element as part of the JSON.
const iteratorResultJSON =
(done ? '{"done":true,"value":' : '{"done":false,"value":') + value + '}';
resolveModelChunk(chunk, iteratorResultJSON);
resolveModelChunk(response, chunk, iteratorResultJSON);
}

function resolveModelChunk<T>(
response: Response,
chunk: SomeChunk<T>,
value: UninitializedModel,
): void {
Expand All @@ -580,6 +563,7 @@ function resolveModelChunk<T>(
const resolvedChunk: ResolvedModelChunk<T> = (chunk: any);
resolvedChunk.status = RESOLVED_MODEL;
resolvedChunk.value = value;
resolvedChunk.reason = response;
if (resolveListeners !== null) {
// This is unfortunate that we're reading this eagerly if
// we already have listeners attached since they might no
Expand Down Expand Up @@ -625,6 +609,7 @@ function initializeModelChunk<T>(chunk: ResolvedModelChunk<T>): void {
initializingHandler = null;

const resolvedModel = chunk.value;
const response = chunk.reason;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I hate it. ... I vote for evil, by the way. 😉


// We go to the BLOCKED state until we've fully resolved this.
// We do this before parsing in case we try to initialize the same chunk
Expand All @@ -639,7 +624,7 @@ function initializeModelChunk<T>(chunk: ResolvedModelChunk<T>): void {
}

try {
const value: T = parseModel(chunk._response, resolvedModel);
const value: T = parseModel(response, resolvedModel);
// Invoke any listeners added while resolving this model. I.e. cyclic
// references. This may or may not fully resolve the model depending on
// if they were blocked.
Expand Down Expand Up @@ -1862,7 +1847,7 @@ function resolveModel(
if (!chunk) {
chunks.set(id, createResolvedModelChunk(response, model));
} else {
resolveModelChunk(chunk, model);
resolveModelChunk(response, chunk, model);
}
}

Expand Down Expand Up @@ -2036,7 +2021,7 @@ function startReadableStream<T>(
// to synchronous emitting.
previousBlockedChunk = null;
}
resolveModelChunk(chunk, json);
resolveModelChunk(response, chunk, json);
});
}
},
Expand Down Expand Up @@ -2124,7 +2109,12 @@ function startAsyncIterable<T>(
false,
);
} else {
resolveIteratorResultChunk(buffer[nextWriteIndex], value, false);
resolveIteratorResultChunk(
response,
buffer[nextWriteIndex],
value,
false,
);
}
nextWriteIndex++;
},
Expand All @@ -2137,12 +2127,18 @@ function startAsyncIterable<T>(
true,
);
} else {
resolveIteratorResultChunk(buffer[nextWriteIndex], value, true);
resolveIteratorResultChunk(
response,
buffer[nextWriteIndex],
value,
true,
);
}
nextWriteIndex++;
while (nextWriteIndex < buffer.length) {
// In generators, any extra reads from the iterator have the value undefined.
resolveIteratorResultChunk(
response,
buffer[nextWriteIndex++],
'"$undefined"',
true,
Expand Down Expand Up @@ -2178,7 +2174,6 @@ function startAsyncIterable<T>(
INITIALIZED,
{done: true, value: undefined},
null,
response,
);
}
buffer[nextReadIndex] =
Expand Down Expand Up @@ -2946,7 +2941,7 @@ function resolveIOInfo(
chunks.set(id, chunk);
initializeModelChunk(chunk);
} else {
resolveModelChunk(chunk, model);
resolveModelChunk(response, chunk, model);
if (chunk.status === RESOLVED_MODEL) {
initializeModelChunk(chunk);
}
Expand Down
Loading