Skip to content

Commit 843d5f8

Browse files
aduh95UlisesGascon
authored andcommitted
esm: fallback to getSource when load returns nullish source
When using the Modules Customization Hooks API to load CommonJS modules, we want to support the returned value of `defaultLoad` which must be nullish to preserve backward compatibility. This can be achieved by fetching the source from the translator. PR-URL: #50825Fixes: #50435 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
1 parent 8d5469c commit 843d5f8

File tree

3 files changed

+59
-12
lines changed

3 files changed

+59
-12
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,5 +262,6 @@ function throwUnknownModuleFormat(url, format){
262262
module.exports={
263263
defaultLoad,
264264
defaultLoadSync,
265+
getSourceSync,
265266
throwUnknownModuleFormat,
266267
};

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

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ function lazyTypes(){
3131
}
3232

3333
const{ containsModuleSyntax }=internalBinding('contextify');
34+
const{ BuiltinModule }=require('internal/bootstrap/realm');
3435
constassert=require('internal/assert');
3536
const{ readFileSync }=require('fs');
3637
const{ dirname, extname, isAbsolute }=require('path');
@@ -58,6 +59,17 @@ const asyncESM = require('internal/process/esm_loader');
5859
const{ emitWarningSync }=require('internal/process/warning');
5960
const{ internalCompileFunction }=require('internal/vm');
6061

62+
// Lazy-loading to avoid circular dependencies.
63+
letgetSourceSync;
64+
/**
65+
* @param{Parameters<typeof import('./load').getSourceSync>[0]} url
66+
* @returns{ReturnType<typeof import('./load').getSourceSync>}
67+
*/
68+
functiongetSource(url){
69+
getSourceSync??=require('internal/modules/esm/load').getSourceSync;
70+
returngetSourceSync(url);
71+
}
72+
6173
/** @type{import('deps/cjs-module-lexer/lexer.js').parse} */
6274
letcjsParse;
6375
/**
@@ -225,21 +237,19 @@ function loadCJSModule(module, source, url, filename){
225237
// eslint-disable-next-line func-name-matching,func-style
226238
constrequireFn=functionrequire(specifier){
227239
letimportAttributes=kEmptyObject;
228-
if(!StringPrototypeStartsWith(specifier,'node:')){
240+
if(!StringPrototypeStartsWith(specifier,'node:')&&!BuiltinModule.normalizeRequirableId(specifier)){
229241
// TODO: do not depend on the monkey-patchable CJS loader here.
230242
constpath=CJSModule._resolveFilename(specifier,module);
231-
if(specifier!==path){
232-
switch(extname(path)){
233-
case'.json':
234-
importAttributes={__proto__: null,type: 'json'};
235-
break;
236-
case'.node':
237-
returnCJSModule._load(specifier,module);
238-
default:
243+
switch(extname(path)){
244+
case'.json':
245+
importAttributes={__proto__: null,type: 'json'};
246+
break;
247+
case'.node':
248+
returnCJSModule._load(specifier,module);
249+
default:
239250
// fall through
240-
}
241-
specifier=`${pathToFileURL(path)}`;
242251
}
252+
specifier=`${pathToFileURL(path)}`;
243253
}
244254
constjob=asyncESM.esmLoader.getModuleJobSync(specifier,url,importAttributes);
245255
job.runSync();
@@ -276,7 +286,8 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule){
276286
debug(`Translating CJSModule ${url}`);
277287

278288
constfilename=StringPrototypeStartsWith(url,'file://') ? fileURLToPath(url) : url;
279-
source=stringify(source);
289+
// In case the source was not provided by the `load` step, we need fetch it now.
290+
source=stringify(source??getSource(newURL(url)).source);
280291

281292
const{ exportNames, module }=cjsPreparseModuleExports(filename,source);
282293
cjsCache.set(url,module);

‎test/es-module/test-esm-loader-hooks.mjs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,4 +736,39 @@ describe('Loader hooks',{concurrency: true }, () =>{
736736
assert.strictEqual(code,0);
737737
assert.strictEqual(signal,null);
738738
});
739+
740+
it('should handle mixed of opt-in modules and non-opt-in ones',async()=>{
741+
const{ code, signal, stdout, stderr }=awaitspawnPromisified(execPath,[
742+
'--no-warnings',
743+
'--experimental-loader',
744+
`data:text/javascript,const fixtures=${JSON.stringify(fixtures.path('empty.js'))};export ${
745+
encodeURIComponent(functionresolve(s,c,n){
746+
if(s.endsWith('entry-point')){
747+
return{
748+
shortCircuit: true,
749+
url: 'file:///c:/virtual-entry-point',
750+
format: 'commonjs',
751+
};
752+
}
753+
returnn(s,c);
754+
})
755+
}export ${
756+
encodeURIComponent(asyncfunctionload(u,c,n){
757+
if(u==='file:///c:/virtual-entry-point'){
758+
return{
759+
shortCircuit: true,
760+
source: `"use strict"require(${JSON.stringify(fixtures)});console.log("Hello");`,
761+
format: 'commonjs',
762+
};
763+
}
764+
returnn(u,c);
765+
})}`,
766+
'entry-point',
767+
]);
768+
769+
assert.strictEqual(stderr,'');
770+
assert.strictEqual(stdout,'Hello\n');
771+
assert.strictEqual(code,0);
772+
assert.strictEqual(signal,null);
773+
});
739774
});

0 commit comments

Comments
(0)