Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 9e39360

Browse files
joyeecheungaduh95
authored andcommitted
esm: improve ERR_REQUIRE_ASYNC_MODULE
This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: <url>" to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #64260 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b02e4 commit 9e39360

24 files changed

Lines changed: 189 additions & 242 deletions

‎doc/api/errors.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required.
27652765

27662766
### `ERR_REQUIRE_ASYNC_MODULE`
27672767

2768+
<!-- YAML
2769+
changes:
2770+
- version: REPLACEME
2771+
pr-url: https://github.com/nodejs/node/pull/64260
2772+
description: Added the `requireStack` and `topLevelAwaitLocations` properties.
2773+
-->
2774+
27682775
When trying to `require()` an [ES Module][], the module turns out to be asynchronous.
27692776
That is, it contains top-level await.
27702777

2771-
To see where the top-level await is, use
2772-
`--experimental-print-required-tla` (this would execute the modules
2773-
before looking for the top-level awaits).
2778+
When uncaught, the flag `--experimental-print-required-tla` prints
2779+
the locations of the top-level awaits in the graph to stderr.
2780+
2781+
This error has the following additional non-enumerable properties:
2782+
2783+
*`requireStack` {string\[]} The chain of modules that led to the failing
2784+
`require()`, starting with the module that required the asynchronous module.
2785+
*`topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in
2786+
the graph. Only populated when `--experimental-print-required-tla` is enabled.
2787+
Each entry has the following properties:
2788+
*`url` {string} The URL of the module containing the top-level await.
2789+
*`line` {number} The 1-based line number of the top-level await.
2790+
*`column` {number} The 1-based column number of the top-level await.
2791+
*`sourceLine` {string} The source line containing the top-level await.
27742792

27752793
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
27762794

‎doc/api/modules.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`,
317317
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
318318
load the asynchronous module using [`import()`][].
319319

320-
If `--experimental-print-required-tla` is enabled, instead of throwing
321-
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
322-
module, try to locate the top-level awaits, and print their location to
323-
help users fix them.
320+
If `--experimental-print-required-tla` is enabled and the error is uncaught,
321+
Node.js will try to locate the top-level `await`s in the `require()`'d module graph
322+
and print the locations in the stderr.
324323

325324
If support for loading ES modules using `require()` results in unexpected
326325
breakage, it can be disabled using `--no-require-module`.

