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

[Static][Fizz] Carry forward bootstrap config to resume if postponing in the shell #27672

Merged
merged 1 commit into from
Nov 8, 2023
Merged

[Static][Fizz] Carry forward bootstrap config to resume if postponing in the shell #27672

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
30 changes: 22 additions & 8 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,13 @@ export type ResumableState = {
nextFormID: number,
streamingFormat: StreamingFormat,

// We carry the bootstrap intializers in resumable state in case we postpone in the shell
// of a prerender. On resume we will reinitialize the bootstrap scripts if necessary.
// If we end up flushing the bootstrap scripts we void these on the resumable state
bootstrapScriptContent?: string | void,
bootstrapScripts?: $ReadOnlyArray<string | BootstrapScriptDescriptor> | void,
bootstrapModules?: $ReadOnlyArray<string | BootstrapScriptDescriptor> | void,

// state for script streaming format, unused if using external runtime / data
instructions: InstructionState,

Expand Down Expand Up @@ -349,9 +356,6 @@ const DEFAULT_HEADERS_CAPACITY_IN_UTF16_CODE_UNITS = 2000;
export function createRenderState(
resumableState: ResumableState,
nonce: string | void,
bootstrapScriptContent: string | void,
bootstrapScripts: $ReadOnlyArray<string | BootstrapScriptDescriptor> | void,
bootstrapModules: $ReadOnlyArray<string | BootstrapScriptDescriptor> | void,
externalRuntimeConfig: string | BootstrapScriptDescriptor | void,
importMap: ImportMap | void,
onHeaders: void | ((headers: HeadersDescriptor) => void),
Expand All @@ -367,6 +371,8 @@ export function createRenderState(

const bootstrapChunks: Array<Chunk | PrecomputedChunk> = [];
let externalRuntimeScript: null | ExternalRuntimeScript = null;
const {bootstrapScriptContent, bootstrapScripts, bootstrapModules} =
resumableState;
if (bootstrapScriptContent !== undefined) {
bootstrapChunks.push(
inlineScriptWithNonce,
Expand Down Expand Up @@ -612,9 +618,6 @@ export function resumeRenderState(
return createRenderState(
resumableState,
nonce,
// These should have already been flushed in the prerender.
undefined,
undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

You removed three options. How come only two are removed?

undefined,
undefined,
undefined,
Expand All @@ -625,6 +628,9 @@ export function resumeRenderState(
export function createResumableState(
identifierPrefix: string | void,
externalRuntimeConfig: string | BootstrapScriptDescriptor | void,
bootstrapScriptContent: string | void,
bootstrapScripts: $ReadOnlyArray<string | BootstrapScriptDescriptor> | void,
bootstrapModules: $ReadOnlyArray<string | BootstrapScriptDescriptor> | void,
): ResumableState {
const idPrefix = identifierPrefix === undefined ? '' : identifierPrefix;

Expand All @@ -638,6 +644,9 @@ export function createResumableState(
idPrefix: idPrefix,
nextFormID: 0,
streamingFormat,
bootstrapScriptContent,
bootstrapScripts,
bootstrapModules,
instructions: NothingSent,
hasBody: false,
hasHtml: false,
Expand Down Expand Up @@ -3714,7 +3723,11 @@ export function pushEndInstance(
function writeBootstrap(
destination: Destination,
renderState: RenderState,
resumableState: ResumableState,
): boolean {
resumableState.bootstrapScriptContent = undefined;
resumableState.bootstrapScripts = undefined;
resumableState.bootstrapModules = undefined;
const bootstrapChunks = renderState.bootstrapChunks;
let i = 0;
for (; i < bootstrapChunks.length - 1; i++) {
Expand All @@ -3731,8 +3744,9 @@ function writeBootstrap(
export function writeCompletedRoot(
destination: Destination,
renderState: RenderState,
resumableState: ResumableState,
): boolean {
return writeBootstrap(destination, renderState);
return writeBootstrap(destination, renderState, resumableState);
}

// Structural Nodes
Expand Down Expand Up @@ -4197,7 +4211,7 @@ export function writeCompletedBoundaryInstruction(
} else {
writeMore = writeChunkAndReturn(destination, completeBoundaryDataEnd);
}
return writeBootstrap(destination, renderState) && writeMore;
return writeBootstrap(destination, renderState, resumableState) && writeMore;
}

const clientRenderScript1Full = stringToPrecomputedChunk(
Expand Down
2 changes: 0 additions & 2 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ export function createRenderState(
undefined,
undefined,
undefined,
undefined,
undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here. Were we sending one less already?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I checked on those, I believe we were sending too many undefineds before. I confirmed the signature has six args and there are six inputs when 2 are removed

);
return {
// Keep this in sync with ReactFizzConfigDOM
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,16 @@ function renderToReadableStream(
const resumableState = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
const request = createRequest(
children,
resumableState,
createRenderState(
resumableState,
options ? options.nonce : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.importMap : undefined,
onHeadersImpl,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMFizzServerBun.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,16 @@ function renderToReadableStream(
const resumableState = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
const request = createRequest(
children,
resumableState,
createRenderState(
resumableState,
options ? options.nonce : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.importMap : undefined,
onHeadersImpl,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMFizzServerEdge.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,16 @@ function renderToReadableStream(
const resumableState = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
const request = createRequest(
children,
resumableState,
createRenderState(
resumableState,
options ? options.nonce : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.importMap : undefined,
onHeadersImpl,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMFizzServerNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,16 @@ function createRequestImpl(children: ReactNodeList, options: void | Options) {
const resumableState = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
return createRequest(
children,
resumableState,
createRenderState(
resumableState,
options ? options.nonce : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.importMap : undefined,
options ? options.onHeaders : undefined,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,16 @@ function prerender(
const resources = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
const request = createPrerenderRequest(
children,
resources,
createRenderState(
resources,
undefined, // nonce is not compatible with prerendered bootstrap scripts
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.importMap : undefined,
onHeadersImpl,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMFizzStaticEdge.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,16 @@ function prerender(
const resources = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
const request = createPrerenderRequest(
children,
resources,
createRenderState(
resources,
undefined, // nonce is not compatible with prerendered bootstrap scripts
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.importMap : undefined,
onHeadersImpl,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMFizzStaticNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,16 @@ function prerenderToNodeStream(
const resumableState = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
const request = createPrerenderRequest(
children,
resumableState,
createRenderState(
resumableState,
undefined, // nonce is not compatible with prerendered bootstrap scripts
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.importMap : undefined,
options ? options.onHeaders : undefined,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-server-dom-fb/src/ReactDOMServerFB.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@ function renderToStream(children: ReactNodeList, options: Options): Stream {
const resumableState = createResumableState(
options ? options.identifierPrefix : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
);
const request = createRequest(
children,
resumableState,
createRenderState(
resumableState,
undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
options ? options.unstable_externalRuntimeSrc : undefined,
),
createRootFormatContext(undefined),
Expand Down
6 changes: 5 additions & 1 deletion packages/react-server/src/ReactFizzServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3968,7 +3968,11 @@ function flushCompletedQueues(

flushSegment(request, destination, completedRootSegment);
request.completedRootSegment = null;
writeCompletedRoot(destination, request.renderState);
writeCompletedRoot(
destination,
request.renderState,
request.resumableState,
);
} else {
// We haven't flushed the root yet so we don't need to check any other branches further down
return;
Expand Down