Skip to content

Commit

Permalink
Updates the approach to emit hoistables eagerly but only if the hoist…
Browse files Browse the repository at this point in the history 
…able is not in a fallback tree. Effectively it makes hoistable elements deep in any fallback noops. The logic here is that fallbacks aren't hydrated, they're always either replaced by the server or client rendered. In either case the primary content should be replacing the fallback hositables and since hoistable elements are really just metadata the imperative need to transiently render them is not as well justified. This avoids a host of complexities around deleting hoistables from fallbacks when the boundary reveals it's content.

Along with this change I made it so primary content hositables flush eagerly and do not wait for their containing boundary to complete first. This is a progmatic solution to the problem of prerendering. when prerendering we assume that all transent state is entirely flushed in the static prelude however by holding back hoistables until the boundary is complete this is violated. We would either need to conditionally emit hoistables for incomplete boundaries when prerending or we would need to serialize the state of the hoistables to recover them on the resuem path. The arguments for holding them back are that it aligns with client hoistable semantics better and if the boundary never hydrates the hoistables will get orphaned. Both of these problems are not insignificant but are also not necessarily blockers and so this approach attempts to balance complexity over impact by emitting them eagerly.
  • Loading branch information
@gnoff
gnoff committed Jan 24, 2024
1 parent 852f521 commit 9a18ab4
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 256 deletions.
209 changes: 71 additions & 138 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2222,10 +2222,10 @@ function pushMeta(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
renderState: RenderState,
hoistableState: null | HoistableState,
textEmbedded: boolean,
insertionMode: InsertionMode,
noscriptTagInScope: boolean,
isFallback: boolean,
): null {
if (enableFloat) {
if (
Expand All @@ -2241,31 +2241,26 @@ function pushMeta(
target.push(textSeparator);
}

if (typeof props.charSet === 'string') {
return pushSelfClosing(
hoistableState
? hoistableState.charsetChunks
: renderState.charsetChunks,
props,
'meta',
);
if (isFallback) {
// Hoistable Elements for fallbacks are simply omitted. we don't want to emit them early
// because they are likely superceded by primary content and we want to avoid needing to clean
// them up when the primary content is ready. They are never hydrated on the client anyway because
// boundaries in fallback are awaited or client render, in either case there is never hydration
return null;
} else if (typeof props.charSet === 'string') {
// "charset" Should really be config and not picked up from tags however since this is
// the only way to embed the tag today we flush it on a special queue on the Request so it
// can go before everything else. Like viewport this means that the tag will escape it's
// parent container.
return pushSelfClosing(renderState.charsetChunks, props, 'meta');
} else if (props.name === 'viewport') {
// "viewport" isn't related to preconnect but it has the right priority
return pushSelfClosing(
hoistableState
? hoistableState.viewportChunks
: renderState.viewportChunks,
props,
'meta',
);
// "viewport" is flushed on the Request so it can go earlier that Float resources that
// might be affected by it. This means it can escape the boundary it is rendered within.
// This is a pragmatic solution to viewport being incredibly sensitive to document order
// without requiring all hoistables to be flushed too early.
return pushSelfClosing(renderState.viewportChunks, props, 'meta');
} else {
return pushSelfClosing(
hoistableState
? hoistableState.hoistableChunks
: renderState.hoistableChunks,
props,
'meta',
);
return pushSelfClosing(renderState.hoistableChunks, props, 'meta');
}
}
} else {
Expand All @@ -2282,6 +2277,7 @@ function pushLink(
textEmbedded: boolean,
insertionMode: InsertionMode,
noscriptTagInScope: boolean,
isFallback: boolean,
): null {
if (enableFloat) {
const rel = props.rel;
Expand Down Expand Up @@ -2437,10 +2433,15 @@ function pushLink(
target.push(textSeparator);
}

const hoistableChunks = hoistableState
? hoistableState.hoistableChunks
: renderState.hoistableChunks;
return pushLinkImpl(hoistableChunks, props);
if (isFallback) {
// Hoistable Elements for fallbacks are simply omitted. we don't want to emit them early
// because they are likely superceded by primary content and we want to avoid needing to clean
// them up when the primary content is ready. They are never hydrated on the client anyway because
// boundaries in fallback are awaited or client render, in either case there is never hydration
return null;
} else {
return pushLinkImpl(renderState.hoistableChunks, props);
}
}
} else {
return pushLinkImpl(target, props);
Expand Down Expand Up @@ -2894,9 +2895,9 @@ function pushTitle(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
renderState: RenderState,
hoistableState: null | HoistableState,
insertionMode: InsertionMode,
noscriptTagInScope: boolean,
isFallback: boolean,
): ReactNodeList {
if (__DEV__) {
if (hasOwnProperty.call(props, 'children')) {
Expand Down Expand Up @@ -2952,11 +2953,15 @@ function pushTitle(
!noscriptTagInScope &&
props.itemProp == null
) {
const hoistableTarget = hoistableState
? hoistableState.hoistableChunks
: renderState.hoistableChunks;
pushTitleImpl(hoistableTarget, props);
return null;
if (isFallback) {
// Hoistable Elements for fallbacks are simply omitted. we don't want to emit them early
// because they are likely superceded by primary content and we want to avoid needing to clean
// them up when the primary content is ready. They are never hydrated on the client anyway because
// boundaries in fallback are awaited or client render, in either case there is never hydration
return null;
} else {
pushTitleImpl(renderState.hoistableChunks, props);
}
} else {
return pushTitleImpl(target, props);
}
Expand Down Expand Up @@ -3490,6 +3495,7 @@ export function pushStartInstance(
hoistableState: null | HoistableState,
formatContext: FormatContext,
textEmbedded: boolean,
isFallback: boolean,
): ReactNodeList {
if (__DEV__) {
validateARIAProperties(type, props);
Expand Down Expand Up @@ -3556,9 +3562,9 @@ export function pushStartInstance(
target,
props,
renderState,
hoistableState,
formatContext.insertionMode,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
isFallback,
)
: pushStartTitle(target, props);
case 'link':
Expand All @@ -3571,6 +3577,7 @@ export function pushStartInstance(
textEmbedded,
formatContext.insertionMode,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
isFallback,
);
case 'script':
return enableFloat
Expand Down Expand Up @@ -3600,10 +3607,10 @@ export function pushStartInstance(
target,
props,
renderState,
hoistableState,
textEmbedded,
formatContext.insertionMode,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
isFallback,
);
// Newline eating tags
case 'listing':
Expand Down Expand Up @@ -4469,7 +4476,7 @@ function hasStylesToHoist(stylesheet: StylesheetResource): boolean {
return false;
}

export function writeHoistablesForPartialBoundary(
export function writeHoistablesForBoundary(
destination: Destination,
hoistableState: HoistableState,
renderState: RenderState,
Expand All @@ -4495,57 +4502,6 @@ export function writeHoistablesForPartialBoundary(
return destinationHasCapacity;
}

export function writeHoistablesForCompletedBoundary(
destination: Destination,
hoistableState: HoistableState,
renderState: RenderState,
): boolean {
// Reset these on each invocation, they are only safe to read in this function
currentlyRenderingBoundaryHasStylesToHoist = false;
destinationHasCapacity = true;

// Flush style tags for each precedence this boundary depends on
hoistableState.styles.forEach(flushStyleTagsLateForBoundary, destination);

// Determine if this boundary has stylesheets that need to be awaited upon completion
hoistableState.stylesheets.forEach(hasStylesToHoist);

// Flush Hoistable Elements
let i;
const charsetChunks = hoistableState.charsetChunks;
for (i = 0; i < charsetChunks.length - 1; i++) {
writeChunk(destination, charsetChunks[i]);
}
if (i < charsetChunks.length) {
destinationHasCapacity = writeChunkAndReturn(destination, charsetChunks[i]);
}
const viewportChunks = hoistableState.viewportChunks;
for (i = 0; i < viewportChunks.length - 1; i++) {
writeChunk(destination, charsetChunks[i]);
}
if (i < viewportChunks.length) {
destinationHasCapacity = writeChunkAndReturn(
destination,
viewportChunks[i],
);
}
const hoistableChunks = hoistableState.hoistableChunks;
for (i = 0; i < hoistableChunks.length - 1; i++) {
writeChunk(destination, hoistableChunks[i]);
}
if (i < hoistableChunks.length) {
destinationHasCapacity = writeChunkAndReturn(
destination,
hoistableChunks[i],
);
}

if (currentlyRenderingBoundaryHasStylesToHoist) {
renderState.stylesToHoist = true;
}
return destinationHasCapacity;
}

function flushResource(this: Destination, resource: Resource) {
for (let i = 0; i < resource.length; i++) {
writeChunk(this, resource[i]);
Expand Down Expand Up @@ -4741,26 +4697,35 @@ export function writePreamble(
}
hoistableChunks.length = 0;

// Flush closing head if necessary
if (htmlChunks && headChunks === null) {
// We have an <html> rendered but no <head> rendered. We however inserted
// a <head> up above so we need to emit the </head> now. This is safe because
// if the main content contained the </head> it would also have provided a
// <head>. This means that all the content inside <html> is either <body> or
// invalid HTML
// we have an <html> but we inserted an implicit <head> tag. We need
// to close it since the main content won't have it
writeChunk(destination, endChunkForTag('head'));
}
}

// This is an opportunity to write hoistables however in the current implemention
// the only hoistables that make sense to write here are Resources. Hoistable Elements
// would have already been written as part of the preamble or will be written as part
// of a boundary completion and thus don't need to be written here.
// We don't bother reporting backpressure at the moment because we expect to
// flush the entire preamble in a single pass. This probably should be modified
// in the future to be backpressure sensitive but that requires a larger refactor
// of the flushing code in Fizz.
export function writeHoistables(
destination: Destination,
resumableState: ResumableState,
renderState: RenderState,
): void {
let i = 0;

// Emit high priority Hoistables

// We omit charsetChunks because we have already sent the shell and if it wasn't
// already sent it is too late now.

const viewportChunks = renderState.viewportChunks;
for (i = 0; i < viewportChunks.length; i++) {
writeChunk(destination, viewportChunks[i]);
}
viewportChunks.length = 0;

renderState.preconnects.forEach(flushResource, destination);
renderState.preconnects.clear();

Expand All @@ -4787,6 +4752,13 @@ export function writeHoistables(

renderState.bulkPreloads.forEach(flushResource, destination);
renderState.bulkPreloads.clear();

// Write embedding hoistableChunks
const hoistableChunks = renderState.hoistableChunks;
for (i = 0; i < hoistableChunks.length; i++) {
writeChunk(destination, hoistableChunks[i]);
}
hoistableChunks.length = 0;
}

export function writePostamble(
Expand Down Expand Up @@ -5259,10 +5231,6 @@ type StylesheetResource = {
export type HoistableState = {
styles: Set<StyleQueue>,
stylesheets: Set<StylesheetResource>,
// Hoistable chunks
charsetChunks: Array<Chunk | PrecomputedChunk>,
viewportChunks: Array<Chunk | PrecomputedChunk>,
hoistableChunks: Array<Chunk | PrecomputedChunk>,
};

export type StyleQueue = {
Expand All @@ -5276,9 +5244,6 @@ export function createHoistableState(): HoistableState {
return {
styles: new Set(),
stylesheets: new Set(),
charsetChunks: [],
viewportChunks: [],
hoistableChunks: [],
};
}

Expand Down Expand Up @@ -6142,44 +6107,12 @@ function hoistStylesheetDependency(
this.stylesheets.add(stylesheet);
}

export function hoistToBoundary(
export function hoistHoistables(
parentState: HoistableState,
childState: HoistableState,
): void {
childState.styles.forEach(hoistStyleQueueDependency, parentState);
childState.stylesheets.forEach(hoistStylesheetDependency, parentState);
let i;
const charsetChunks = childState.charsetChunks;
for (i = 0; i < charsetChunks.length; i++) {
parentState.charsetChunks.push(charsetChunks[i]);
}
const viewportChunks = childState.viewportChunks;
for (i = 0; i < charsetChunks.length; i++) {
parentState.viewportChunks.push(viewportChunks[i]);
}
const hoistableChunks = childState.hoistableChunks;
for (i = 0; i < hoistableChunks.length; i++) {
parentState.hoistableChunks.push(hoistableChunks[i]);
}
}

export function hoistToRoot(
renderState: RenderState,
hoistableState: HoistableState,
): void {
let i;
const charsetChunks = hoistableState.charsetChunks;
for (i = 0; i < charsetChunks.length; i++) {
renderState.charsetChunks.push(charsetChunks[i]);
}
const viewportChunks = hoistableState.viewportChunks;
for (i = 0; i < charsetChunks.length; i++) {
renderState.viewportChunks.push(viewportChunks[i]);
}
const hoistableChunks = hoistableState.hoistableChunks;
for (i = 0; i < hoistableChunks.length; i++) {
renderState.hoistableChunks.push(hoistableChunks[i]);
}
}

// This function is called at various times depending on whether we are rendering
Expand Down
6 changes: 2 additions & 4 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ export {
writeClientRenderBoundaryInstruction,
writeStartPendingSuspenseBoundary,
writeEndPendingSuspenseBoundary,
writeHoistablesForPartialBoundary,
writeHoistablesForCompletedBoundary,
writeHoistablesForBoundary,
writePlaceholder,
writeCompletedRoot,
createRootFormatContext,
Expand All @@ -163,8 +162,7 @@ export {
writePreamble,
writeHoistables,
writePostamble,
hoistToBoundary,
hoistToRoot,
hoistHoistables,
prepareHostDispatcher,
resetResumableState,
completeResumableState,
Expand Down
Loading

0 comments on commit 9a18ab4

Please sign in to comment.