GeezerVison is showing github.com — esm: fix cache collision on JSON files using file: URL · nodejs/node@c48cd84
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/esm/translators.js +7-3Lines changed: 7 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -11,6 +11,7 @@ const { | 11 | 11 | SafeArrayIterator, | 12 | 12 | SafeMap, | 13 | 13 | SafeSet, | | 14 | + StringPrototypeIncludes, | 14 | 15 | StringPrototypeReplaceAll, | 15 | 16 | StringPrototypeSlice, | 16 | 17 | StringPrototypeStartsWith, | @@ -443,9 +444,12 @@ translators.set('json', function jsonStrategy(url, source) { | 443 | 444 | debug(`Loading JSONModule ${url}`); | 444 | 445 | const pathname = StringPrototypeStartsWith(url, 'file:') ? | 445 | 446 | fileURLToPath(url) : null; | | 447 | + const shouldCheckAndPopulateCJSModuleCache = | | 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, '#'); | 446 | 450 | let modulePath; | 447 | 451 | let module; | 448 | | - if (pathname) { | | 452 | + if (shouldCheckAndPopulateCJSModuleCache) { | 449 | 453 | modulePath = isWindows ? | 450 | 454 | StringPrototypeReplaceAll(pathname, '/', '\\') : pathname; | 451 | 455 | module = CJSModule._cache[modulePath]; | @@ -457,7 +461,7 @@ translators.set('json', function jsonStrategy(url, source) { | 457 | 461 | } | 458 | 462 | } | 459 | 463 | source = stringify(source); | 460 | | - if (pathname) { | | 464 | + if (shouldCheckAndPopulateCJSModuleCache) { | 461 | 465 | // A require call could have been called on the same file during loading and | 462 | 466 | // that resolves synchronously. To make sure we always return the identical | 463 | 467 | // export, we have to check again if the module already exists or not. | @@ -484,7 +488,7 @@ translators.set('json', function jsonStrategy(url, source) { | 484 | 488 | err.message = errPath(url) + ': ' + err.message; | 485 | 489 | throw err; | 486 | 490 | } | 487 | | - if (pathname) { | | 491 | + if (shouldCheckAndPopulateCJSModuleCache) { | 488 | 492 | CJSModule._cache[modulePath] = module; | 489 | 493 | } | 490 | 494 | cjsCache.set(url, module); | |
Collapse file Copy file name to clipboardExpand all lines: test/es-module/test-esm-json.mjs +61-1Lines changed: 61 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -2,7 +2,10 @@ import { spawnPromisified } from '../common/index.mjs'; | 2 | 2 | import * as fixtures from '../common/fixtures.mjs'; | 3 | 3 | import assert from 'node:assert'; | 4 | 4 | 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 * as tmpdir from '../common/tmpdir.js'; | 6 | 9 |
| 7 | 10 | import secret from '../fixtures/experimental.json' assert { type: 'json' }; | 8 | 11 |
| @@ -21,4 +24,61 @@ describe('ESM: importing JSON', () => { | 21 | 24 | assert.strictEqual(code, 0); | 22 | 25 | assert.strictEqual(signal, null); | 23 | 26 | }); | | 27 | + | | 28 | + test('should load different modules when the URL is different', async (t) => { | | 29 | + const root = tmpdir.fileURL(`./test-esm-json-${Math.random()}/`); | | 30 | + try { | | 31 | + await mkdir(root, { recursive: true }); | | 32 | + | | 33 | + await t.test('json', async () => { | | 34 | + let i = 0; | | 35 | + const url = new URL('./foo.json', root); | | 36 | + await writeFile(url, JSON.stringify({ id: i++ })); | | 37 | + const absoluteURL = await import(`${url}`, { | | 38 | + assert: { type: 'json' }, | | 39 | + }); | | 40 | + await writeFile(url, JSON.stringify({ id: i++ })); | | 41 | + const queryString = await import(`${url}?a=2`, { | | 42 | + assert: { type: 'json' }, | | 43 | + }); | | 44 | + await writeFile(url, JSON.stringify({ id: i++ })); | | 45 | + const hash = await import(`${url}#a=2`, { | | 46 | + assert: { type: 'json' }, | | 47 | + }); | | 48 | + await writeFile(url, JSON.stringify({ id: i++ })); | | 49 | + const queryStringAndHash = await import(`${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 | + await t.test('js', async () => { | | 62 | + let i = 0; | | 63 | + const url = new URL('./foo.mjs', root); | | 64 | + await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`); | | 65 | + const absoluteURL = await import(`${url}`); | | 66 | + await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`); | | 67 | + const queryString = await import(`${url}?a=1`); | | 68 | + await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`); | | 69 | + const hash = await import(`${url}#a=1`); | | 70 | + await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`); | | 71 | + const queryStringAndHash = await import(`${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 | + await rm(root, { force: true, recursive: true }); | | 82 | + } | | 83 | + }); | 24 | 84 | }); |
Collapse file Copy file name to clipboard +30Lines changed: 30 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +import '../common/index.mjs'; | | 2 | +import * as fixtures from '../common/fixtures.mjs'; | | 3 | +import { register } from 'node:module'; | | 4 | +import assert from 'node:assert'; | | 5 | + | | 6 | +async function resolve(referrer, context, next) { | | 7 | + const result = await next(referrer, context); | | 8 | + const url = new URL(result.url); | | 9 | + url.searchParams.set('randomSeed', Math.random()); | | 10 | + result.url = url.href; | | 11 | + return result; | | 12 | +} | | 13 | + | | 14 | +function load(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 | + return next(url, context); | | 23 | +} | | 24 | + | | 25 | +register(`data:text/javascript,export ${encodeURIComponent(resolve)};export ${encodeURIComponent(load)}`); | | 26 | + | | 27 | +assert.notDeepStrictEqual( | | 28 | + await import(fixtures.fileURL('empty.json'), { assert: { type: 'json' } }), | | 29 | + await import(fixtures.fileURL('empty.json'), { assert: { type: 'json' } }), | | 30 | +); |
|