Proposal: Have the ESM loader handle all entry points
Personne n'a encore pris cette issue.
- Langage dominant
- JavaScript
- Étoiles
- 122k
- Forks
- 37.3k
- Merge moyen
- 4 j 2 h
- PR mergées (30 j)
- 283
Description
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.mjspython test/pseudo-tty/../../tools/pseudo-tty.py ./node test/pseudo-tty/console_colors.jspython test/pseudo-tty/../../tools/pseudo-tty.py ./node test/pseudo-tty/test-fatal-error.jspython test/pseudo-tty/../../tools/pseudo-tty.py ./node test/pseudo-tty/test-trace-sigint.js
@nodejs/loaders @nodejs/tsc @nodejs/performance @nodejs/async_hooks
Guide de contribution
Ouvrir le guide de contribution
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Piste de recherche
Commencez par lib/internal/modules/run_main.js, en particulier shouldUseESMLoader et la branche if (useESMLoader), puis exécutez make test pour reproduire les échecs. Choisissez un groupe délimité parmi les tests permission, async-hooks ou parallel listés et examinez leurs assertions actuelles. C’est terminé lorsque les tests sélectionnés passent à la fois avant et avec le chemin du chargeur ESM toujours activé, contribuant ainsi à atteindre zéro échec.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- javascript, node.js
- Domaine
- backend, testing
- Type d'issue
- Fonctionnalité
- Difficulté
- 5/5
- Temps estimé
- Plus d'une semaine
- Activité
- Calme
- Clarté
- Plutôt claire
- Accessibilité débutants
- 42/100