-
Notifications
You must be signed in to change notification settings - Fork 0
#### Summary #4
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.
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
Comments
|
Summary
This patch adds
require()
support for synchronous ESM graphs underthe flag
--experimental-require-module
This 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-module
is enabled, and the ECMAScriptmodule being loaded by
require()
meets the following requirements:"type": "module"
field inthe closest package.json or a
.mjs
extension.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
import
s contains top-levelawait
,ERR_REQUIRE_ASYNC_MODULE
will be thrown. In this case, users shouldload the asynchronous module using
import()
.If
--experimental-print-required-tla
is enabled, instead of throwingERR_REQUIRE_ASYNC_MODULE
before 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. nodejs/node#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 nodejs/node#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 nodejs/node#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_modules
size 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-module
or--experimental-loader
or--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.Originally posted by @joyeecheung in nodejs/node#51977
Originally posted by @Pitan1993 in https://github.com/Pitan1993/codespaces-flask/issues/13
Originally posted by @Pitan1993 in https://github.com/Pitan1993/Ptj/issues/5
Originally posted by @Pitan1993 in https://github.com/Pitan1993/codespaces-flask/issues/14
The text was updated successfully, but these errors were encountered: