module: support require()ing synchronous ESM graphs#51977
Closed
joyeecheung wants to merge 0 commit intonodejs:mainfrom
Closed
module: support require()ing synchronous ESM graphs#51977joyeecheung wants to merge 0 commit intonodejs:mainfrom
joyeecheung wants to merge 0 commit intonodejs:mainfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This patch adds
require()support for synchronous ESM graphs underthe flag
--experimental-require-moduleThis is based on the the following design aspect of ESM:
also synchronous, and, by the time the module graph is instantiated
(before evaluation starts), this is is already known.
If
--experimental-require-moduleis enabled, and the ECMAScriptmodule being loaded by
require()meets the following requirements:"type": "module"field inthe closest package.json or a
.mjsextension.await).require()will load the requested module as an ES Module, and returnthe module name space object. In this case it is similar to dynamic
import()but is run synchronously and returns the name space objectdirectly.
If the module being
require()'d contains top-levelawait, or the modulegraph it
imports contains top-levelawait,ERR_REQUIRE_ASYNC_MODULEwill be thrown. In this case, users shouldload the asynchronous module using
import().If
--experimental-print-required-tlais enabled, instead of throwingERR_REQUIRE_ASYNC_MODULEbefore evaluation, Node.js will evaluate themodule, try to locate the top-level awaits, and print their location to
help users find them.
Background
There were some previous discussions about this idea back in 2019 (e.g. #49450). I I didn't go through all of them, but in 2024 I believe we can agree that not supporting
require(esm)is creating enough pain for our users that we should really deprioritize the drawbacks of it. A non-perfect solution is still better than having nothing at all IMO.There was a previous attempt in #30891 which tried to support TLA from the start and thus needed to run the event loop recursively, which would be unsafe and therefore it was closed (synchronous-only
require(esm)was brought up in #30891 (comment) but the PR didn't end up going that way). I have the impression that there were some other attempts before, but non active AFAIK.This PR tries to keep it simple - only load ESM synchronously when we know it's synchronous (which is part of the design of ESM and is supported by the V8 API), and if it contains TLA, we throw. That should at least address the majority of use cases of ESM (TLA in a module that's supposed to be import'ed is already not a great idea, they are more meant for entry points. If they are really needed, users can use
import()to make that asynchronicity explicit).When I was refactoring the module loader implementation and touching the V8 Module API to fix other issues, this idea appears to be natural to me (since ESM is really designed to have this synchronocity in mind) and does not actually need that much work in 2024 (er, with some refactorings that I already did for other issues at least..), so here is another attempt at it.
Motivation
The motivation for this is probably obvious, but I'll give my take again in case there are unfamiliar readers: CJS/ESM interop would always be done on a best-effort basis and they should not be mixed if avoidable, but today the majority of the popular packages out there in the registry are still CJS. There needs to be an escape hatch for simple cases while the transition happens.
With
require(esm), when a dependency goes ESM-only, it is less likely to be a breaking change for users as long as it's a synchronous ESM (with no top-level await), which should be the case most of the time. This helps package authors transition to ESM without worrying about user experience, or having to release it as dual module which bloats thenode_modulessize even further and leads to identity problems due to the duplication.The design of ESM already ensures that synchronous evaluation and therefore interop with CJS for a synchronous graph is possible (e.g. see tc39/proposal-top-level-await#61), and we won't be alone in restricting TLA for certain features(e.g. w3c/ServiceWorker#1407 service workers on the web also disallows TLA) it would be a shame not to make use of that. Ongoing proposal like import defer could also help addressing the lazy-loading needs without breaking the synchronous aspect of ESM.
TODOs
There are still some feature interactions that this implementation doesn't handle (e.g.
--experimental-detect-moduleor--experimental-loaderor--experimental-wasm-modules). Some edge cases involving cycles probably would have undefined behaviors. I don't think this needs to handle interactions with everything (especially other experimental features) perfectly to land as a first iteration of an experimental feature. We can continue iterating on it while it's experimental.