Skip to content

Commit c48cd84

Browse files
aduh95targos
authored andcommitted
esm: fix cache collision on JSON files using file: URL
PR-URL: #49887Fixes: #49724 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent dc80cce commit c48cd84

3 files changed

Lines changed: 98 additions & 4 deletions

File tree

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
SafeArrayIterator,
1212
SafeMap,
1313
SafeSet,
14+
StringPrototypeIncludes,
1415
StringPrototypeReplaceAll,
1516
StringPrototypeSlice,
1617
StringPrototypeStartsWith,
@@ -443,9 +444,12 @@ translators.set('json', function jsonStrategy(url, source) {
443444
debug(`Loading JSONModule ${url}`);
444445
constpathname=StringPrototypeStartsWith(url,'file:') ?
445446
fileURLToPath(url) : null;
447+
constshouldCheckAndPopulateCJSModuleCache=
448+
// We want to involve the CJS loader cache only for `file:` URL with no search query and no hash.
449+
pathname&&!StringPrototypeIncludes(url,'?')&&!StringPrototypeIncludes(url,'#');
446450
letmodulePath;
447451
letmodule;
448-
if(pathname){
452+
if(shouldCheckAndPopulateCJSModuleCache){
449453
modulePath=isWindows ?
450454
StringPrototypeReplaceAll(pathname,'/','\\') : pathname;
451455
module=CJSModule._cache[modulePath];
@@ -457,7 +461,7 @@ translators.set('json', function jsonStrategy(url, source) {
457461
}
458462
}
459463
source=stringify(source);
460-
if(pathname){
464+
if(shouldCheckAndPopulateCJSModuleCache){
461465
// A require call could have been called on the same file during loading and
462466
// that resolves synchronously. To make sure we always return the identical
463467
// export, we have to check again if the module already exists or not.
@@ -484,7 +488,7 @@ translators.set('json', function jsonStrategy(url, source) {
484488
err.message=errPath(url)+': '+err.message;
485489
throwerr;
486490
}
487-
if(pathname){
491+
if(shouldCheckAndPopulateCJSModuleCache){
488492
CJSModule._cache[modulePath]=module;
489493
}
490494
cjsCache.set(url,module);

‎test/es-module/test-esm-json.mjs‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ import { spawnPromisified } from '../common/index.mjs';
22
import*asfixturesfrom'../common/fixtures.mjs';
33
importassertfrom'node:assert';
44
import{execPath}from'node:process';
5-
import{describe,it}from'node:test';
5+
import{describe,it,test}from'node:test';
6+
7+
import{mkdir,rm,writeFile}from'node:fs/promises';
8+
import*astmpdirfrom'../common/tmpdir.js';
69

710
importsecretfrom'../fixtures/experimental.json'assert{type: 'json' };
811

@@ -21,4 +24,61 @@ describe('ESM: importing JSON', () => {
2124
assert.strictEqual(code,0);
2225
assert.strictEqual(signal,null);
2326
});
27+
28+
test('should load different modules when the URL is different',async(t)=>{
29+
constroot=tmpdir.fileURL(`./test-esm-json-${Math.random()}/`);
30+
try{
31+
awaitmkdir(root,{recursive: true});
32+
33+
awaitt.test('json',async()=>{
34+
leti=0;
35+
consturl=newURL('./foo.json',root);
36+
awaitwriteFile(url,JSON.stringify({id: i++}));
37+
constabsoluteURL=awaitimport(`${url}`,{
38+
assert: {type: 'json'},
39+
});
40+
awaitwriteFile(url,JSON.stringify({id: i++}));
41+
constqueryString=awaitimport(`${url}?a=2`,{
42+
assert: {type: 'json'},
43+
});
44+
awaitwriteFile(url,JSON.stringify({id: i++}));
45+
consthash=awaitimport(`${url}#a=2`,{
46+
assert: {type: 'json'},
47+
});
48+
awaitwriteFile(url,JSON.stringify({id: i++}));
49+
constqueryStringAndHash=awaitimport(`${url}?a=2#a=2`,{
50+
assert: {type: 'json'},
51+
});
52+
53+
assert.notDeepStrictEqual(absoluteURL,queryString);
54+
assert.notDeepStrictEqual(absoluteURL,hash);
55+
assert.notDeepStrictEqual(queryString,hash);
56+
assert.notDeepStrictEqual(absoluteURL,queryStringAndHash);
57+
assert.notDeepStrictEqual(queryString,queryStringAndHash);
58+
assert.notDeepStrictEqual(hash,queryStringAndHash);
59+
});
60+
61+
awaitt.test('js',async()=>{
62+
leti=0;
63+
consturl=newURL('./foo.mjs',root);
64+
awaitwriteFile(url,`export default ${JSON.stringify({id: i++})}\n`);
65+
constabsoluteURL=awaitimport(`${url}`);
66+
awaitwriteFile(url,`export default ${JSON.stringify({id: i++})}\n`);
67+
constqueryString=awaitimport(`${url}?a=1`);
68+
awaitwriteFile(url,`export default ${JSON.stringify({id: i++})}\n`);
69+
consthash=awaitimport(`${url}#a=1`);
70+
awaitwriteFile(url,`export default ${JSON.stringify({id: i++})}\n`);
71+
constqueryStringAndHash=awaitimport(`${url}?a=1#a=1`);
72+
73+
assert.notDeepStrictEqual(absoluteURL,queryString);
74+
assert.notDeepStrictEqual(absoluteURL,hash);
75+
assert.notDeepStrictEqual(queryString,hash);
76+
assert.notDeepStrictEqual(absoluteURL,queryStringAndHash);
77+
assert.notDeepStrictEqual(queryString,queryStringAndHash);
78+
assert.notDeepStrictEqual(hash,queryStringAndHash);
79+
});
80+
}finally{
81+
awaitrm(root,{force: true,recursive: true});
82+
}
83+
});
2484
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import'../common/index.mjs';
2+
import*asfixturesfrom'../common/fixtures.mjs';
3+
import{register}from'node:module';
4+
importassertfrom'node:assert';
5+
6+
asyncfunctionresolve(referrer,context,next){
7+
constresult=awaitnext(referrer,context);
8+
consturl=newURL(result.url);
9+
url.searchParams.set('randomSeed',Math.random());
10+
result.url=url.href;
11+
returnresult;
12+
}
13+
14+
functionload(url,context,next){
15+
if(context.importAssertions.type==='json'){
16+
return{
17+
shortCircuit: true,
18+
format: 'json',
19+
source: JSON.stringify({data: Math.random()}),
20+
};
21+
}
22+
returnnext(url,context);
23+
}
24+
25+
register(`data:text/javascript,export ${encodeURIComponent(resolve)};export ${encodeURIComponent(load)}`);
26+
27+
assert.notDeepStrictEqual(
28+
awaitimport(fixtures.fileURL('empty.json'),{assert: {type: 'json'}}),
29+
awaitimport(fixtures.fileURL('empty.json'),{assert: {type: 'json'}}),
30+
);

0 commit comments

Comments
 (0)