Skip to content

Commit 8d5469c

Browse files
fasttimeUlisesGascon
authored andcommitted
esm: do not call getSource when format is commonjs
Ensure that `defaultLoad` does not uselessly access the file system to get the source of modules that are known to be in CommonJS format. This allows CommonJS imports to resolve in the current phase of the event loop. Refs: eslint/eslint#17683 PR-URL: #50465 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 1862235 commit 8d5469c

File tree

6 files changed

+69
-9
lines changed

6 files changed

+69
-9
lines changed

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -132,20 +132,21 @@ async function defaultLoad(url, context = kEmptyObject){
132132
if(urlInstance.protocol==='node:'){
133133
source=null;
134134
format??='builtin';
135-
}else{
136-
letcontextToPass=context;
135+
}elseif(format!=='commonjs'||defaultType==='module'){
137136
if(source==null){
138137
({ responseURL, source }=awaitgetSource(urlInstance,context));
139-
contextToPass={__proto__: context, source };
138+
context={__proto__: context, source };
140139
}
141140

142-
// Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax.
143-
format??=awaitdefaultGetFormat(urlInstance,contextToPass);
141+
if(format==null){
142+
// Now that we have the source for the module, run `defaultGetFormat` to detect its format.
143+
format=awaitdefaultGetFormat(urlInstance,context);
144144

145-
if(format==='commonjs'&&contextToPass!==context&&defaultType!=='module'){
146-
// For backward compatibility reasons, we need to discard the source in
147-
// order for the CJS loader to re-fetch it.
148-
source=null;
145+
if(format==='commonjs'){
146+
// For backward compatibility reasons, we need to discard the source in
147+
// order for the CJS loader to re-fetch it.
148+
source=null;
149+
}
149150
}
150151
}
151152

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
constfixtures=require('../common/fixtures');
5+
constassert=require('node:assert');
6+
7+
(async()=>{
8+
9+
// Make sure that the CommonJS module lexer has been initialized.
10+
// See https://github.com/nodejs/node/blob/v21.1.0/lib/internal/modules/esm/translators.js#L61-L81.
11+
awaitimport(fixtures.fileURL('empty.js'));
12+
13+
lettickDuringCJSImport=false;
14+
process.nextTick(()=>{tickDuringCJSImport=true;});
15+
awaitimport(fixtures.fileURL('empty.cjs'));
16+
17+
assert(!tickDuringCJSImport);
18+
19+
})().then(common.mustCall());
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import'../common/index.mjs';
2+
importfixturesfrom'../common/fixtures.js';
3+
importassertfrom'node:assert';
4+
5+
lettickDuringCJSImport=false;
6+
process.nextTick(()=>{tickDuringCJSImport=true;});
7+
awaitimport(fixtures.fileURL('empty.cjs'));
8+
9+
assert(!tickDuringCJSImport);
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Flags: --experimental-loader ./test/fixtures/es-module-loaders/preset-cjs-source.mjs
2+
import'../common/index.mjs';
3+
import*asfixturesfrom'../common/fixtures.mjs';
4+
importassertfrom'assert';
5+
6+
const{default: existingFileSource}=awaitimport(fixtures.fileURL('es-modules','cjs-file.cjs'));
7+
const{default: noSuchFileSource}=awaitimport(newURL('./no-such-file.cjs',import.meta.url));
8+
9+
assert.strictEqual(existingFileSource,'no .cjs file was read to get this source');
10+
assert.strictEqual(noSuchFileSource,'no .cjs file was read to get this source');

‎test/fixtures/empty.cjs‎

Whitespace-only changes.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
exportfunctionresolve(specifier,context,next){
2+
if(specifier.endsWith('/no-such-file.cjs')){
3+
// Shortcut to avoid ERR_MODULE_NOT_FOUND for non-existing file, but keep the url for the load hook.
4+
return{
5+
shortCircuit: true,
6+
url: specifier,
7+
};
8+
}
9+
returnnext(specifier);
10+
}
11+
12+
exportfunctionload(href,context,next){
13+
if(href.endsWith('.cjs')){
14+
return{
15+
format: 'commonjs',
16+
shortCircuit: true,
17+
source: 'module.exports = "no .cjs file was read to get this source"',
18+
};
19+
}
20+
returnnext(href);
21+
}

0 commit comments

Comments
(0)