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

Jump to bottom

[Fizz] Gate legacyContext field on disableLegacyContext #30173

Merged
merged 1 commit into from
Jul 1, 2024
Merged

[Fizz] Gate legacyContext field on disableLegacyContext #30173

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
58 changes: 36 additions & 22 deletions packages/react-server/src/ReactFizzServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,12 @@ type RenderTask = {
abortSet: Set<Task>, // the abortable set that this task belongs to
keyPath: Root | KeyNode, // the path of all parent keys currently rendering
formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML)
legacyContext: LegacyContext, // the current legacy context that this task is executing in
context: ContextSnapshot, // the current new context that this task is executing in
treeContext: TreeContext, // the current tree context that this task is executing in
componentStack: null | ComponentStackNode, // stack frame description of the currently rendering component
thenableState: null | ThenableState,
isFallback: boolean, // whether this task is rendering inside a fallback tree
legacyContext: LegacyContext, // the current legacy context that this task is executing in
// DON'T ANY MORE FIELDS. We at 16 already which otherwise requires converting to a constructor.
// Consider splitting into multiple objects or consolidating some fields.
};
Expand All @@ -272,12 +272,12 @@ type ReplayTask = {
abortSet: Set<Task>, // the abortable set that this task belongs to
keyPath: Root | KeyNode, // the path of all parent keys currently rendering
formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML)
legacyContext: LegacyContext, // the current legacy context that this task is executing in
context: ContextSnapshot, // the current new context that this task is executing in
treeContext: TreeContext, // the current tree context that this task is executing in
componentStack: null | ComponentStackNode, // stack frame description of the currently rendering component
thenableState: null | ThenableState,
isFallback: boolean, // whether this task is rendering inside a fallback tree
legacyContext: LegacyContext, // the current legacy context that this task is executing in
// DON'T ANY MORE FIELDS. We at 16 already which otherwise requires converting to a constructor.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kassens Fastest way to construct an object in all VMs is inline objects. However only up to 16 fields. After that we need to switch to using a constructor instead to avoid V8 deopting them. These objects are not super rare so too many fields is not good anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Cheers, assumed it was something like this but didn't know the case or limit.