‎lib/internal/errors.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,20 +1699,36 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
16991699
if(!getOptionValue('--experimental-print-required-tla')){
17001700
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702+
if(filename){
1703+
message+=`\nRequired module: ${filename}`;
1704+
}
17021705
if(parent){
17031706
const{ getRequireStack }=require('internal/modules/helpers');
17041707
constrequireStack=getRequireStack(parent);
17051708
if(requireStack.length>0){
17061709
message+='\nRequire stack:\n- '+
17071710
ArrayPrototypeJoin(requireStack,'\n- ');
17081711
}
1709-
this.requireStack=requireStack;
1712+
ObjectDefineProperty(this,'requireStack',{
1713+
__proto__: null,
1714+
enumerable: false,
1715+
configurable: true,
1716+
writable: true,
1717+
value: requireStack,
1718+
});
17101719
}
17111720
if(locations&&locations.length>0){
17121721
const{ urlToFilename }=require('internal/modules/helpers');
17131722
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1714-
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1723+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column-1)}^\n`);
17151724
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
1725+
ObjectDefineProperty(this,'topLevelAwaitLocations',{
1726+
__proto__: null,
1727+
enumerable: false,
1728+
configurable: true,
1729+
writable: true,
1730+
value: locations,
1731+
});
17161732
}
17171733
returnmessage;
17181734
},Error);

‎lib/internal/modules/esm/loader.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class ModuleLoader {
288288
conststatus=job.module.getStatus();
289289
debug('Module status',job,status);
290290
// hasAsyncGraph is available after module been instantiated.
291-
if(status>=kInstantiated&&job.module.hasAsyncGraph){
292-
job.throwAsyncGraphError(parent);
291+
if(status>=kInstantiated){
292+
job.throwIfAsyncGraph(parent);
293293
}
294294
if(status===kEvaluated){
295295
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -317,9 +317,11 @@ class ModuleLoader {
317317
}
318318
if(status!==kEvaluating){
319319
assert(status===kUninstantiated,`Unexpected module status ${status}`);
320-
// A previous require() of the same graph may have bailed out before
321-
// instantiation because it contains top-level await.
322-
job.throwIfAsyncGraph(parent);
320+
// If we get here, either there's a race where the job is still being instantiated
321+
// by an in-flight import(), or the cached module previously encountered an
322+
// instantiation error during a prior load (e.g. due to a mismatched import).
323+
// TODO(joyeecheung): the current check is too broad. We should attempt to
324+
// get the potential instantiation error and throw it.
323325
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
324326
}
325327
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 61 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const{
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeFind,
67
ArrayPrototypeJoin,
78
ArrayPrototypePop,
@@ -14,6 +15,7 @@ const {
1415
PromiseResolve,
1516
RegExpPrototypeExec,
1617
RegExpPrototypeSymbolReplace,
18+
RegExpPrototypeSymbolSplit,
1719
SafePromiseAllReturnArrayLike,
1820
SafePromiseAllReturnVoid,
1921
SafeSet,
@@ -36,8 +38,10 @@ const {
3638
kUninstantiated,
3739
}=internalBinding('module_wrap');
3840
const{
41+
getPromiseDetails,
3942
privateSymbols: {
4043
entry_point_module_private_symbol,
44+
module_source_private_symbol: kModuleSource,
4145
},
4246
}=internalBinding('util');
4347
/**
@@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
134138
* @typedef {object} TopLevelAwaitLocation
135139
* @property {string} url URL of the module containing the top-level await.
136140
* @property {number} line 1-based line number of the top-level await.
137-
* @property {number} column 0-based column number of the top-level await.
141+
* @property {number} column 1-based column number of the top-level await.
138142
* @property {string} sourceLine The source line containing the top-level await.
139143
*/
140144

141145
/**
142-
* Locate the top-level awaits in the given module by parsing the source with acron.
146+
* Locate the top-level awaits in the given module by parsing the source with acorn.
143147
* @param {string} source Module source code.
144148
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145149
*/
@@ -173,27 +177,62 @@ function findTopLevelAwait(source) {
173177
returnfound;
174178
}
175179

180+
/**
181+
* Collect the modules that contain top-level await in the linked graph of a job.
182+
* @param {ModuleJobBase} root The root of the module graph to search.
183+
* @returns {ModuleWrap[]} Modules that contain top-level await.
184+
*/
185+
functionfindModulesWithTopLevelAwait(root){
186+
constfound=[];
187+
constseen=newSafeSet();
188+
conststack=[root];
189+
while(stack.length>0){
190+
constjob=ArrayPrototypePop(stack);
191+
if(seen.has(job)){continue;}
192+
seen.add(job);
193+
if(job.module?.hasTopLevelAwait){
194+
ArrayPrototypePush(found,job.module);
195+
}
196+
letlinked=job.linked;
197+
if(isPromise(linked)){
198+
linked=getPromiseDetails(linked)?.[1];
199+
}
200+
// If `require(esm)` comes from the deprecated async loader hook worker thread,
201+
// linked may be pending at this point. In that case, this branch would be skipped -
202+
// we just allow lossy reporting of TLA locations in an edge case when a deprecated
203+
// feature is used in combination with another experimental flag.
204+
if(ArrayIsArray(linked)){
205+
for(leti=0;i<linked.length;i++){
206+
ArrayPrototypePush(stack,linked[i]);
207+
}
208+
}
209+
}
210+
returnfound;
211+
}
212+
176213
/**
177214
* Locate the top-level awaits in the given modules.
178-
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
215+
* @param {ModuleJobBase} root The root of the module graph to search.
179216
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180217
*/
181-
functiongetTopLevelAwaitLocations(modules){
218+
functiongetTopLevelAwaitLocations(root){
219+
constmodules=findModulesWithTopLevelAwait(root);
182220
constlocations=[];
183221
for(leti=0;i<modules.length;i++){
184222
constmodule=modules[i];
185-
constsource=module.source;
223+
constsource=module[kModuleSource];
186224
if(typeofsource!=='string'){continue;}// Not retained during compilation. Skip.
187225
constfound=findTopLevelAwait(source);
188226
if(found.length===0){continue;}
189-
constlines=StringPrototypeSplit(source,'\n');
227+
constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source);
190228
for(letj=0;j<found.length;j++){
191229
const{ start }=found[j].loc;
192230
ArrayPrototypePush(locations,{
193231
__proto__: null,
194232
url: module.url,
195233
line: start.line,
196-
column: start.column,
234+
// Acorn reports 0-based columns, convert them to 1-based to match `line`.
235+
column: start.column+1,
197236
sourceLine: lines[start.line-1],
198237
});
199238
}
@@ -260,61 +299,18 @@ class ModuleJobBase {
260299
}
261300

262301
/**
263-
* Collect the modules that contain top-level await in the linked graph of
264-
* this job. Whether each module contains top-level await is known at
265-
* compilation, so for a synchronously linked graph this finds asynchronous
266-
* graphs before instantiation.
267-
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268-
* which case the subgraphs that are not synchronously linked are skipped
269-
* and callers should still consult hasAsyncGraph after instantiation.
270-
* @returns {ModuleWrap[]}
271-
*/
272-
findModulesWithTopLevelAwait(){
273-
constfound=[];
274-
constseen=newSafeSet();
275-
conststack=[this];
276-
while(stack.length>0){
277-
constjob=ArrayPrototypePop(stack);
278-
if(seen.has(job)){continue;}
279-
seen.add(job);
280-
if(job.module?.hasTopLevelAwait){
281-
ArrayPrototypePush(found,job.module);
282-
}
283-
// job.linked is the array of evaluation-phase dependency jobs when the
284-
// linking is synchronous. Skip it if it's still a promise.
285-
if(!isPromise(job.linked)){
286-
for(leti=0;i<job.linked.length;i++){
287-
ArrayPrototypePush(stack,job.linked[i]);
288-
}
289-
}
290-
}
291-
returnfound;
292-
}
293-
294-
/**
295-
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296-
* contains top-level await.
297-
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298-
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299-
* collected by the caller, to avoid walking the graph again.
300-
*/
301-
throwAsyncGraphError(parent,modules=this.findModulesWithTopLevelAwait()){
302-
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303-
constfilename=urlToFilename(this.url);
304-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
305-
}
306-
307-
/**
308-
* If the a require()'d graph contains top-level await, collect the source locations
302+
* If the require()'d graph contains top-level await, collect the source locations
309303
* of the top-level awaits using source code retained during compilation and throw
310-
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
304+
* ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated.
311305
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312306
*/
313307
throwIfAsyncGraph(parent){
314-
constmodules=this.findModulesWithTopLevelAwait();
315-
if(modules.length>0){
316-
this.throwAsyncGraphError(parent,modules);
308+
if(!this.module.hasAsyncGraph){
309+
return;
317310
}
311+
constlocations=getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : [];
312+
constfilename=urlToFilename(this.url);
313+
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parent,locations);
318314
}
319315

320316
/**
@@ -529,19 +525,15 @@ class ModuleJob extends ModuleJobBase {
529525
status=this.module.getStatus();
530526
}
531527
if(status===kInstantiated||status===kErrored){
532-
if(this.module.hasAsyncGraph){
533-
this.throwAsyncGraphError(parent);
534-
}
528+
this.throwIfAsyncGraph(parent);
535529
if(status===kInstantiated){
536530
setHasStartedUserESMExecution();
537531
constnamespace=this.module.evaluateSync();
538532
return{__proto__: null,module: this.module, namespace };
539533
}
540534
throwthis.module.getError();
541535
}elseif(status===kEvaluating||status===kEvaluated){
542-
if(this.module.hasAsyncGraph){
543-
this.throwAsyncGraphError(parent);
544-
}
536+
this.throwIfAsyncGraph(parent);
545537
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
546538
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
547539
// detected earlier during the linking phase, though the CJS handling in the ESM
@@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase {
637629
}
638630
return{__proto__: null,module: this.module};
639631
}elseif(status===kInstantiated||status===kUninstantiated){
640-
// The require() of this (synchronously linked) module bailed out: either
641-
// it was rejected for containing top-level await after instantiation
642-
// (kInstantiated), or its instantiation failed and left it uninstantiated
643-
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644-
// run() from import, finish the instantiation and evaluate it asynchronously,
645-
// re-throwing any instantiation error.
632+
// If we get here, the module was initially required and is now being imported.
633+
// The require() module failed either because the graph has TLA (kInstantiated),
634+
// or instantiation failed (kUninstantiated, e.g. missing named export).
635+
// Try finishing the instantiation - if it succeeds, proceed to evaluation,
636+
// otherwise the branch below re-throw any instantiation error.
646637
if(status===kUninstantiated){
647638
this.module.instantiate();
648639
}
@@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase {
668659
// On the deprecated async loader hook worker thread, dependencies linked by an
669660
// earlier import may not be walkable synchronously, so double-check with
670661
// V8 now that the graph is instantiated.
671-
if(this.module.hasAsyncGraph){
672-
this.throwAsyncGraphError(parent);
673-
}
662+
this.throwIfAsyncGraph(parent);
674663
setHasStartedUserESMExecution();
675664
try{
676665
constnamespace=this.module.evaluateSync();

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
const{
1212
privateSymbols: {
1313
host_defined_option_symbol,
14+
module_source_private_symbol: kModuleSource,
1415
},
1516
}=internalBinding('util');
1617
const{
@@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
332333
// only serves as a shortcut.
333334
if(wrap.hasTopLevelAwait&&
334335
getOptionValue('--experimental-print-required-tla')){
335-
wrap.source=source;
336+
wrap[kModuleSource]=source;
336337
}
337338

338339
// Cache the source map for the module if present.

0 commit comments

Comments
 (0)