Skip to content

Commit

Permalink
The implementation ended up not merging the boundary resource concept…
Browse files Browse the repository at this point in the history
… with hoistable state due to the very different nature in which these things need to hoist (one during task completion and the other during flush but only when flushing late boundaries). Given that I've gone back to resource specific naming rather than calling it BoundaryState.
  • Loading branch information
gnoff committed Nov 27, 2023
1 parent e4f6980 commit 27aa27a
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 68 deletions.
61 changes: 30 additions & 31 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,6 @@ export function createRenderState(

nonce,
// like a module global for currently rendering boundary
boundaryState: null,
stylesToHoist: false,
};

Expand Down Expand Up @@ -2252,7 +2251,7 @@ function pushLink(
props: Object,
resumableState: ResumableState,
renderState: RenderState,
boundaryState: null | BoundaryState,
boundaryResources: null | BoundaryResources,
hoistableState: HoistableState,
textEmbedded: boolean,
insertionMode: InsertionMode,
Expand Down Expand Up @@ -2374,8 +2373,8 @@ function pushLink(
// We add the newly created resource to our StyleQueue and if necessary
// track the resource with the currently rendering boundary
styleQueue.sheets.set(key, resource);
if (boundaryState) {
boundaryState.stylesheets.add(resource);
if (boundaryResources) {
boundaryResources.stylesheets.add(resource);
}
} else {
// We need to track whether this boundary should wait on this resource or not.
Expand All @@ -2386,8 +2385,8 @@ function pushLink(
if (styleQueue) {
const resource = styleQueue.sheets.get(key);
if (resource) {
if (boundaryState) {
boundaryState.stylesheets.add(resource);
if (boundaryResources) {
boundaryResources.stylesheets.add(resource);
}
}
}
Expand Down Expand Up @@ -2454,7 +2453,7 @@ function pushStyle(
props: Object,
resumableState: ResumableState,
renderState: RenderState,
boundaryState: null | BoundaryState,
boundaryResources: null | BoundaryResources,
textEmbedded: boolean,
insertionMode: InsertionMode,
noscriptTagInScope: boolean,
Expand Down Expand Up @@ -2554,8 +2553,8 @@ function pushStyle(
// it. However, it's possible when you resume that the style has already been emitted
// and then it wouldn't be recreated in the RenderState and there's no need to track
// it again since we should've hoisted it to the shell already.
if (boundaryState) {
boundaryState.styles.add(styleQueue);
if (boundaryResources) {
boundaryResources.styles.add(styleQueue);
}
}

Expand Down Expand Up @@ -3455,7 +3454,7 @@ export function pushStartInstance(
props: Object,
resumableState: ResumableState,
renderState: RenderState,
boundaryState: null | BoundaryState,
boundaryResources: null | BoundaryResources,
hoistableState: HoistableState,
formatContext: FormatContext,
textEmbedded: boolean,
Expand Down Expand Up @@ -3535,7 +3534,7 @@ export function pushStartInstance(
props,
resumableState,
renderState,
boundaryState,
boundaryResources,
hoistableState,
textEmbedded,
formatContext.insertionMode,
Expand All @@ -3559,7 +3558,7 @@ export function pushStartInstance(
props,
resumableState,
renderState,
boundaryState,
boundaryResources,
textEmbedded,
formatContext.insertionMode,
!!(formatContext.tagScope & NOSCRIPT_SCOPE),
Expand Down Expand Up @@ -4108,7 +4107,7 @@ export function writeCompletedBoundaryInstruction(
resumableState: ResumableState,
renderState: RenderState,
id: number,
boundaryResources: BoundaryState,
boundaryResources: BoundaryResources,
): boolean {
let requiresStyleInsertion;
if (enableFloat) {
Expand Down Expand Up @@ -4439,7 +4438,7 @@ function hasStylesToHoist(stylesheet: StylesheetResource): boolean {

export function writeResourcesForBoundary(
destination: Destination,
boundaryResources: BoundaryState,
boundaryResources: BoundaryResources,
renderState: RenderState,
): boolean {
// Reset these on each invocation, they are only safe to read in this function
Expand Down Expand Up @@ -4739,7 +4738,7 @@ const arrayCloseBracket = stringToPrecomputedChunk(']');
// [["JS_escaped_string1", "JS_escaped_string2"]]
function writeStyleResourceDependenciesInJS(
destination: Destination,
boundaryResources: BoundaryState,
boundaryResources: BoundaryResources,
): void {
writeChunk(destination, arrayFirstOpenBracket);

Expand Down Expand Up @@ -4932,7 +4931,7 @@ function writeStyleResourceAttributeInJS(
// [["JSON_escaped_string1", "JSON_escaped_string2"]]
function writeStyleResourceDependenciesInAttr(
destination: Destination,
boundaryResources: BoundaryState,
boundaryResources: BoundaryResources,
): void {
writeChunk(destination, arrayFirstOpenBracket);

Expand Down Expand Up @@ -5198,7 +5197,7 @@ export function createHoistableState(): HoistableState {
};
}

export type BoundaryState = {
export type BoundaryResources = {
// style dependencies
styles: Set<StyleQueue>,
stylesheets: Set<StylesheetResource>,
Expand All @@ -5211,7 +5210,7 @@ export type StyleQueue = {
sheets: Map<string, StylesheetResource>,
};

export function createBoundaryState(): BoundaryState {
export function createBoundaryResources(): BoundaryResources {
return {
styles: new Set(),
stylesheets: new Set(),
Expand Down Expand Up @@ -6064,35 +6063,35 @@ function escapeStringForLinkHeaderQuotedParamValueContextReplacer(
}
}

export function hoistHoistableState(
parentHoistableState: HoistableState,
hoistableState: HoistableState,
export function hoistHoistables(
target: HoistableState,
source: HoistableState,
) {
parentHoistableState.charset.push(...hoistableState.charset);
parentHoistableState.viewport.push(...hoistableState.viewport);
parentHoistableState.chunks.push(...hoistableState.chunks);
target.charset.push(...source.charset);
target.viewport.push(...source.viewport);
target.chunks.push(...source.chunks);
}

function hoistStyleQueueDependency(
this: BoundaryState,
this: BoundaryResources,
styleQueue: StyleQueue,
) {
this.styles.add(styleQueue);
}

function hoistStylesheetDependency(
this: BoundaryState,
this: BoundaryResources,
stylesheet: StylesheetResource,
) {
this.stylesheets.add(stylesheet);
}

export function hoistBoundaryState(
targetState: BoundaryState,
sourceState: BoundaryState,
export function hoistBoundaryResources(
target: BoundaryResources,
source: BoundaryResources,
): void {
sourceState.styles.forEach(hoistStyleQueueDependency, targetState);
sourceState.stylesheets.forEach(hoistStylesheetDependency, targetState);
source.styles.forEach(hoistStyleQueueDependency, target);
source.stylesheets.forEach(hoistStylesheetDependency, target);
}

// This function is called at various times depending on whether we are rendering
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ export const doctypeChunk: PrecomputedChunk = stringToPrecomputedChunk('');

export type {
ResumableState,
BoundaryState,
HoistableState,
BoundaryResources,
FormatContext,
} from './ReactFizzConfigDOM';

Expand All @@ -152,13 +153,13 @@ export {
writeCompletedRoot,
createRootFormatContext,
createResumableState,
createBoundaryState,
createBoundaryResources,
createHoistableState,
writePreamble,
writeHoistables,
writePostamble,
hoistBoundaryState,
hoistHoistableState,
hoistBoundaryResources,
hoistHoistables,
prepareHostDispatcher,
resetResumableState,
completeResumableState,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-noop-renderer/src/ReactNoopServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ type Destination = {
};

type RenderState = null;
type BoundaryState = null;
type HoistableState = null;
type BoundaryResources = null;

const POP = Buffer.from('/', 'utf8');

Expand Down Expand Up @@ -266,15 +266,15 @@ const ReactNoopServer = ReactFizzServer({
writeHoistables() {},
writePostamble() {},

createBoundaryState(): BoundaryState {
createBoundaryResources(): BoundaryResources {
return null;
},

createHoistableState(): HoistableState {
return null;
},

hoistHoistableState(
hoistHoistables(
parentHoistableState: HoistableState,
hoistableState: HoistableState,
) {},
Expand Down
Loading

0 comments on commit 27aa27a

Please sign in to comment.