nodejs / nodejs/node

Proposal: Have the ESM loader handle all entry points

未关闭
#50,356 31 条评论 6 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

esm module never-stale performance
主要语言
JavaScript
星标
122k
派生
37.3k
平均合并
4 天 2 小时
30 天内合并 PR
283

描述

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 https://github.com/nodejs/node/pull/44323#issuecomment-1775434155) 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

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

从 lib/internal/modules/run_main.js 开始,重点查看 shouldUseESMLoader 和 if (useESMLoader) 分支,然后运行 make test 以复现失败。 从列出的 permission、async-hooks 或 parallel 测试中选择一个范围有限的组,并检查其当前的 assertions。选定的测试在原本的情况下以及始终启用 ESM loader 路径的情况下都通过,并为达到零失败做出贡献,即表示完成。

由索引模型根据 Issue 内容生成。

评估

技术栈
javascript, node.js
领域
backend, testing
Issue 类型
功能
难度
5/5
预计耗时
一周以上
活跃度
冷清
描述清晰度
基本清楚
新手友好度
42/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。