lib: add internal/primordials_staging module#63464
Conversation
|
Review requested:
|
nodejs-github-bot
added
lib / src
LiviaMedeiros
added
the
experimental
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #63464 +/- ##
==========================================
+ Coverage 90.05% 90.07% +0.02%
==========================================
Files 714 715 +1
Lines 225876 226046 +170
Branches 42737 42757 +20
==========================================
+ Hits 203408 203620 +212
+ Misses 14244 14203 -41
+ Partials 8224 8223 -1
🚀 New features to boost your workflow:
|
github-actions
Bot
removed
the
request-ci
There was a problem hiding this comment.
freeze_intrinsics also runs in pre-execution, so this wouldn't be necessary.
There was a problem hiding this comment.
Agreed. Still, i'd prefer it to stay for consistency, so it doesn't differ from linter suggestion and the internals never take builtins from globalThis directly.
Unless there is another reason to keep globalThis here.
| _init({ | ||
| AsyncDisposableStack, | ||
| DisposableStack, | ||
| Float16Array, | ||
| Intl, | ||
| Iterator, | ||
| ShadowRealm, | ||
| SharedArrayBuffer, | ||
| Temporal, | ||
| Uint8Array: { | ||
| fromBase64, | ||
| fromHex, | ||
| }, | ||
| WebAssembly, | ||
| }) { |
There was a problem hiding this comment.
This is only capturing the global object properties, not the object tree, so eg. Temporal.Instant.fromEpochNanoseconds(), new WebAssembly.Module() are still user-mutable.
There was a problem hiding this comment.
Right. There's intentionally no recursive object tree replication because realistically there's no need for that: nested objects that are actually used in internals can be added in follow-ups.
I'll update the docs and add some nested objects (e.g. TemporalInstant) to the current implementation.
Signed-off-by: LiviaMedeiros <livia@cirno.name>
LiviaMedeiros
force-pushed
the
internal-primordials-staging
branch
from
54db761 to
fcf2b27
Compare
4 participants
We have quite a few builtins that can not be included in primordials, usually because their existence is conditional (either they require experimental flag, or they can be disabled with
--no-flag).We have to retrieve such builtins from
globalThis, which is exposed to userland and is mutable by design.Additionally, in some modules that are part of early bootstrap we can not do top-level assignment synchronously, because the builtins are not defined yet. Because of that, we have to get them on demand from userland, making them extremely prone to prior mutation.
This PR adds
internal/primordials_stagingmodule which is populated on pre-execution stage (late enough to have them on global, and early enough to not be affected by userland code), and allows lazy loading of experimental builtins without relying onglobalThisin runtime.The name is subject to bikeshedding. I chose
stagingsince objects in this module are expected to be included inprimordialsat some point in the future.Some related PRs:
Buffer.fromHex()andBuffer.fromBase64()#61157 - would benefit from this, otherwise it has to lazy load mutable/polyfillable methods in runtime.Temporal.Instantsupport toStatsandBigIntStats#60789 - would be hardened by this, otherwise it might lazyload polyfilled/mutatedTemporal.Temporal, this is broader-scoped alternative to that part.TODO:
cc @nodejs/startup @Renegade334