It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:
[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json
Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.
PR-URL: https://github.com/nodejs/node/pull/54045
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/53619
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Tooling in the ecosystem have been using the __esModule property to
recognize transpiled ESM in consuming code. For example, a 'log'
package written in ESM:
export function log(val) { console.log(val); }
Can be transpiled as:
exports.__esModule = true;
exports.default = function log(val) { console.log(val); }
The consuming code may be written like this in ESM:
import log from 'log'
Which gets transpiled to:
const _mod = require('log');
const log = _mod.__esModule ? _mod.default : _mod;
So to allow transpiled consuming code to recognize require()'d real ESM
as ESM and pick up the default exports, we add a __esModule property by
building a source text module facade for any module that has a default
export and add .__esModule = true to the exports. We don't do this to
modules that don't have default exports to avoid the unnecessary
overhead. This maintains the enumerability of the re-exported names
and the live binding of the exports.
The source of the facade is defined as a constant per-isolate property
required_module_facade_source_string, which looks like this
export * from 'original';
export { default } from 'original';
export const __esModule = true;
And the 'original' module request is always resolved by
createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping
over the original module.
PR-URL: https://github.com/nodejs/node/pull/52166
Refs: https://github.com/nodejs/node/issues/52134
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
PR-URL: https://github.com/nodejs/node/pull/53711
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch:
1. Refactor the routines used to compile and run an embedder
entrypoint. In JS land special handling for SEA is done
directly in main/embedding.js instead of clobbering the CJS
loader. Add warnings to remind users that currently the
require() in SEA bundled scripts only supports loading builtins.
2. Don't use the bundled SEA code cache when compiling CJS
loaded from disk, since in that case we are certainly not
compiling the code bundled into the SEA. Use a is_sea_main
flag in CompileFunctionForCJSLoader() (which replaces an unused
argument) to pass this into the C++ land - the code cache is
still read directly from C++ to avoid the overhead of
ArrayBuffer creation.
3. Move SEA loading code into
MaybeLoadSingleExecutableApplication() which calls
LoadEnvironment() with its own StartExecutionCallback().
This avoids more hidden switches in StartExecution() and
make them explicit. Also add some TODOs about how to support
ESM in embedded applications.
4. Add more comments
PR-URL: https://github.com/nodejs/node/pull/53573
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit adds a tracing channel for module loading
through `import()` and `require()`.
Co-Authored-By: Stephen Belanger <admin@stephenbelanger.com>
PR-URL: https://github.com/nodejs/node/pull/44340
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
PR-URL: https://github.com/nodejs/node/pull/52658
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
This reverts commit 22cb99d073.
PR-URL: https://github.com/nodejs/node/pull/53183
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Unifies the CJS and ESM source map cache map with SourceMapCacheMap
and allows the CJS cache entries to be queried more efficiently with
a source url without iteration on an IterableWeakMap.
Add a test to verify that the CJS source map cache entry can be
reclaimed.
PR-URL: https://github.com/nodejs/node/pull/51711
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
`process.getBuiltinModule(id)` provides a way to load built-in modules
in a globally available function. ES Modules that need to support
other environments can use it to conditionally load a Node.js built-in
when it is run in Node.js, without having to deal with the resolution
error that can be thrown by `import` in a non-Node.js environment or
having to use dynamic `import()` which either turns the module into an
asynchronous module, or turns a synchronous API into an asynchronous
one.
```mjs
if (globalThis.process.getBuiltinModule) {
// Run in Node.js, use the Node.js fs module.
const fs = globalThis.process.getBuiltinModule('fs');
// If `require()` is needed to load user-modules, use
// createRequire()
const module = globalThis.process.getBuiltinModule('module');
const require = module.createRequire(import.meta.url);
const foo = require('foo');
}
```
If `id` specifies a built-in module available in the current Node.js
process, `process.getBuiltinModule(id)` method returns the
corresponding built-in module. If `id` does not correspond to any
built-in module, `undefined` is returned.
`process.getBuiltinModule(id)` accept built-in module IDs that are
recognized by `module.isBuiltin(id)`. Some built-in modules must be
loaded with the `node:` prefix.
The built-in modules returned by `process.getBuiltinModule(id)` are
always the original modules - that is, it's not affected by
`require.cache`.
PR-URL: https://github.com/nodejs/node/pull/52762
Fixes: https://github.com/nodejs/node/issues/52599
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
So that if there are circular dependencies in the synchronous
module graph, they could be resolved using the cached jobs.
In case linking fails and the error gets caught, reset the
cache right after linking. If it succeeds, the caller will
cache it again. Otherwise the error bubbles up to users,
and since we unset the cache for the unlinkable module
the next attempt would still fail.
PR-URL: https://github.com/nodejs/node/pull/52868
Fixes: https://github.com/nodejs/node/issues/52864
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/52706
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
This patch:
1. Adds ESM syntax detection to compileFunctionForCJSLoader()
for --experimental-detect-module and allow it to emit the
warning for how to load ESM when it's used to parse ESM as
CJS but detection is not enabled.
2. Moves the ESM detection of --experimental-detect-module for
the entrypoint from executeUserEntryPoint() into
Module.prototype._compile() and handle it directly in the
CJS loader so that the errors thrown during compilation *and
execution* during the loading of the entrypoint does not
need to be bubbled all the way up. If the entrypoint doesn't
parse as CJS, and detection is enabled, the CJS loader will
re-load the entrypoint as ESM on the spot asynchronously using
runEntryPointWithESMLoader() and cascadedLoader.import(). This
is fine for the entrypoint because unlike require(ESM) we don't
the namespace of the entrypoint synchronously, and can just
ignore the returned value. In this case process.mainModule is
reset to undefined as they are not available for ESM entrypoints.
3. Supports --experimental-detect-module for require(esm).
PR-URL: https://github.com/nodejs/node/pull/52047
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@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>
Instead of using an async function wrapper, just try compiling code with
unknown module format as SourceTextModule when it cannot be compiled
as CJS and the error message indicates that it's worth a retry. If
it can be parsed as SourceTextModule then it's considered ESM.
Also, move shouldRetryAsESM() to C++ completely so that
we can reuse it in the CJS module loader for require(esm).
Drive-by: move methods that don't belong to ContextifyContext
out as static methods and move GetHostDefinedOptions to
ModuleWrap.
PR-URL: https://github.com/nodejs/node/pull/52413
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Previously there is an edge case where submodules loaded by require()
may not be loaded by import() again from different intermediate
edges in the graph. This patch fixes that, added tests, and added
debug logs.
Drive-by: make loader a private field so it doesn't show up in logs.
PR-URL: https://github.com/nodejs/node/pull/52487
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Symbol properties are typically more GC-efficient than using WeakMaps,
since WeakMap requires ephemeron GC. `module[kModuleExportNames]`
would be easier to read than `importedCJSCache.get(module).exportNames`
as well.
PR-URL: https://github.com/nodejs/node/pull/52095
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch disallows CJS <-> ESM edges when they come from
require(esm) requested in ESM evalaution.
Drive-by: don't reuse the cache for imported CJS modules to stash
source code of required ESM because the former is also used for
cycle detection.
PR-URL: https://github.com/nodejs/node/pull/52264
Fixes: https://github.com/nodejs/node/issues/52145
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@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>
This patch adds `require()` support for synchronous ESM graphs under
the flag `--experimental-require-module`
This is based on the the following design aspect of ESM:
- The resolution can be synchronous (up to the host)
- The evaluation of a synchronous graph (without top-level await) is
also synchronous, and, by the time the module graph is instantiated
(before evaluation starts), this is is already known.
If `--experimental-require-module` is enabled, and the ECMAScript
module being loaded by `require()` meets the following requirements:
- Explicitly marked as an ES module with a `"type": "module"` field in
the closest package.json or a `.mjs` extension.
- Fully synchronous (contains no top-level `await`).
`require()` will load the requested module as an ES Module, and return
the module name space object. In this case it is similar to dynamic
`import()` but is run synchronously and returns the name space object
directly.
```mjs
// point.mjs
export function distance(a, b) {
return (b.x - a.x) ** 2 + (b.y - a.y) ** 2;
}
class Point {
constructor(x, y) { this.x = x; this.y = y; }
}
export default Point;
```
```cjs
const required = require('./point.mjs');
// [Module: null prototype] {
// default: [class Point],
// distance: [Function: distance]
// }
console.log(required);
(async () => {
const imported = await import('./point.mjs');
console.log(imported === required); // true
})();
```
If the module being `require()`'d contains top-level `await`, or the
module graph it `import`s contains top-level `await`,
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users
should load the asynchronous module using `import()`.
If `--experimental-print-required-tla` is enabled, instead of throwing
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
module, try to locate the top-level awaits, and print their location to
help users fix them.
PR-URL: https://github.com/nodejs/node/pull/51977
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
So that we can use it to handle code caching in a central place.
Drive-by: use per-isolate persistent strings for the parameters
and mark GetHostDefinedOptions() since it's only used in one
compilation unit
PR-URL: https://github.com/nodejs/node/pull/52016
Refs: https://github.com/nodejs/node/issues/47472
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
When the entry point is a module and the graph it imports still
contains unsettled top-level await when the Node.js instance
finishes the event loop, search from the entry point module
for unsettled top-level await and print their location.
To avoid unnecessary overhead, we register a promise that only
gets settled when the entry point graph evaluation returns
from await, and only search the module graph if it's still
unsettled by the time the instance is exiting.
This patch only handles this for entry point modules. Other kinds of
modules are more complicated so will be left for the future.
Drive-by: update the terminology "unfinished promise" to the
more correct one "unsettled promise" in the codebase.
PR-URL: https://github.com/nodejs/node/pull/51999
Fixes: https://github.com/nodejs/node/issues/42868
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Split the `internal/process/esm_loader` file which contains the
singleton cascaded loader:
- The the singleton cascaded loader now directly resides in
`internal/modules/esm/loader`, where the constructor also lives.
This file is the root of most circular dependency of ESM code,
(because components of the loader need the singleton itself),
so this makes the dependency more obvious. Added comments about
loading it lazily to avoid circular dependency.
- The getter to the cascaded loader is also turned into a method
to make the side effect explicit.
- The sequence of `loadESM()` and `handleMainPromise` is now merged
together into `runEntryPointWithESMLoader()` in
`internal/modules/run_main` because this is intended to run entry
points with the ESM loader and not just any module.
- Documents how top-level await is handled.
PR-URL: https://github.com/nodejs/node/pull/51999
Fixes: https://github.com/nodejs/node/issues/42868
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Previously we only had an internal assertion to ensure certain
code is executed before any user-provided CJS is run. This patch
adds another assertion for ESM.
Note that this internal state is not updated during source text
module execution via vm because to run any code via vm, some
user JS code must have already been executed anyway.
In addition this patch moves the states into internal/modules/helpers
to avoid circular dependencies. Also moves toggling the states to
true *right before* user code execution instead of after in case
we are half-way in the execution when internals try to check them.
PR-URL: https://github.com/nodejs/node/pull/51748
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
This patch adds support for using
`vm.constants.USE_MAIN_CONTEXT_DEFAULT_LOADER` as
`importModuleDynamically` in all APIs that take the option
except `vm.SourceTextModule`. This allows users to have a shortcut
to support dynamic import() in the compiled code without missing
the compilation cache if they don't need customization of the
loading process. We emit an experimental warning when the
`import()` is actually handled by the default loader through
this option instead of requiring `--experimental-vm-modules`.
In addition this refactors the documentation for
`importModuleDynamically` and adds a dedicated section for it
with examples.
`vm.SourceTextModule` is not supported in this patch because
it needs additional refactoring to handle `initializeImportMeta`,
which can be done in a follow-up.
PR-URL: https://github.com/nodejs/node/pull/51244
Fixes: https://github.com/nodejs/node/issues/51154
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
This puts it together with the cjsParseCache and reduces the
circular dependency on the singleton loader, which is the
only place where this cache is stored.
Drive-by: remove always-false module status check because there's
no longer a local module variable after
https://github.com/nodejs/node/pull/34605 which is now invalid
leftover code at this point and only doesn't throw because
we happen to have a top-level variable called module.
PR-URL: https://github.com/nodejs/node/pull/51157
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/51033
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/50466
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ensure that `defaultLoad` does not uselessly access the file system to
get the source of modules that are known to be in CommonJS format.
This allows CommonJS imports to resolve in the current phase of the
event loop.
Refs: https://github.com/eslint/eslint/pull/17683
PR-URL: https://github.com/nodejs/node/pull/50465
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
When using the Modules Customization Hooks API to load CommonJS modules,
we want to support the returned value of `defaultLoad` which must be
nullish to preserve backward compatibility. This can be achieved by
fetching the source from the translator.
PR-URL: https://github.com/nodejs/node/pull/50825
Fixes: https://github.com/nodejs/node/issues/50435
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
PR-URL: https://github.com/nodejs/node/pull/50322
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Daniel Lemire <daniel@lemire.me>
PR-URL: https://github.com/nodejs/node/pull/50322
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
This bootstraps ESM loaders in the ShadowRealm with
`ShadowRealm.prototype.importValue` as its entry point and enables
loading ESM and CJS modules in the ShadowRealm. The module is imported
without a parent URL and resolved with the current process's working
directory.
PR-URL: https://github.com/nodejs/node/pull/48655
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
The flag is always true and can be determined by isLoaderWorker solely.
PR-URL: https://github.com/nodejs/node/pull/48655
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This allows user to opt-out from using the monkey-patchable CJS loader,
even to load CJS modules.
PR-URL: https://github.com/nodejs/node/pull/50004
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
PR-URL: https://github.com/nodejs/node/pull/50127
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Users cannot access any API that can be used to return a module or
module namespace in this callback without --experimental-vm-modules
anyway, so this would eventually lead to a rejection. This patch
rejects in this case with our own error message and use a constant
host-defined option for the rejection, so that scripts with the
same source can still be compiled using the compilation cache
if no `import()` is actually called in the script.
PR-URL: https://github.com/nodejs/node/pull/50137
Refs: https://github.com/nodejs/node/issues/35375
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Instead of using the public versions of the vm APIs internally,
use the internal versions so that we can skip unnecessary
argument validation.
The public versions would need special care to the generation
of host-defined options to hit the isolate compilation cache
when imporModuleDynamically isn't used, while internally it's
almost always used, so this allows us to handle the host-defined
options separately.
PR-URL: https://github.com/nodejs/node/pull/50137
Refs: https://github.com/nodejs/node/issues/35375
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
The old import assertions proposal has been
renamed to "import attributes" with the follwing major changes:
1. The keyword is now `with` instead of `assert`.
2. Unknown assertions cause an error rather than being ignored,
This commit updates the documentation to encourage folks to use the new
syntax, and add aliases for module customization hooks.
PR-URL: https://github.com/nodejs/node/pull/50140
Fixes: https://github.com/nodejs/node/issues/50134
Refs: 159c82c5e6
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
createDynamicModule() properly escapes import names, but not export
names. In WebAssembly, any string is a valid export name. Importing a
WebAssembly module that uses a non-identifier export name leads to
either a syntax error in createDynamicModule() or to code injection,
that is, to the evaluation of almost arbitrary JavaScript code outside
of the WebAssembly module.
To address this issue, adopt the same mechanism in createExport() that
createImport() already uses. Add tests for both exports and imports.
PR-URL: https://github.com/nodejs-private/node-private/pull/461
Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/489
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
CVE-ID: CVE-2023-39333
PR-URL: https://github.com/nodejs/node/pull/49639
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This makes it possile to hit the in-isolate compilation cache when
host-defined options are not necessary.
PR-URL: https://github.com/nodejs/node/pull/49950
Refs: https://github.com/nodejs/node/issues/35375
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
PR-URL: https://github.com/nodejs/node/pull/49912
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/49869
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/49633
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
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>
The current API shape si not great because it's too limited and
redundant with the use of `MessagePort`.
PR-URL: https://github.com/nodejs/node/pull/49529
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
PR-URL: https://github.com/nodejs/node/pull/49028
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/49038
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Marco Ippolito
Michaël Zasso
Joyee Cheung
Rafael Gonzaga
Geoffrey Booth
Antoine du Hamel
Daniel Bayley
Yagiz Nizipli
Matteo Collina
Vinicius Lourenço
Chengzhong Wu
Gabriel Bota
Chengzhong Wu
Paolo Insogna
Jacob Smith
unbyte
per4uk
Hiroki Osame
Bruce MacNaughton
Alex Yang
Francesco Trotta
James Sumners
Tobias Nießen
Aras Abbasi
João Lenon
André Alves
Guy Bedford