Skip to content

Commit

Permalink
fixup! module: support require()ing synchronous ESM graphs
Browse files Browse the repository at this point in the history
  • Loading branch information
joyeecheung committed Mar 9, 2024
1 parent a1b3cd4 commit a235a15
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 24 deletions.
30 changes: 17 additions & 13 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -879,17 +879,21 @@ added: REPLACEME

> Stability: 1.1 - Active Developement
Supports loading a synchronous ES module graph in `require()`. If the module
graph is not synchronous (contains top-level await), Node.js throws an error
without executing the module. If `--print-pending-tla` is passed, Node.js
will evaluate the module, try to locate the top-level awaits, and print
their location to help users fix them.

With this option alone, the module being `require()`'d should be explicitly
marked as an ES module either using the `"type": "module"` field in
`package.json` or the `.mjs` extension. To load implicit ES modules with
automatic syntax-based module type detection, use
`--experimental-require-module-with-detection`.
Supports loading a synchronous ES module graph in `require()`.

When `--experimental-require-module` is enabled, and `require()`
resolves to an ES module, if the module is fully synchronous
(contains no top-level await), the `require()` call returns the
module name space object. Otherwise, Node.js throws an error
without executing the module. If `--experimental-print-required-tla`
is passed, Node.js will evaluate the module, try to locate the
top-level awaits, and print their location to help users fix them.

With this option alone, the module being `require()`'d should be
explicitly marked as an ES module either using the `"type": "module"`
field in `package.json` or the `.mjs` extension. To load implicit ES
modules ending with `.js` using automatic syntax-based module type
detection, use `--experimental-require-module-with-detection`.

### `--experimental-require-module-with-detection`

Expand Down Expand Up @@ -1611,7 +1615,7 @@ changes:

Identical to `-e` but prints the result.

### `--print-pending-tla`
### `--experimental-print-required-tla`

<!-- YAML
added: REPLACEME
Expand Down Expand Up @@ -2602,7 +2606,7 @@ Node.js options that are allowed are:
* `--policy-integrity`
* `--preserve-symlinks-main`
* `--preserve-symlinks`
* `--print-pending-tla`
* `--experimental-print-required-tla`
* `--prof-process`
* `--redirect-warnings`
* `--report-compact`
Expand Down
4 changes: 2 additions & 2 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2496,8 +2496,8 @@ object.
When trying to `require()` a [ES Module][] under `--experimental-require-module`,
the module turns out to be asynchronous. That is, it contains top-level await.
To see where the top-level await is, use
`--print-pending-tla` (this would execute the modules before looking for
the top-level awaits).
`--experimental-print-required-tla` (this would execute the modules
before looking for the top-level awaits).

<a id="ERR_REQUIRE_ESM"></a>

Expand Down
9 changes: 5 additions & 4 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -571,9 +571,10 @@ void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
}
}

// If --print-pending-tla is true, proceeds to evaluation even if it's
// async because we want to search for the TLA and help users locate them.
if (module->IsGraphAsync() && !env->options()->print_pending_tla) {
// If --experimental-print-required-tla is true, proceeds to evaluation even
// if it's async because we want to search for the TLA and help users locate
// them.
if (module->IsGraphAsync() && !env->options()->print_required_tla) {
THROW_ERR_REQUIRE_ASYNC_MODULE(env);
return;
}
Expand Down Expand Up @@ -607,7 +608,7 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
}

if (module->IsGraphAsync()) {
CHECK(env->options()->print_pending_tla);
CHECK(env->options()->print_required_tla);
auto stalled = module->GetStalledTopLevelAwaitMessage(isolate);
if (stalled.size() != 0) {
for (auto pair : stalled) {
Expand Down
2 changes: 1 addition & 1 deletion src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ ERRORS_WITH_CODE(V)
V(ERR_REQUIRE_ASYNC_MODULE, \
"require() cannot be used on an ESM graph with top-level await. Use " \
"import() instead. To see where the top-level await comes from, use " \
"--print-pending-tla.") \
"--experimental-print-required-tla.") \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, \
"Script execution was interrupted by `SIGINT`") \
V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, "Failed to set PSK identity hint") \
Expand Down
4 changes: 2 additions & 2 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"ES module syntax, try again to evaluate them as ES modules",
&EnvironmentOptions::detect_module,
kAllowedInEnvvar);
AddOption("--print-pending-tla",
AddOption("--experimental-print-required-tla",
"Print pending top-level await. If --experimental-require-module "
"is true, evaluate asynchronous graphs loaded by `require()` but "
"do not run the microtasks, in order to to find and print "
"top-level await in the graph",
&EnvironmentOptions::print_pending_tla,
&EnvironmentOptions::print_required_tla,
kAllowedInEnvvar);
AddOption("--experimental-require-module",
"Allow loading explicit ES Modules in require().",
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class EnvironmentOptions : public Options {
bool abort_on_uncaught_exception = false;
std::vector<std::string> conditions;
bool detect_module = false;
bool print_pending_tla = false;
bool print_required_tla = false;
bool require_module = false;
bool require_module_with_detection = false;
std::string dns_result_order;
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-require-module-tla.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ assert.throws(() => {
{
spawnSyncAndExit(process.execPath, [
'--experimental-require-module',
'--print-pending-tla',
'--experimental-print-required-tla',
fixtures.path('es-modules/tla/require-execution.js'),
], {
signal: null,
Expand Down

0 comments on commit a235a15

Please sign in to comment.