Skip to content

module: remove async module customization hooks - #67090

Open
GeoffreyBooth wants to merge 2 commits into
nodejs:mainfrom
GeoffreyBooth:remove-module-register
Open

GeoffreyBooth wants to merge 2 commits into
nodejs:mainfrom
GeoffreyBooth:remove-module-register

Conversation

@GeoffreyBooth

@GeoffreyBooth GeoffreyBooth commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

This should presumably land after the next amaro version bump on main. The bundled deps/amaro/dist/register-strip.mjs and register-transform.mjs still call module.register(). This PR should land after amaro is updated to a release that includes nodejs/amaro#497 (module.registerHooks).

Removes module.register() (DEP0205), the --experimental-loader and --loader flags, and the machinery that supported them: the loader hooks worker thread, the hooks proxy, the async module customization hook chains for resolving and loading, the internal worker class used to run the loader thread, and the asynchronous module job linking. Module customization is now provided only by module.registerHooks(), which runs synchronously on the thread that loads the modules.

  • The ES module loader always loads and links modules synchronously; the promise handling and the re-invented require() for imported CommonJS are gone. import() still defers evaluation to a later microtask.
  • worker_threads.isInternalThread is kept and always false; removing it is left for a semver-major change.
  • DEP0205 stays in deprecations.md as End-of-Life. ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED and ERR_LOADER_CHAIN_INCOMPLETE move to Legacy in errors.md.
  • Tests that only exercised the async hooks are removed; new tests cover require() after a failed import() and that import() does not evaluate the module before returning.
  • REPLACEME placeholders for the version are left for landing.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/loaders
  • @nodejs/test_runner
  • @nodejs/userland-migrations

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 28, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@GeoffreyBooth
GeoffreyBooth force-pushed the remove-module-register branch from a98403b to 95c14fc Compare October 29, 2026 02:24
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@GeoffreyBooth
GeoffreyBooth force-pushed the remove-module-register branch 2 times, most recently from 6cea4a9 to 355da9b Compare October 30, 2026 02:44
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@GeoffreyBooth
GeoffreyBooth force-pushed the remove-module-register branch from 355da9b to aab37fe Compare October 30, 2026 03:23
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@GeoffreyBooth
GeoffreyBooth force-pushed the remove-module-register branch from aab37fe to ada3473 Compare October 30, 2026 04:57
@GeoffreyBooth
GeoffreyBooth marked this pull request as ready for review October 30, 2026 05:00
@GeoffreyBooth GeoffreyBooth added module Issues and PRs related to the module subsystem. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders. labels Oct 30, 2026
@GeoffreyBooth
GeoffreyBooth force-pushed the remove-module-register branch from ada3473 to 937a866 Compare October 30, 2026 05:02
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Remove `module.register()` (DEP0205), the `--experimental-loader` and
`--loader` flags, and the machinery that supported them: the loader
hooks worker thread, the hooks proxy, the async module customization
hook chains for resolving and loading, the internal worker class used to
run the loader thread, and the asynchronous module job linking. Module
customization is now provided only by `module.registerHooks()`, which
runs synchronously on the thread that loads the modules.

As the loader thread no longer exists, the ES module loader always loads
and links modules synchronously, so the code that handled promises and
the re-invented `require()` for imported CommonJS customized by the
async module customization hooks is removed too. `import()` still defers
evaluation of the module to a later microtask, as before.

`worker_threads.isInternalThread` is kept for compatibility and is now
always `false`; removing it is left for a future semver-major change.

The `ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED` and
`ERR_LOADER_CHAIN_INCOMPLETE` error codes are no longer used and are
documented as removed.

Tests that exist only to exercise the async module customization hooks
are removed.

Assisted-by: Claude
Signed-off-by: Geoffrey Booth <webadmin@geoffreybooth.com>
@GeoffreyBooth
GeoffreyBooth force-pushed the remove-module-register branch from 937a866 to a825144 Compare October 30, 2026 05:15
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.54015% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.65%. Comparing base (1915b77) to head (a94c7c2).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/node_watchdog.cc 0.00% 0 Missing and 1 partial ⚠️
src/node_worker.cc 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67090      +/-   ##
==========================================
- Coverage   90.67%   90.65%   -0.02%     
==========================================
  Files         797      794       -3     
  Lines      278777   276716    -2061     
  Branches    53610    53307     -303     
