PR-URL: https://github.com/nodejs/node/pull/52686
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Avoid repetitively calling into JS callback from C++ in
`ModuleWrap::Link`. This removes the convoluted callback style of the
internal `ModuleWrap` link step.
PR-URL: https://github.com/nodejs/node/pull/52058
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This refactors the code that compiles SourceTextModule for the
built-in ESM loader to use a common routine so that it's easier
to customize cache handling for the ESM loader. In addition
this introduces a common symbol for import.meta and import()
so that we don't need to create additional closures as handlers,
since we can get all the information we need from the V8 callback
already. This should reduce the memory footprint of ESM as well.
PR-URL: https://github.com/nodejs/node/pull/52291
Refs: https://github.com/nodejs/node/issues/47472
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Check if the value returned from user linker function is a null-ish
value.
`validateInternalField` should be preferred when checking `this`
argument to guard against null-ish `this`.
Co-authored-by: Mike Ralphson <mike.ralphson@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/52162
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
We are now directly checking the existence of a private symbol
in the object to determine if an object is a ContextifyContext
anyway, so there is no need to implement it in C++ anymore.
PR-URL: https://github.com/nodejs/node/pull/51685
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
The old import assertions proposal has been
renamed to "import attributes" with the following major changes:
1. The keyword is now `with` instead of `assert`.
2. Unknown assertions cause an error rather than being ignored.
This PR updates the documentation to encourage folks to use the new
syntax, and add aliases to preserve backward compatibility.
PR-URL: https://github.com/nodejs/node/pull/50141
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Previously when managing the importModuleDynamically callback of
vm.compileFunction(), we use an ID number as the host defined option
and maintain a per-Environment ID -> CompiledFnEntry map to retain
the top-level referrer function returned by vm.compileFunction() in
order to pass it back to the callback, but it would leak because with
how we used v8::Persistent to maintain this reference, V8 would not
be able to understand the cycle and would just think that the
CompiledFnEntry was supposed to live forever. We made an attempt
to make that reference known to V8 by making the CompiledFnEntry weak
and using a private symbol to make CompiledFnEntry strongly
references the top-level referrer function in
https://github.com/nodejs/node/pull/46785, but that turned out to be
unsound, because the there's no guarantee that the top-level function
must be alive while import() can still be initiated from that
function, since V8 could discard the top-level function and only keep
inner functions alive, so relying on the top-level function to keep
the CompiledFnEntry alive could result in use-after-free which caused
a revert of that fix.
With this patch we use a symbol in the host defined options instead of
a number, because with the stage-3 symbol-as-weakmap-keys proposal
we could directly use that symbol to keep the referrer alive using a
WeakMap. As a bonus this also keeps the other kinds of referrers
alive as long as import() can still be initiated from that
Script/Module, so this also fixes the long-standing crash caused by
vm.Script being GC'ed too early when its importModuleDynamically
callback still needs it.
PR-URL: https://github.com/nodejs/node/pull/48510
Refs: https://github.com/nodejs/node/issues/44211
Refs: https://github.com/nodejs/node/issues/42080
Refs: https://github.com/nodejs/node/issues/47096
Refs: https://github.com/nodejs/node/issues/43205
Refs: https://github.com/nodejs/node/issues/38695
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Throwing error after checking type is repeated.
So replace it with validate function.
PR-URL: https://github.com/nodejs/node/pull/46176
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
fixup: add support for `Object.create(null)`
fixup: extend to any 1-argument Object.create call
fixup: add tests
PR-URL: https://github.com/nodejs/node/pull/46083
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This moves the following utils into modules/esm/utils.js:
- Code related to default conditions
- The callbackMap (which is now created in the module instead of
hanging off the module_wrap binding, since the C++ land
does not need it).
- Per-isolate module callbacks
These are self-contained code that can be included into the
built-in snapshot.
PR-URL: https://github.com/nodejs/node/pull/45849
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
In a previous commit, the loader implementation was modified to be
protected against most prototype pollution, but was kept vulnerable to
`Array.prototype` pollution. This commit fixes that, the tradeoff is
that it modifies the `ESMLoader.prototype.import` return type from an
`Array` to an array-like object.
Refs: https://github.com/nodejs/node/pull/45044
PR-URL: https://github.com/nodejs/node/pull/45175
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/42820
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/37598
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Co-authored-by: ExE Boss <3889017+ExE-Boss@users.noreply.github.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/37028
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
add validateFunction and refactor to use validateFunction
PR-URL: https://github.com/nodejs/node/pull/37045
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/36983
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
I added a new custom ESLint rule to fix these problems.
We have a lot of replaceable codes with primordials.
Accessing built-in objects is restricted by existing rule
(no-restricted-globals), but accessing property in the built-in objects
is not restricted right now. We manually review codes that can be
replaced by primordials, but there's a lot of code that actually needs
to be fixed. We have often made pull requests to replace the primordials
with.
Restrict accessing global built-in objects such as `Promise`.
Restrict calling static methods such as `Array.from` or `Symbol.for`.
Don't restrict prototype methods to prevent false-positive.
PR-URL: https://github.com/nodejs/node/pull/35448
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
To achieve this, some internal custom inspect functions had to be
changed. They relied upon the former behavior.
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/33449
Fixes: https://github.com/nodejs/node/issues/33419
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/31278
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Macros, like CHECK, cause issues for tracking coverage because
they modify the source before it's placed in V8. Upon investigation
it seemed that we only used this functionality in two places:
internal/vm/module.js, and internal/async_hooks.js (in comments).
Given this, it seemed to make more sense to move CHECK to
JavaScript, and retire a mostly unused build step.
PR-URL: https://github.com/nodejs/node/pull/30755
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/30635
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Store all primordials as properties of the primordials object.
Static functions are prefixed by the constructor's name and prototype
methods are prefixed by the constructor's name followed by "Prototype".
For example: primordials.Object.keys becomes primordials.ObjectKeys.
PR-URL: https://github.com/nodejs/node/pull/30610
Refs: https://github.com/nodejs/node/issues/29766
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Since the bootstrap does not actually use ESM at all, there
is no need to create this map so early. This patch moves
the initialization of the map to pre-execution,
so that the only binding loaded in loaders is native_module.
In addition, switch to SafeWeakMap.
PR-URL: https://github.com/nodejs/node/pull/27323
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This patches changes the `safe_globals` internal module into a
script that gets run during bootstrap and saves JavaScript builtins
(primordials) into an object that is available for all other builtin
modules to access lexically later.
PR-URL: https://github.com/nodejs/node/pull/25816
Refs: https://github.com/nodejs/node/issues/18795
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
internalBinding is used so often that it should just automatically be
available for usage in internals.
PR-URL: https://github.com/nodejs/node/pull/23025
Refs: https://github.com/nodejs/node/commit/2a9eb31
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
At the last TC39 meeting, a new type of Module Records backed by
JavaScript source called Dynamic Module Records was discussed, and
it is now at Stage 1. Regardless of whether that proposal makes it
all the way into the spec, SourceTextModule is indeed a more
descriptive and accurate name for what this class represents.
PR-URL: https://github.com/nodejs/node/pull/22007
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
This commit adds a constant named defaultModuleName to avoid duplicating
it in the Module constructor function.
PR-URL: https://github.com/nodejs/node/pull/20709
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
