Skip to content

lib: include ESM loader in the built-in snapshot #61769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
joyeecheung wants to merge 3 commits into
base: main
Choose a base branch
from
Open

lib: include ESM loader in the built-in snapshot #61769

joyeecheung wants to merge 3 commits into from

Conversation

@joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Feb 11, 2026 •
edited
Loading

Benchmark number from an ARM64 Linux machine: empty/minimal CJS startup is now slightly slower in worker but other metrics get a slight boost (because they all incur ESM loader initialization). In reality ESM loading is likely to happen at some point in the lifetime of an application especially with the growing adoption of ESM and require(esm), so real-world applications that are more than just an empty script should get a bit of speed up from being able to just deserialize the ESM loader instead of initializing it from scrach.

                                                                                         confidence improvement accuracy (*)   (**)  (***)
misc/startup-core.js n=30 mode='process' script='benchmark/fixtures/empty.mjs'                  ***      6.06 %       ±0.60% ±0.80% ±1.04%
misc/startup-core.js n=30 mode='process' script='benchmark/fixtures/import-builtins.mjs'        ***      2.40 %       ±0.30% ±0.40% ±0.52%
misc/startup-core.js n=30 mode='process' script='benchmark/fixtures/require-builtins.js'        ***      0.57 %       ±0.32% ±0.42% ±0.55%
misc/startup-core.js n=30 mode='process' script='test/fixtures/semicolon.js'                            -0.22 %       ±0.56% ±0.75% ±0.97%
misc/startup-core.js n=30 mode='worker' script='benchmark/fixtures/empty.mjs'                   ***      1.89 %       ±0.26% ±0.34% ±0.44%
misc/startup-core.js n=30 mode='worker' script='benchmark/fixtures/import-builtins.mjs'         ***      0.74 %       ±0.28% ±0.38% ±0.49%
misc/startup-core.js n=30 mode='worker' script='benchmark/fixtures/require-builtins.js'         ***     -2.20 %       ±0.26% ±0.34% ±0.44%
misc/startup-core.js n=30 mode='worker' script='test/fixtures/semicolon.js'                     ***     -1.95 %       ±0.30% ±0.40% ±0.52%

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 11, 2026

Review requested:

  • @nodejs/loaders
  • @nodejs/performance
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Feb 11, 2026
mcollina
mcollina approved these changes Feb 11, 2026
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@codecov
Copy link

codecov bot commented Feb 11, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.77%. Comparing base (04946a7) to head (71c535a).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #61769      +/-   ##
==========================================
+ Coverage   89.73%   89.77%   +0.03%     
==========================================
  Files         675      674       -1     
  Lines      204648   204601      -47     
  Branches    39330    39320      -10     
==========================================
+ Hits       183651   183686      +35     
+ Misses      13282    13214      -68     
+ Partials     7715     7701      -14
Files with missing lines Coverage Δ
lib/internal/bootstrap/switches/is_main_thread.js 95.92% <100.00%> (+0.01%) ⬆️
lib/internal/modules/esm/get_format.js 94.83% <100.00%> (+1.68%) ⬆️
lib/internal/modules/esm/loader.js 98.76% <100.00%> (+1.79%) ⬆️
lib/internal/modules/esm/resolve.js 98.94% <100.00%> (+2.74%) ⬆️
lib/internal/modules/esm/translators.js 97.64% <100.00%> (+5.26%) ⬆️
lib/internal/process/pre_execution.js 95.98% <100.00%> (+0.01%) ⬆️
lib/internal/repl/completion.js 95.13% <100.00%> (ø)

... and 32 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.

watilde
watilde approved these changes Feb 11, 2026
View reviewed changes
Copy link
Member

@watilde watilde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question for my understanding: I noticed defaultResolve and defaultLoadSync are still lazy-loaded with ??= while translators was moved to pre-loading. Is this because:

  • These are only used when there are no custom loader hooks (less common path)?
  • Their dependencies are heavier and would hurt startup more?
  • Or is this something that could be optimized similarly in the future?

Just curious about the design rationale

@joyeecheung
Copy link
Member Author

joyeecheung commented Feb 11, 2026

I noticed defaultResolve and defaultLoadSync are still lazy-loaded with ??= while translators was moved to pre-loading.

Did you mean in esm/loader.js? I believe I've removed them. If not could you leave a pointer to the line where it still remains? Happy to update them in this PR.

GeoffreyBooth
GeoffreyBooth approved these changes Feb 11, 2026
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes

@watilde watilde watilde approved these changes

Assignees

No one assigned

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@joyeecheung @nodejs-github-bot @mcollina @GeoffreyBooth @watilde