-
Notifications
You must be signed in to change notification settings - Fork 47k
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.
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
[Fizz] Gate legacyContext field on disableLegacyContext #30173
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
b21603f
to
1d8abc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
running out of fields
what does that mean?
PR looks good though.
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
We're running out of fields and this one we can avoid at runtime in any modern builds.