Skip to content

Commit bfc68c8

Browse files
himself65joyeecheung
authored andcommitted
module: convert schema-only core module on convertCJSFilenameToURL
Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: #58612Fixes: #58607 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent e8f4a7d commit bfc68c8

3 files changed

Lines changed: 93 additions & 4 deletions

File tree

‎lib/internal/modules/customization_hooks.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
ArrayPrototypeSplice,
77
ObjectAssign,
88
ObjectFreeze,
9+
StringPrototypeSlice,
910
StringPrototypeStartsWith,
1011
Symbol,
1112
}=primordials;
@@ -126,9 +127,12 @@ function registerHooks(hooks) {
126127
*/
127128
functionconvertCJSFilenameToURL(filename){
128129
if(!filename){returnfilename;}
129-
constbuiltinId=BuiltinModule.normalizeRequirableId(filename);
130-
if(builtinId){
131-
return`node:${builtinId}`;
130+
letnormalizedId=filename;
131+
if(StringPrototypeStartsWith(filename,'node:')){
132+
normalizedId=StringPrototypeSlice(filename,5);
133+
}
134+
if(BuiltinModule.canBeRequiredByUsers(normalizedId)){
135+
return`node:${normalizedId}`;
132136
}
133137
// Handle the case where filename is neither a path, nor a built-in id,
134138
// which is possible via monkey-patching.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
'use strict';
2+
3+
// This tests that when builtins that demand the `node:` prefix are
4+
// required, the URL returned by the default resolution step is still
5+
// prefixed and valid, and that gets passed to the load hook is still
6+
// the one with the `node:` prefix. The one with the prefix
7+
// stripped for internal lookups should not get passed into the hooks.
8+
9+
constcommon=require('../common');
10+
constassert=require('assert');
11+
const{ registerHooks }=require('module');
12+
13+
constschemelessBlockList=newSet([
14+
'sea',
15+
'sqlite',
16+
'test',
17+
'test/reporters',
18+
]);
19+
20+
consttestModules=[];
21+
for(constmodofschemelessBlockList){
22+
testModules.push(`node:${mod}`);
23+
}
24+
25+
consthook=registerHooks({
26+
resolve: common.mustCall((specifier,context,nextResolve)=>{
27+
constresult=nextResolve(specifier,context);
28+
assert.match(result.url,/^node:/);
29+
assert.strictEqual(schemelessBlockList.has(result.url.slice(5,result.url.length)),true);
30+
returnresult;
31+
},testModules.length),
32+
load: common.mustCall(functionload(url,context,nextLoad){
33+
assert.match(url,/^node:/);
34+
assert.strictEqual(schemelessBlockList.has(url.slice(5,url.length)),true);
35+
constresult=nextLoad(url,context);
36+
assert.strictEqual(result.source,null);
37+
return{
38+
source: 'throw new Error("I should not be thrown because the loader ignores user-supplied source for builtins")',
39+
format: 'builtin',
40+
};
41+
},testModules.length),
42+
});
43+
44+
for(constmodoftestModules){
45+
require(mod);
46+
}
47+
48+
hook.deregister();

‎test/module-hooks/test-module-hooks-load-builtin-require.js‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ const { registerHooks } = require('module');
1111

1212
// Pick a builtin that's unlikely to be loaded already - like zlib.
1313
assert(!process.moduleLoadList.includes('NativeModule zlib'));
14+
lethook;
1415

15-
consthook=registerHooks({
16+
hook=registerHooks({
1617
load: common.mustCall(functionload(url,context,nextLoad){
1718
assert.strictEqual(url,'node:zlib');
1819
constresult=nextLoad(url,context);
@@ -27,3 +28,39 @@ const hook = registerHooks({
2728
assert.strictEqual(typeofrequire('zlib').createGzip,'function');
2829

2930
hook.deregister();
31+
32+
// This tests that when builtins that demand the `node:` prefix are
33+
// required, the URL returned by the default resolution step is still
34+
// prefixed and valid, and that gets passed to the load hook is still
35+
// the one with the `node:` prefix. The one with the prefix
36+
// stripped for internal lookups should not get passed into the hooks.
37+
constschemelessBlockList=newSet([
38+
'sea',
39+
'sqlite',
40+
'test',
41+
'test/reporters',
42+
]);
43+
44+
consttestModules=[];
45+
for(constmodofschemelessBlockList){
46+
testModules.push(`node:${mod}`);
47+
}
48+
49+
hook=registerHooks({
50+
load: common.mustCall(functionload(url,context,nextLoad){
51+
assert.match(url,/^node:/);
52+
assert.strictEqual(schemelessBlockList.has(url.slice(5,url.length)),true);
53+
constresult=nextLoad(url,context);
54+
assert.strictEqual(result.source,null);
55+
return{
56+
source: 'throw new Error("I should not be thrown because the loader ignores user-supplied source for builtins")',
57+
format: 'builtin',
58+
};
59+
},testModules.length),
60+
});
61+
62+
for(constmodoftestModules){
63+
require(mod);
64+
}
65+
66+
hook.deregister();

0 commit comments

Comments
 (0)