Proposal: Have the ESM loader handle all entry points

[GeoffreyBooth]

In current Node, the ESM loader handles the application entry point when it is an ES module or if any of the following flags are passed: --import, --experimental-default-type, --experimental-detect-module, --experimental-loader. This means that the ESM loader handles CommonJS entry points when certain flags are passed. (The ESM loader supports ES modules, CommonJS modules, WebAssembly modules and JSON files.) A function shouldUseESMLoader in run_main.js determines whether the ESM loader should handle the main entry point.

If we were to remove this function and simply always have the ESM loader handle all entry points, there would be a few gains:

• There would be only one code path for starting the application entry point, rather than two, simplifying the codebase and making it more maintainable.
• The simpler the codebase can become, the more likely it will be that we can find performance improvements, such as by moving parts of the flow into C++.

It would also mean the end of monkey-patching the CommonJS loader, which has never been officially supported and I think is ready to be officially retired now that the module customization hooks exist and provide fully equivalent functionality, including for customizing require calls. They are Release Candidate and I expect them to become stable in the next few months. This potential change of having the ESM loader handle all entries would happen after that, at the next major version of Node. It shouldn’t be a semver-major change, but monkey-patching is common enough that I’d expect some breakage and therefore we should treat it as semver-major.

If you take the run_main.js if (useESMLoader) { line and change it to if (true) { and run make test, as of today there are 154 failing tests. For the most part, these are tests that just need updating because the assertions in the tests are more specific than they need to be for what they’re validating. They test things like the number of promises created since Node started up, which varies when the ESM loader is engaged because the loader itself creates a few; or they need --allow-fs to include allowing reading some package.json files that the ESM loader loads that the CommonJS loader didn’t; or they are validating too precise of a stack trace, that changes when the ESM loader is used; and so on. The largest group of failing tests is for async hooks, and I’ve discussed this with @Qard (see #44323 (comment)) and we think that those tests should simply be updated, once the major refactor of AsyncLocalStorage lands.

In the meantime, I think we should start making smaller PRs to start updating batches of tests that fail when shouldUseESMLoader is always true, so that these tests pass both today and in “always using the ESM loader” mode. Once we can get that 154 number down to zero, then we can do the cutover, ideally before Node 22 to be released as part of 22.0.0. This issue can serve as the tracking issue for the effort.

These are the tests (or APIs) that need updating:

• ./node --experimental-permission --allow-fs-read=* --allow-child-process test/parallel/test-permission-fs-wildcard.js
• ./node --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process test/parallel/test-permission-fs-read.js
• ./node --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process test/parallel/test-permission-fs-symlink-target-write.js
• ./node --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process test/parallel/test-permission-fs-symlink.js
• ./node --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process test/parallel/test-permission-fs-traversal-path.js
• ./node --experimental-permission --allow-fs-read=* test/parallel/test-permission-experimental.js
• ./node --expose-gc test/async-hooks/test-pipewrap.js
• ./node --expose-gc test/async-hooks/test-querywrap.js
• ./node --expose-gc test/parallel/test-heapdump-async-hooks-init-promise.js
• ./node --expose-internals test/async-hooks/test-emit-after-on-destroyed.js
• ./node --expose-internals test/async-hooks/test-emit-before-on-destroyed.js
• ./node --expose-internals test/async-hooks/test-emit-init.js
• ./node --expose-internals test/async-hooks/test-http-agent-handle-reuse-parallel.js
• ./node --expose-internals test/async-hooks/test-http-agent-handle-reuse-serial.js
• ./node --expose-internals test/async-hooks/test-improper-order.js
• ./node --expose-internals test/async-hooks/test-zlib.zlib-binding.deflate.js
• ./node --expose-internals test/message/internal_assert_fail.js
• ./node --expose-internals test/message/internal_assert.js
• ./node --expose-internals test/parallel/test-bootstrap-modules.js
• ./node --expose-internals test/parallel/test-cluster-dgram-bind-fd.js
• ./node --expose-internals test/parallel/test-util-promisify.js
• ./node --inspect=0 test/known_issues/test-inspector-cluster-port-clash.js
• ./node --no-warnings --expose-internals test/parallel/test-whatwg-webstreams-adapters-to-writablestream.js
• ./node --pending-deprecation test/parallel/test-module-parent-deprecation.js
• ./node --test-udp-no-try-send test/async-hooks/test-udpsendwrap.js
• ./node test/addons/callback-scope/test-async-hooks.js
• ./node test/addons/make-callback-recurse/test.js
• ./node test/addons/report-api/test.js
• ./node test/async-hooks/test-async-await.js
• ./node test/async-hooks/test-crypto-pbkdf2.js
• ./node test/async-hooks/test-crypto-randomBytes.js
• ./node test/async-hooks/test-disable-in-init.js
• ./node test/async-hooks/test-embedder.api.async-resource-no-type.js
• ./node test/async-hooks/test-embedder.api.async-resource.js
• ./node test/async-hooks/test-enable-disable.js
• ./node test/async-hooks/test-enable-in-init.js
• ./node test/async-hooks/test-filehandle-no-reuse.js
• ./node test/async-hooks/test-fseventwrap.js
• ./node test/async-hooks/test-fsreqcallback-access.js
• ./node test/async-hooks/test-fsreqcallback-readFile.js
• ./node test/async-hooks/test-getaddrinforeqwrap.js
• ./node test/async-hooks/test-getnameinforeqwrap.js
• ./node test/async-hooks/test-graph.fsreq-readFile.js
• ./node test/async-hooks/test-graph.http.js
• ./node test/async-hooks/test-graph.intervals.js
• ./node test/async-hooks/test-graph.pipe.js
• ./node test/async-hooks/test-graph.pipeconnect.js
• ./node test/async-hooks/test-graph.shutdown.js
• ./node test/async-hooks/test-graph.signal.js
• ./node test/async-hooks/test-graph.statwatcher.js
• ./node test/async-hooks/test-graph.tcp.js
• ./node test/async-hooks/test-graph.timeouts.js
• ./node test/async-hooks/test-graph.tls-write-12.js
• ./node test/async-hooks/test-graph.tls-write.js
• ./node test/async-hooks/test-httpparser.request.js
• ./node test/async-hooks/test-httpparser.response.js
• ./node test/async-hooks/test-immediate.js
• ./node test/async-hooks/test-nexttick-default-trigger.js
• ./node test/async-hooks/test-pipeconnectwrap.js
• ./node test/async-hooks/test-promise.chain-promise-before-init-hooks.js
• ./node test/async-hooks/test-promise.js
• ./node test/async-hooks/test-promise.promise-before-init-hooks.js
• ./node test/async-hooks/test-queue-microtask.js
• ./node test/async-hooks/test-shutdownwrap.js
• ./node test/async-hooks/test-signalwrap.js
• ./node test/async-hooks/test-statwatcher.js
• ./node test/async-hooks/test-tcpwrap.js
• ./node test/async-hooks/test-timers.setInterval.js
• ./node test/async-hooks/test-timers.setTimeout.js
• ./node test/async-hooks/test-tlswrap.js
• ./node test/async-hooks/test-ttywrap.readstream.js
• ./node test/async-hooks/test-udpwrap.js
• ./node test/async-hooks/test-unhandled-exception-valid-ids.js
• ./node test/async-hooks/test-unhandled-rejection-context.js
• ./node test/async-hooks/test-writewrap.js
• ./node test/message/assert_throws_stack.js
• ./node test/message/util_inspect_error.js
• ./node test/message/util-inspect-error-cause.js
• ./node test/node-api/test_callback_scope/test-async-hooks.js
• ./node test/node-api/test_make_callback_recurse/test.js
• ./node test/node-api/test_policy/test_policy.js
• ./node test/parallel/test-async-hooks-correctly-switch-promise-hook.js
• ./node test/parallel/test-async-hooks-disable-during-promise.js
• ./node test/parallel/test-async-hooks-enable-recursive.js
• ./node test/parallel/test-async-hooks-promise-triggerid.js
• ./node test/parallel/test-async-hooks-promise.js
• ./node test/parallel/test-async-hooks-top-level-clearimmediate.js
• ./node test/parallel/test-async-local-storage-exit-does-not-leak.js
• ./node test/parallel/test-async-wrap-promise-after-enabled.js
• ./node test/parallel/test-child-process-fork-closed-channel-segfault.js
• ./node test/parallel/test-cli-permission-deny-fs.js
• ./node test/parallel/test-cluster-advanced-serialization.js
• ./node test/parallel/test-cluster-dgram-2.js
• ./node test/parallel/test-cluster-fork-windowsHide.js
• ./node test/parallel/test-cluster-send-deadlock.js
• ./node test/parallel/test-cluster-send-socket-to-worker-http-server.js
• ./node test/parallel/test-cluster-worker-disconnect-on-error.js
• ./node test/parallel/test-cluster-worker-events.js
• ./node test/parallel/test-cluster-worker-forced-exit.js
• ./node test/parallel/test-cluster-worker-init.js
• ./node test/parallel/test-cluster-worker-no-exit.js
• ./node test/parallel/test-debugger-break.js
• ./node test/parallel/test-debugger-breakpoint-exists.js
• ./node test/parallel/test-debugger-clear-breakpoints.js
• ./node test/parallel/test-debugger-exceptions.js
• ./node test/parallel/test-debugger-exec-scope.mjs
• ./node test/parallel/test-debugger-extract-function-name.mjs
• ./node test/parallel/test-debugger-heap-profiler.js
• ./node test/parallel/test-debugger-help.mjs
• ./node test/parallel/test-debugger-list.js
• ./node test/parallel/test-debugger-object-type-remote-object.js
• ./node test/parallel/test-debugger-profile-command.js
• ./node test/parallel/test-debugger-profile.js
• ./node test/parallel/test-debugger-random-port-with-inspect-port.js
• ./node test/parallel/test-debugger-random-port.js
• ./node test/parallel/test-debugger-run-after-quit-restart.js
• ./node test/parallel/test-debugger-sb-before-load.js
• ./node test/parallel/test-debugger-scripts.js
• ./node test/parallel/test-debugger-set-context-line-number.mjs
• ./node test/parallel/test-debugger-use-strict.js
• ./node test/parallel/test-debugger-watch-validation.js
• ./node test/parallel/test-debugger-watchers.mjs
• ./node test/parallel/test-diagnostics-channel-process.js
• ./node test/parallel/test-error-reporting.js
• ./node test/parallel/test-inspector-debug-brk-flag.js
• ./node test/parallel/test-inspector-exception.js
• ./node test/parallel/test-inspector.js
• ./node test/parallel/test-module-main-preserve-symlinks-fail.js
• ./node test/parallel/test-node-output-console.mjs
• ./node test/parallel/test-node-output-errors.mjs
• ./node test/parallel/test-node-output-sourcemaps.mjs
• ./node test/parallel/test-node-output-vm.mjs
• ./node test/parallel/test-performance-nodetiming.js
• ./node test/parallel/test-process-external-stdio-close-spawn.js
• ./node test/parallel/test-process-external-stdio-close.js
• ./node test/parallel/test-process-uncaught-exception-monitor.js
• ./node test/parallel/test-promise-hook-create-hook.js
• ./node test/parallel/test-promise-hook-exceptions.js
• ./node test/parallel/test-promise-hook-on-after.js
• ./node test/parallel/test-promise-hook-on-resolve.js
• ./node test/parallel/test-runner-output.mjs
• ./node test/parallel/test-sync-io-option.js
• ./node test/parallel/test-v8-coverage.js
• ./node test/parallel/test-worker-debug.js
• ./node test/parallel/test-worker-load-file-with-extension-other-than-js.js
• ./node test/parallel/test-worker-message-port-inspect-during-init-hook.js
• ./node test/sequential/test-debugger-custom-port.js
• ./node test/sequential/test-debugger-launch.mjs
• ./node test/sequential/test-module-loading.js
• ./node test/sequential/test-perf-hooks.js
• ./node test/sequential/test-watch-mode.mjs
• python test/pseudo-tty/../../tools/pseudo-tty.py ./node test/pseudo-tty/console_colors.js
• python test/pseudo-tty/../../tools/pseudo-tty.py ./node test/pseudo-tty/test-fatal-error.js
• python test/pseudo-tty/../../tools/pseudo-tty.py ./node test/pseudo-tty/test-trace-sigint.js

@nodejs/loaders @nodejs/tsc @nodejs/performance @nodejs/async_hooks

[mcollina]

I think that we need stable loaders first and at least a full deprecation cycle, which would mean deprecation of customizing require in v22 and removal in v23.

[ljharb]

What's the point of calling CJS "stable" for over a decade if breaking it is ever on the table?

[GeoffreyBooth]

which would mean deprecation of customizing require in v22 and removal in v23.

Updating all these tests might not be ready in time for 22 regardless, but I think we should still get started. We don't need to debate just yet whether this should happen in 22 or 23.

Let's add the deprecation warning now. The hooks are hopefully stable, I'm just waiting for any bug reports to trickle in before opening a PR to make it official. And to be clear, the deprecation isn't around customizing require; that is supported via the hooks. The deprecation is around monkey patching, which isn't part of our API today and never has been.

And to be even clearer: this proposal doesn't involve removing the CommonJS loader. It just means that monkey patching wouldn't affect the entry point, since that's effectively imported when loaded by the ESM loader. I think monkey patching would still work for any modules required by a CommonJS entry point.

[ljharb]

If it's something that's worked for a decade in a module system that's been called "stable" - meaning, everything that's possible to do with it is part of the API - then it's something that shouldn't be broken.

Specifically, monkey patching require.extensions needs to work forever.

[joyeecheung]

For now the customization hooks do not appear to be equivalent for the customization of require, it needs a runtime flag whereas monkey-patching of require does not. For example this means popular modules like https://www.npmjs.com/package/v8-compile-cache would be broken.

They test things like the number of promises created since Node started up, which varies when the ESM loader is engaged because the loader itself creates a few

This look like a problem of the ESM loader itself, instead of a problem of the test?

[joyeecheung]

As for the gains mentioned in the OP:

There would be only one code path for starting the application entry point, rather than two, simplifying the codebase and making it more maintainable.
The simpler the codebase can become, the more likely it will be that we can find performance improvements, such as by moving parts of the flow into C++.

I am not fully convinced by this. The CommonJS loader as it stands is currently much simpler than the ESM loader. It's also easier to optimize and snapshot, whereas the ESM loader is currently much more complex and involves a lot of unnecessary operation during initialization. The ESM loader also relies on TDZs and circular dependencies to work, which is a footgun. I think those should be refactored out and cleaned up before we can consider making it the only entry point, otherwise this only results in breakage in the ecosystem + performance regressions with only "potential performance gains", but not effective ones. The proposal now seems to be mostly about an internal refactoring, I think we should do it the other way around - improve the current ESM loader implementation so that there are actually performance gains by using it by default, put this behind a flag, and use the flag to test it and prove that it improves performance & can minimize breakages in the ecosystem, then flip the flag and remove the CommonJS loader when we have proof (possibly leaving the CommonJS loader around for a few cycles so that people can revert back with a flag when problem arises).

[targos]

Customization hooks don't need a runtime flag anymore. We even recommend to use module.register instead.

[Flarna]

ESM hooks require an additional command line option --import to get them installed early enough.
Monkey patching CJS loader works also if done early enough in the main application file. Quite some tools relying on monkey patching CJS loader have something like "require me early" in their docs.

[targos]

ESM hooks require an additional command line option --import to get them installed early enough.

Even when the entry point is CommonJS? I would expect this to work:

const { register } = require('module');

register(...)

// customized
const something = require('something')

[koshic]

Customization hooks don't need a runtime flag anymore. We even recommend to use module.register instead.

node --import register-hook-a.js --import register-hook-b.js is not the same as node --loader loader-a.js --loader-b.js as an example - imported modules loaded in parallel so there is no chaining as we have with --loader or synchronous --require.

So it's hard say that 'we are stable, all use cases covered by hooks API & ESM loader'.

[aduh95]

ESM hooks require an additional command line option --import to get them installed early enough.

Even when the entry point is CommonJS? I would expect this to work:

const { register } = require('module');

register(...)

// customized
const something = require('something')

This only works if that CJS module (the one calling require) was loaded by the ESM loader – atm the ESM loader will delegate the loading of CJS files to the CJS loader; however you can opt-in with e.g. --experimental-default-type=module to have the ESM loader handle everything by default, in which case the above snippet should work as expected.

[aduh95]

Customization hooks don't need a runtime flag anymore. We even recommend to use module.register instead.

node --import register-hook-a.js --import register-hook-b.js is not the same as node --loader loader-a.js --loader-b.js as an example - imported modules loaded in parallel so there is no chaining as we have with --loader or synchronous --require.

So it's hard say that 'we are stable, all use cases covered by hooks API & ESM loader'.

You can register loaders customization hooks using --require.

[koshic]

You can register loaders customization hooks using --require.

I can't:

➜  nexus git:(master) ✗ node -v                         
v21.0.0
➜  nexus git:(master) ✗ cat ./a.mjs                     
console.log("a");
➜  nexus git:(master) ✗ node --require ./a.mjs ./app.mjs
node:internal/modules/cjs/loader:1198
    throw new ERR_REQUIRE_ESM(filename, true);
    ^

[Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/xxx/a.mjs not supported.
Instead change the require of /Users/xxx/a.mjs to a dynamic import() which is available in all CommonJS modules.] {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v21.0.0
➜  nexus git:(master) ✗ 

Sure, I can use CJS... But stop, we are talking about 'stable ESM', right? )

PS In my opinion, it's wrong to use CJS as kind of 'assembly language' in ESM world to implement some low-level tricks.

[mcollina]

Specifically, monkey patching require.extensions need to work forever.

At the point, I'm a bit frustrated by the support for monkey patching that I would prefer to cut ties to many old things just to let Node.js move forward.

A slightly different take on this path could be: remove the require patching when all release lines had stable loaders from the start.

[GeoffreyBooth]

I can’t:

You can if you --require a CommonJS file that calls register.

PS In my opinion, it’s wrong to use CJS as kind of ‘assembly language’ in ESM world to implement some low-level tricks.

You can also do it without any flags and without any CommonJS. Run Node with this as your entry point:

import { register } from 'node:module'

register('./hooks1.js', import.meta.url)
register('./hooks2.js', import.meta.url)

import('./app.js')

Because this is an import() expression rather than a static import, it’ll get evaluated where it’s seen rather than ahead of time during the parse phase. This makes it essentially an async version of require. In this case there’s no need to await it because there’s no code afterward.

[GeoffreyBooth]

I think those should be refactored out and cleaned up before we can consider making it the only entry point

For now all I'm proposing is that we get our test suite passing when the ESM loader handles all entry points. We can always add other criteria before actually making this change, sure, and refactoring can happen in parallel with updating the tests.

And a big motivation for doing this is the complexity of the ESM loader. I agree it needs a cleanup; that's why I'm trying to simplify the initial code path.

[ljharb]

Specifically, monkey patching require.extensions need to work forever.

At the point, I'm a bit frustrated by the support for monkey patching that I would prefer to cut ties to many old things just to let Node.js move forward.

A slightly different take on this path could be: remove the require patching when all release lines had stable loaders from the start.

There are a great many packages that will never be updated that could work perfectly forever. CJS is marked “stable”, it’s not supposed to change.

The internal implementation can change in any way, but the observable behavior must not, otherwise how can anyone trust when node declares something “stable”?

[mcollina]

The internal implementation can change in any way, but the observable behavior must not, otherwise how can anyone trust when node declares something “stable”?

You are referring to the old "locked" state that was removed in #11200. All stable APIs can face a deprecation cycle and be removed or altered. It's precisely what a "breaking change" is for.

The deprecation cycle is what I mentioned in #50356 (comment).

[ljharb]

Thanks, you’re right, but there’s still a lot of packages created during that time period, with that expectation. The breakage will be quite significant, and “use loaders instead” isn’t a viable option for most of these use cases.

[koshic]

You can also do it without any flags and without any CommonJS. Run Node with this as your entry point

Thank you for that suggestion, but it doesn't work in real-world scenarios when loaders (ok, hooks) passed from parent process as part of NODE_OPTIONS, thx to chaining. With proposed approach, user should add 'import ts-node' (as an example) at special wrapper around any .ts file he wants to run , because '--import .pnp.loader.mjs --import ts-node' combination will fail at 'ts-node' resolution. Sorry for such offtopic, it's a good example shows that current ESM APIs are... unfinished?

[GeoffreyBooth]

Sorry for such offtopic, it’s a good example shows that current ESM APIs are… unfinished?

I disagree that they’re unfinished, but why don’t you open a separate issue for this? I do agree that it’s off-topic for this one 😄

[mcollina]

Thanks, you’re right, but there’s still a lot of packages created during that time period, with that expectation. The breakage will be quite significant, and “use loaders instead” isn’t a viable option for most of these use cases.

I agree on the brekage impact. However it's generically possible to track the deprecation and act on it only after we have minimized the disruption.

I'm generically done with supporting unmaintained modules forever.

What use cases are not covered by the new register API?

[ljharb]

I would like to ensure that a module that has patched require.extensions will continue to work forever. I have no opinion or attachment on/to the internal implementation; having only the one loader path for everything seems ideal, modulo #50356 (comment).

I don't think that "but we'd have only one implementation!" is compelling enough to warrant literally any breakage of any kind, but if it can be done in a nonbreaking way, the benefits are obvious.

[joyeecheung]

What use cases are not covered by the new register API?

I don't think the hooks allow you to customize how the modules are compiled? That's why we have specifically have this branch in the CommonJS loader

node/lib/internal/modules/cjs/loader.js

Line 1264 in 4f5db8b

if (patched) {

and there are some pretty popular modules like https://www.npmjs.com/package/v8-compile-cache that depend on patching Module.prototype methods. Judging from the number of downloads of that package, I think we need to be careful not to create performance regressions in the module loading. I am pretty sure this package is not the only one that relies on the patchability of the CommonJS loader, so the flip should be done after investigation into the breakage + providing enough equivalents. So far what I've seen are only vague evaluations of the breakage and migration paths that aren't very concrete.

[joyeecheung]

Also some questions about the register API:

1. It's run on a different thread which comes with a performance impact that simple monkey-patching doesn't have and that should be dealt with. Also running it on a different thread can already result in behavior parity.
2. The hooks are async, how are they supposed to provide equivalent behavior of the original monkey-patchability of the CommonJS loader which is synchronous?

[aduh95]

2. The hooks are async, how are they supposed to provide equivalent behavior of the original monkey-patchability of the CommonJS loader which is synchronous?

That's the point of running them on a different thread (that and encapsulation), it lets us run async code in a way that appears sync to the main thread.

[GeoffreyBooth]

I don’t think the hooks allow you to customize how the modules are compiled?

So part of the plan for the loaders/customization team is to add customization hooks for other subsystems, like import { register } from 'node:fs' and from node:repl etc. What you’re describing sounds like a potential compile hook that can be used via a potential import { register } from 'node:vm'. Creating such a hook would also allow v8-compile-cache to support ES modules.

Also some questions about the register API:

Let’s discuss this in a new thread: #50392

Whether or not there are use case gaps with the customization hooks doesn’t really matter right now; we’re not on the verge just yet of making the end-goal change, of having the ESM loader handle all entry points. Unless we think that we should never make such a change, we can defer that evaluation until we’re ready to open a PR to do it. In the meantime, I’d like for us to get started updating the tests I mentioned in the top post, so that we’re eventually in a position to make the change.