==========================================
- Hits       252782   250866    -1916     
+ Misses      16343    16199     -144     
+ Partials     9652     9651       -1     
Files with missing lines Coverage Δ
lib/internal/bench_runner/cli.js 94.15% <ø> (-0.01%) ⬇️
lib/internal/errors.js 98.80% <100.00%> (-0.01%) ⬇️
lib/internal/main/eval_string.js 79.48% <100.00%> (ø)
lib/internal/main/worker_thread.js 94.87% <100.00%> (-0.24%) ⬇️
lib/internal/modules/customization_hooks.js 100.00% <100.00%> (ø)
lib/internal/modules/esm/load.js 95.54% <100.00%> (-1.27%) ⬇️
lib/internal/modules/esm/loader.js 99.56% <100.00%> (-0.35%) ⬇️
lib/internal/modules/esm/module_job.js 100.00% <100.00%> (+0.41%) ⬆️
lib/internal/modules/esm/module_map.js 98.48% <100.00%> (+0.01%) ⬆️
lib/internal/modules/esm/translators.js 97.42% <100.00%> (-0.21%) ⬇️
... and 14 more

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AugustinMauroy

Copy link
Copy Markdown
Member

since it's simplify loading phase, it's there are an positive impact in term of perf ?

@marco-ippolito

Copy link
Copy Markdown
Member

I think this should be treated as semver major due to the large impact

@GeoffreyBooth

Copy link
Copy Markdown
Member Author

@AugustinMauroy: since it's simplify loading phase, it's there are an positive impact in term of perf ?

I ran Node’s benchmark/compare.js with compare.R, 20 runs each, on builds from just before and after this PR. This is one machine, and the builds aren’t from the latest main. Positive means the PR is faster:

  • misc/startup-core (process and worker modes, 10 cases): +0.3% to +5.4%, none significant at the 1% level.
  • esm/startup-esm-graph (250–2000 modules): +1.1% to +2.7%, but the benchmark’s noise at this run count (±17%) is far larger than any effect.
  • esm/esm-loader-import: significant gains for data: specifiers (+2.4%) and nonexistent node: specifiers (+7.7%). The other three cases were flat and not significant.

So there’s no regression, and at most a small gain in the import path, because loading no longer has to check for async hooks. The larger win is for hook users. In a separate hyperfine run, a script using module.register() took about 141 ms versus about 107 ms with registerHooks() (that gain comes from the sync API already on main). Overall this PR is mostly simplification and maintenance, not a general speedup.

benchmark results (misc/startup-core, esm/startup-esm-graph, esm/esm-loader-import)
$ node benchmark/compare.js --old ./node-old --new ./node-new --runs 20 --filter startup-core misc | Rscript benchmark/compare.R
                                                                                         confidence improvement accuracy (*)   (**)  (***)
misc/startup-core.js n=30 mode='process' script='benchmark/fixtures/empty.mjs'                           3.58 %       ±4.25% ±5.77%  ±7.75%
misc/startup-core.js n=30 mode='process' script='benchmark/fixtures/import-builtins.mjs'                 2.18 %       ±3.55% ±4.77%  ±6.30%
misc/startup-core.js n=30 mode='process' script='benchmark/fixtures/require-builtins.js'                 3.81 %       ±3.93% ±5.28%  ±6.99%
misc/startup-core.js n=30 mode='process' script='test/fixtures/semicolon.js'                             0.34 %       ±2.94% ±3.94%  ±5.19%
misc/startup-core.js n=30 mode='process' script='test/fixtures/snapshot/typescript.js'                   0.78 %       ±2.73% ±3.66%  ±4.81%
misc/startup-core.js n=30 mode='worker' script='benchmark/fixtures/empty.mjs'                            5.15 %       ±6.54% ±8.78% ±11.58%
misc/startup-core.js n=30 mode='worker' script='benchmark/fixtures/import-builtins.mjs'           *      4.66 %       ±4.44% ±6.00%  ±7.98%
misc/startup-core.js n=30 mode='worker' script='benchmark/fixtures/require-builtins.js'                  2.32 %       ±3.93% ±5.28%  ±6.98%
misc/startup-core.js n=30 mode='worker' script='test/fixtures/semicolon.js'                              5.35 %       ±5.61% ±7.51%  ±9.88%
misc/startup-core.js n=30 mode='worker' script='test/fixtures/snapshot/typescript.js'                    0.99 %       ±2.72% ±3.64%  ±4.79%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 10 comparisons, you can thus
expect the following amount of false-positive results:
  0.50 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.10 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

