ESM resolution and loading is now always synchronous from a
non-loader-hook thread. If no asynchrnous loader hooks are
registered, the resolution/loading is entirely synchronous.
If asynchronous loader hooks are registered, these would be
synchronous on the non-loader-hook thread, and asynchronous
on the loader hook thread.
This avoids several races caused by async/sync loading sharing
the same cache. In particular, asynchronous loader hooks
now works with `require(esm)` - previously it tends to break
due to races.
In addition, when an asynchronous loader hook
returns a promise that never settles, the main thread no longer
silently exits with exit code 13, leaving the code below
any module loading calls silently ignored without being executed.
Instead, it now throws ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED
which can be caught and handled by the main thread. If the module
request comes from `import()`, the never-settling promise is
now relayed to the result returned by `import()`.
Drive-by: when annotating the error about importing undetectable
named exports from CommonJS, it now no longer reload the source
code of the CommonJS module, and instead reuses format information
cached when the module was loaded for linking.
PR-URL: https://github.com/nodejs/node/pull/60380
Fixes: https://github.com/nodejs/node/issues/59666
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
- This updates the comments that assume loader hooks must be async
- Differentiate the sync/async loader hook paths in naming
`#customizations` is now `#asyncLoaderHooks` to make it clear
it's from the async APIs.
- Differentiate the paths running on the loader hook thread
(affects the loading of async other loader hooks and are async)
v.s. paths on the main thread calling out to code on the loader
hook thread (do not handle loading of other async loader hooks, and
can be sync by blocking).
- `Hooks` is now `AsyncLoaderHooksOnLoaderHookWorker`
- `CustomizedModuleLoader` is now
`AsyncLoaderHooksProxiedToLoaderHookWorker` and moved into
`lib/internal/modules/esm/hooks.js` as it implements the same
interface as `AsyncLoaderHooksOnLoaderHookWorker`
- `HooksProxy` is now `AsyncLoaderHookWorker`
- Adjust the JSDoc accordingly
- Clarify the "loader worker" as the "async loader hook worker"
i.e. when there's no _async_ loader hook registered, there won't
be this worker, to avoid the misconception that this worker
is spawned unconditionally.
- The code run on the loader hook worker to process
`--experimental-loader` is moved into
`lib/internal/modules/esm/worker.js` for clarity.
- The initialization configuration `forceDefaultLoader` is split
into `shouldSpawnLoaderHookWorker` and `shouldPreloadModules`
as those can be separate.
- `--experimental-vm-modules` is now processed during pre-execution
and no longer part of the initialization of the built-in ESM
loader, as it only exposes the vm APIs of ESM, and is unrelated
to built-in ESM loading.
PR-URL: https://github.com/nodejs/node/pull/60278
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
I missed marking a couple of test cases flaky in my previous PR:
https://github.com/nodejs/node/pull/58583
tests missed were:
- parallel.test-inspector-network-content-type
- benchmark.test-benchmark-websocket
After marking the above flaky I re-ran a test build and noticed
the build still failed.
Unfortuantely the build still fails because the build detects that
node process are left behind after the tests, presumably because tests
that spawn child processes are being timed out and only the parent
process is killed off, leaving the child processes behind.
see: https://github.com/nodejs/node/pull/59014#issuecomment-3057162303
We've decided to skip these tests until the underlying issue is
resolved.
The tests being skipped are:
- parallel.test-http-proxy-fetch
- parallel.test-https-proxy-fetch
- benchmark.test-benchmark-websocket
- parallel.test-inspector-network-fetch
- parallel.test-inspector-network-content-type
- es-module.test-wasm-web-api
- parallel.test-fetch
- parallel.test-without-async-context-frame
- report.test-report-exclude-network
PR-URL: https://github.com/nodejs/node/pull/59014
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
The comment in the test file stated that:
"SIMD is not supported on rhel8-ppc64le"
This is incorrect -- WASM SIMD support is dependent on the available
instructions, not the operating system. So it is more correct to say
that WASM SIMD is not supported on IBM Power8 architecture.
PR-URL: https://github.com/nodejs/node/pull/58767
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
When --import is used, the ESM loader is used to handle even pure
CJS entry points, and it can run into CJS module facades in the
evaluating state when the parent CJS module is being evaluated.
In this case it should be allowed, since the ESM <-> CJS cycles
that are meant to be disallowed (for the time being) should
already be detected before evaluation and wouldn't get here,
and CJS <-> CJS cycles are fine.
PR-URL: https://github.com/nodejs/node/pull/58598
Fixes: https://github.com/nodejs/node/issues/58515
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Underlying issues still need to be investigated
but this will prevent these failures from failing
the daily builds.
Timeouts:
- parallel.test-http-proxy-fetch
- parallel.test-https-proxy-fetch
- benchmark.test-benchmark-websocket
- parallel.test-inspector-network-fetch
- es-module.test-wasm-web-api
- parallel.test-fetch
- parallel.test-without-async-context-frame
- report.test-report-exclude-network
Flaky test:
- parallel.test-process-cpuUsage
PR-URL: https://github.com/nodejs/node/pull/58583
Refs: https://github.com/nodejs/node/issues/58582
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
By default, the `readableWebStream` method of `FileHandle` returns
a ReadableStream that, when finished, does not close the underlying
FileHandle. This can lead to issues if the stream is consumed
without having a reference to the FileHandle to close after use.
This commit adds an `autoClose` option to the `readableWebStream`
method, which, when set to `true`, will automatically close the
FileHandle when the stream is finished or canceled.
The test modified in this commit demonstrates one of the cases where
this is necessary in that the stream is consumed by separate code than
the FileHandle which was being left to close the underlying fd when
it is garbage collected, which is a deprecated behavior.
PR-URL: https://github.com/nodejs/node/pull/58548
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
There are several cleanups here that are not just style nits...
1. The `common.isMainThread` was just a passthrough to the
`isMainThread` export on the worker_thread module. It's
use was inconsistent and just obfuscated the fact that
the test file depend on the `worker_threads` built-in.
By eliminating it we simplify the test harness a bit and
make it clearer which tests depend on the worker_threads
check.
2. The `common.isDumbTerminal` is fairly unnecesary since
that just wraps a public API check.
3. Several of the `common.skipIf....` checks were inconsistently
used and really don't need to be separate utility functions.
A key part of the motivation here is to work towards making more
of the tests more self-contained and less reliant on the common
test harness where possible.
PR-URL: https://github.com/nodejs/node/pull/56712
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