// Consider splitting into multiple objects or consolidating some fields.
};
Expand Down Expand Up @@ -462,11 +462,11 @@ function RequestInstance(
abortSet,
null,
rootFormatContext,
emptyContextObject,
rootContextSnapshot,
emptyTreeContext,
null,
false,
emptyContextObject,
);
pingedTasks.push(rootTask);
}
Expand Down Expand Up @@ -604,11 +604,11 @@ export function resumeRequest(
abortSet,
null,
postponedState.rootFormatContext,
emptyContextObject,
rootContextSnapshot,
emptyTreeContext,
null,
false,
emptyContextObject,
);
pingedTasks.push(rootTask);
return request;
Expand All @@ -630,11 +630,11 @@ export function resumeRequest(
abortSet,
null,
postponedState.rootFormatContext,
emptyContextObject,
rootContextSnapshot,
emptyTreeContext,
null,
false,
emptyContextObject,
);
pingedTasks.push(rootTask);
return request;
Expand Down Expand Up @@ -698,19 +698,19 @@ function createRenderTask(
abortSet: Set<Task>,
keyPath: Root | KeyNode,
formatContext: FormatContext,
legacyContext: LegacyContext,
context: ContextSnapshot,
treeContext: TreeContext,
componentStack: null | ComponentStackNode,
isFallback: boolean,
legacyContext: LegacyContext,
): RenderTask {
request.allPendingTasks++;
if (blockedBoundary === null) {
request.pendingRootTasks++;
} else {
blockedBoundary.pendingTasks++;
}
const task: RenderTask = {
const task: RenderTask = ({
replay: null,
node,
childIndex,
Expand All @@ -721,13 +721,15 @@ function createRenderTask(
abortSet,
keyPath,
formatContext,
legacyContext,
context,
treeContext,
componentStack,
thenableState,
isFallback,
};
}: any);
if (!disableLegacyContext) {
task.legacyContext = legacyContext;
}
abortSet.add(task);
return task;
}
Expand All @@ -743,11 +745,11 @@ function createReplayTask(
abortSet: Set<Task>,
keyPath: Root | KeyNode,
formatContext: FormatContext,
legacyContext: LegacyContext,
context: ContextSnapshot,
treeContext: TreeContext,
componentStack: null | ComponentStackNode,
isFallback: boolean,
legacyContext: LegacyContext,
): ReplayTask {
request.allPendingTasks++;
if (blockedBoundary === null) {
Expand All @@ -756,7 +758,7 @@ function createReplayTask(
blockedBoundary.pendingTasks++;
}
replay.pendingTasks++;
const task: ReplayTask = {
const task: ReplayTask = ({
replay,
node,
childIndex,
Expand All @@ -767,13 +769,15 @@ function createReplayTask(
abortSet,
keyPath,
formatContext,
legacyContext,
context,
treeContext,
componentStack,
thenableState,
isFallback,
};
}: any);
if (!disableLegacyContext) {
task.legacyContext = legacyContext;
}
abortSet.add(task);
return task;
}
Expand Down Expand Up @@ -1188,13 +1192,13 @@ function renderSuspenseBoundary(
fallbackAbortSet,
fallbackKeyPath,
task.formatContext,
task.legacyContext,
task.context,
task.treeContext,
// This stack should be the Suspense boundary stack because while the fallback is actually a child segment
// of the parent boundary from a component standpoint the fallback is a child of the Suspense boundary itself
suspenseComponentStack,
true,
!disableLegacyContext ? task.legacyContext : emptyContextObject,
);
// TODO: This should be queued at a separate lower priority queue so that we only work
// on preparing fallbacks if we don't have any more main content to task on.
Expand Down Expand Up @@ -1328,13 +1332,13 @@ function replaySuspenseBoundary(
fallbackAbortSet,
fallbackKeyPath,
task.formatContext,
task.legacyContext,
task.context,
task.treeContext,
// This stack should be the Suspense boundary stack because while the fallback is actually a child segment
// of the parent boundary from a component standpoint the fallback is a child of the Suspense boundary itself
suspenseComponentStack,
true,
!disableLegacyContext ? task.legacyContext : emptyContextObject,
);
// TODO: This should be queued at a separate lower priority queue so that we only work
// on preparing fallbacks if we don't have any more main content to task on.
Expand Down Expand Up @@ -3271,13 +3275,13 @@ function spawnNewSuspendedReplayTask(
task.abortSet,
task.keyPath,
task.formatContext,
task.legacyContext,
task.context,
task.treeContext,
// We pop one task off the stack because the node that suspended will be tried again,
// which will add it back onto the stack.
task.componentStack !== null ? task.componentStack.parent : null,
task.isFallback,
!disableLegacyContext ? task.legacyContext : emptyContextObject,
);

const ping = newTask.ping;
Expand Down Expand Up @@ -3317,13 +3321,13 @@ function spawnNewSuspendedRenderTask(
task.abortSet,
task.keyPath,
task.formatContext,
task.legacyContext,
task.context,
task.treeContext,
// We pop one task off the stack because the node that suspended will be tried again,
// which will add it back onto the stack.
task.componentStack !== null ? task.componentStack.parent : null,
task.isFallback,
!disableLegacyContext ? task.legacyContext : emptyContextObject,
);

const ping = newTask.ping;
Expand All @@ -3341,7 +3345,9 @@ function renderNode(
// Snapshot the current context in case something throws to interrupt the
// process.
const previousFormatContext = task.formatContext;
const previousLegacyContext = task.legacyContext;
const previousLegacyContext = !disableLegacyContext
? task.legacyContext
: emptyContextObject;
const previousContext = task.context;
const previousKeyPath = task.keyPath;
const previousTreeContext = task.treeContext;
Expand Down Expand Up @@ -3383,7 +3389,9 @@ function renderNode(
// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
task.legacyContext = previousLegacyContext;
if (!disableLegacyContext) {
task.legacyContext = previousLegacyContext;
}
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
Expand Down Expand Up @@ -3435,7 +3443,9 @@ function renderNode(
// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
task.legacyContext = previousLegacyContext;
if (!disableLegacyContext) {
task.legacyContext = previousLegacyContext;
}
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
Expand Down Expand Up @@ -3469,7 +3479,9 @@ function renderNode(
// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
task.legacyContext = previousLegacyContext;
if (!disableLegacyContext) {
task.legacyContext = previousLegacyContext;
}
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
Expand All @@ -3485,7 +3497,9 @@ function renderNode(
// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
task.legacyContext = previousLegacyContext;
if (!disableLegacyContext) {
task.legacyContext = previousLegacyContext;
}
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
Expand Down