$ node benchmark/compare.js --old ./node-old --new ./node-new --runs 20 --filter startup-esm-graph esm | Rscript benchmark/compare.R
                                           confidence improvement accuracy (*)    (**)   (***)
esm/startup-esm-graph.js n=10 modules=1000                 1.90 %      ±17.57% ±23.53% ±30.95%
esm/startup-esm-graph.js n=10 modules=2000                 1.13 %      ±17.39% ±23.30% ±30.64%
esm/startup-esm-graph.js n=10 modules=250                  1.24 %      ±17.81% ±23.85% ±31.37%
esm/startup-esm-graph.js n=10 modules=500                  2.67 %      ±17.46% ±23.38% ±30.75%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

$ node benchmark/compare.js --old ./node-old --new ./node-new --runs 20 --filter esm-loader-import esm | Rscript benchmark/compare.R
                                                                      confidence improvement accuracy (*)   (**)  (***)
esm/esm-loader-import.js specifier='./relative-existing.js' n=1000                   -0.32 %       ±1.24% ±1.68% ±2.24%
esm/esm-loader-import.js specifier='./relative-nonexistent.js' n=1000                -0.10 %       ±1.11% ±1.49% ±1.97%
esm/esm-loader-import.js specifier='data:text/javascript,{i}' n=1000         ***      2.44 %       ±1.23% ±1.65% ±2.18%
esm/esm-loader-import.js specifier='node:os' n=1000                                   2.13 %       ±2.17% ±2.92% ±3.87%
esm/esm-loader-import.js specifier='node:prefixed-nonexistent' n=1000        ***      7.73 %       ±0.80% ±1.07% ±1.40%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 5 comparisons, you can thus
expect the following amount of false-positive results:
  0.25 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.05 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

I also checked with hyperfine on same-config builds, before and after this PR. For the common path (no hooks) there’s no measurable difference. All of these are within noise:

Scenario Before After
node -e 0 99.1 ± 5.0 ms 99.0 ± 5.7 ms
ESM entry (three modules) 107.1 ± 7.7 ms 105.6 ± 5.6 ms
CJS entry 104.8 ± 4.3 ms 103.1 ± 4.6 ms

The removal mainly benefits hook users. A script using module.register() async hooks took about 141 ms, versus about 107 ms with the equivalent registerHooks() (about 24% faster). That gain comes from the sync API, which is already on main.

@GeoffreyBooth

Copy link
Copy Markdown
Member Author

@marco-ippolito I’m happy for this to land only in the 27 line with do not backport labels, so in practice it ships as semver-major. There’s no benefit in backporting it, since registerHooks() is available in the older lines.

I’d still argue it isn’t semver-major in the policy sense, because module.register() and --experimental-loader were experimental:

  • Before it was deprecated, module.register() was documented as “Stability: 1.1 - Active development”, and the “Asynchronous customization hooks” section still carries that marking. documentation.md says Stability 1 features are “not subject to semantic versioning rules” and that “removal may occur in any future release.”
  • --experimental-loader has had “experimental” in its name since the rename in cli: rename --loader to --experimental-loader #29752, and its docs say it “is discouraged and may be removed in a future version of Node.js.”

We also went well beyond what an experimental API requires. After the discussion in #62395 (e.g. “experimental status is already a warning”, and the concerns about existing adoption), we shipped a docs-only deprecation in v25.9.0/v24.15.0 (#62395) and a runtime deprecation in v26.0.0 (#62401). Users have had a full major of runtime warnings, and the plan to remove in 27 has been on record since #62401.

@GeoffreyBooth GeoffreyBooth added the dont-land-on-v26.x PRs that should not land on the v26.x-staging branch and should not be released in v26.x. label Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dont-land-on-v26.x PRs that should not land on the v26.x-staging branch and should not be released in v26.x. esm Issues and PRs related to the ECMAScript Modules implementation. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. loaders Issues and PRs related to ES module loaders. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants