This improves Permission Model usage when allowing read access to
specifi modules. To achieve that, the permission model check on
internalModuleStat has been removed meaning that on module loading,
uv_fs_stat is performed on files and folders even when the permission
model is enabled. Although a uv_fs_stat is performed, reading/executing
the module will still pass by the permission model check.
Without this PR when an app tries to --allow-fs-read=./a.js
--allow-fs-read=./b.js where `a` attempt to load b, it will fails as
it reads $pwd and no permission has been given to this path.
PR-URL: https://github.com/nodejs/node/pull/55797
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Previously in the CommonJS loader, --inspect-brk is implemented
checking whether the module points to the result of re-resolving
process.argv[1] to determine whether the module is the entry point.
This is unnecessarily complex, especially now that we store that
information in the module as kIsMainSymbol. This patch updates
it to simply check that symbol property instead.
PR-URL: https://github.com/nodejs/node/pull/55679
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This refactors the CommonJS loading a bit to create a center point
that handles source loading (`loadSource`) and make format detection
more consistent to pave the way for future synchronous hooks.
- Handle .mjs in the .js handler, similar to how .cjs has been handled.
- Generate the legacy ERR_REQUIRE_ESM in a getRequireESMError() for
both .mts and require(esm) handling (when it's disabled).
PR-URL: https://github.com/nodejs/node/pull/55590
Refs: https://github.com/nodejs/loaders/pull/198
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
This tracks the asynchronicity in the ModuleWraps when they turn out to
contain TLA after instantiation, and throw the right error
(ERR_REQUIRE_ASYNC_MODULE) when it's required again. It removes
the freezing of ModuleWraps since it's not meaningful to freeze
this when the rest of the module loader is mutable, and we
can record the asynchronicity in the ModuleWrap right after
compilation after we get a V8 upgrade that contains
v8::Module::HasTopLevelAwait() instead of searching through
the module graph repeatedly which can be slow.
PR-URL: https://github.com/nodejs/node/pull/55520
Fixes: https://github.com/nodejs/node/issues/55516
Refs: https://github.com/nodejs/node/issues/52697
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Trim off irrelevant internal stack frames for require(esm) warnings
so it's easier to locate where the call comes from when
--trace-warnings is used.
PR-URL: https://github.com/nodejs/node/pull/55496
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.
PR-URL: https://github.com/nodejs/node/pull/55502
Fixes: https://github.com/nodejs/node/issues/55500
Refs: https://github.com/nodejs/node/issues/52697
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
When emitting the experimental warning for `require(esm)`, include
information about the parent module and the module being require()-d
to help users locate and update them.
PR-URL: https://github.com/nodejs/node/pull/55397
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55471
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
This reverts commit efbba60e5b.
PR-URL: https://github.com/nodejs/node/pull/55414
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
As the documentation states, the `context.importAssertion` should be
still supported and emit a warning. This is true for the `load` hook,
but not correct for context of the `resolve` hook.
This commit fixes the inconsistency.
PR-URL: https://github.com/nodejs/node/pull/55365
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The two proposals reached stage 4 at the October 2024 meeting.
PR-URL: https://github.com/nodejs/node/pull/55333
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55332
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55316
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Previously we assumed if `--experimental-detect-module` is true, then
`--experimental-require-module` is true, which isn't the case, as
the two can be enabled/disabled separately. This patch fixes the
checks so `--no-experimental-require-module` is still effective when
`--experimental-detect-module` is enabled.
Drive-by: make the assertion messages more informative and remove
obsolete TODO about allowing TLA in entrypoints handled by
require(esm).
PR-URL: https://github.com/nodejs/node/pull/55250
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
This is faster and more consistent with other places using the
regular expression to detect node_modules.
PR-URL: https://github.com/nodejs/node/pull/55243
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/53848
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/54563
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>
When creating an fast api the callback might use the receiver. In that
case if the internal binding is destructured the method won't have
access to the reciver and it will throw. Passing the receiver as second
argument ensures the receiver is available.
PR-URL: https://github.com/nodejs/node/pull/54408
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Co-Authored-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/54933
Refs: https://github.com/nodejs/node/pull/49975
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This unflags --experimental-require-module so require(esm) can be
used without the flag. For now, when require() actually encounters
an ESM, it will still emit an experimental warning. To opt out
of the feature, --no-experimental-require-module can be used.
There are some tests specifically testing ERR_REQUIRE_ESM. Some
of them are repurposed to test --no-experimental-require-module.
Some of them are modified to just expect loading require(esm) to
work, when it's appropriate.
PR-URL: https://github.com/nodejs/node/pull/55085
Refs: https://github.com/nodejs/node/issues/52697
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
This patch implements a "module-sync" exports condition
for packages to supply a sycnrhonous ES module to the
Node.js module loader, no matter it's being required
or imported. This is similar to the "module" condition
that bundlers have been using to support `require(esm)`
in Node.js, and allows dual-package authors to opt into
ESM-first only newer versions of Node.js that supports
require(esm) while avoiding the dual-package hazard.
```json
{
"type": "module",
"exports": {
"node": {
// On new version of Node.js, both require() and import get
// the ESM version
"module-sync": "./index.js",
// On older version of Node.js, where "module" and
// require(esm) are not supported, use the transpiled CJS version
// to avoid dual-package hazard. Library authors can decide
// to drop support for older versions of Node.js when they think
// it's time.
"default": "./dist/index.cjs"
},
// On any other environment, use the ESM version.
"default": "./index.js"
}
}
```
We end up implementing a condition with a different name
instead of reusing "module", because existing code in the
ecosystem using the "module" condition sometimes also expect
the module resolution for these ESM files to work in CJS
style, which is supported by bundlers, but the native
Node.js loader has intentionally made ESM resolution
different from CJS resolution (e.g. forbidding `import
'./noext'` or `import './directory'`), so it would be
semver-major to implement a `"module"` condition
without implementing the forbidden ESM resolution rules.
For now, this just implments a new condition as semver-minor
so it can be backported to older LTS.
Refs: https://webpack.js.org/guides/package-exports/#target-environment-independent-packages
PR-URL: https://github.com/nodejs/node/pull/54648
Fixes: https://github.com/nodejs/node/issues/52173
Refs: https://github.com/joyeecheung/test-module-condition
Refs: https://github.com/nodejs/node/issues/52697
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
This implements an API for users to intentionally flush the
accumulated compile cache instead of waiting until process
shutdown. It may be useful for application that loads dependencies
first and then either reload itself in other instances, or spawning
other instances that load an overlapping set of its dependencies -
in this case its useful to flush the cache early instead of waiting
until the shutdown of itself.
Currently flushing is triggered by either process
shutdown or user requests. In the future we should simply start the
writes right after module loading on a separate thread, and this method
only blocks until all the pending writes (if any) on the other thread
are finished. In that case, the off-thread writes should finish long
before any attempt of flushing is made so the method would then only
incur a negligible overhead from thread synchronization.
PR-URL: https://github.com/nodejs/node/pull/54971
Fixes: https://github.com/nodejs/node/issues/54770
Fixes: https://github.com/nodejs/node/issues/54465
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This lays the foundation for supporting synchronous hooks proposed
in https://github.com/nodejs/loaders/pull/198 for ESM.
- Corrects and adds several JSDoc comments for internal functions
of the ESM loader, as well as explaining how require() for
import CJS work in the special resolve/load paths. This doesn't
consolidate it with import in require(esm) yet due to caching
differences, which is left as a TODO.
- The moduleProvider passed into ModuleJob is replaced as
moduleOrModulePromise, we call the translators directly in the
ESM loader and verify it right after loading for clarity.
- Reuse a few refactored out helpers for require(esm) in
getModuleJobForRequire().
PR-URL: https://github.com/nodejs/node/pull/54769
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The synchronous CJS translator can handle entrypoints now, this
can be hit when --import is used, so lift the bogus assertions and
added tests.
PR-URL: https://github.com/nodejs/node/pull/54592
Fixes: https://github.com/nodejs/node/issues/54577
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch adds the following API for tools to enable compile
cache dynamically and query its status.
- module.enableCompileCache(cacheDir)
- module.getCompileCacheDir()
In addition this adds a NODE_DISABLE_COMPILE_CACHE environment
variable to disable the code cache enabled by the APIs as
an escape hatch to avoid unexpected/undesired effects of
the compile cache (e.g. less precise test coverage).
When the module.enableCompileCache() method is invoked without
a specified directory, Node.js will use the value of
the NODE_COMPILE_CACHE environment variable if it's set, or
defaults to `path.join(os.tmpdir(), 'node-compile-cache')`
otherwise. Therefore it's recommended for tools to call this
method without specifying the directory to allow overrides.
PR-URL: https://github.com/nodejs/node/pull/54501
Fixes: https://github.com/nodejs/node/issues/53639
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Source map is not necessary in strip-only mode. However, to map the
source file in debuggers to the original TypeScript source, add a
sourceURL magic comment to hint that it is a generated source.
PR-URL: https://github.com/nodejs/node/pull/54402
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
