From 06fde1dcdbb79bfb38087f7a390814333dbdb192 Mon Sep 17 00:00:00 2001 From: Avocado Date: Wed, 26 Aug 2026 14:49:46 +0900 Subject: [PATCH] esm: fix relative require in non-file CJS source When a load hook provides the source of a CommonJS module, the require calls in that source are meant to go through the ESM loader so that the registered hooks apply to them, as documented for the load hook. That did not happen when the module had a URL that is not a file:. The require function built for those modules called Module._resolveFilename() before handing the specifier to the ESM resolver, only to look at the extension of the resulting path and decide whether to treat the request as JSON or as a native addon. Because the CJS resolver works on file paths, it cannot resolve a relative specifier against a referrer such as custom:cjs, so it threw MODULE_NOT_FOUND and the ESM resolution below it was never reached. The resolve hook was not called at all. Keep that lookup, since the JSON and .node branches depend on it, but let it fail when the referrer is not a file and leave the specifier for the ESM resolver to handle. A specifier that the hooks do not claim now fails with ERR_UNSUPPORTED_RESOLVE_REQUEST, which reports that the referrer scheme is not hierarchical, instead of MODULE_NOT_FOUND coming from the CJS resolver. Signed-off-by: Avocado --- lib/internal/modules/esm/translators.js | 37 +++++++++++++------ ...esm-loader-cjs-source-relative-require.mjs | 17 +++++++++ .../es-module-loaders/non-file-cjs-source.mjs | 25 +++++++++++++ 3 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 test/es-module/test-esm-loader-cjs-source-relative-require.mjs create mode 100644 test/fixtures/es-module-loaders/non-file-cjs-source.mjs diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index e3baaaefa6c0..1a7840e96dc5 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -129,19 +129,32 @@ function loadCJSModuleWithSpecialRequire(module, source, url, filename, isMain, let importAttributes = kEmptyObject; if (!StringPrototypeStartsWith(specifier, 'node:') && !BuiltinModule.normalizeRequirableId(specifier)) { // TODO: do not depend on the monkey-patchable CJS loader here. - const path = CJSModule._resolveFilename(specifier, module); - switch (extname(path)) { - case '.json': - importAttributes = { __proto__: null, type: 'json' }; - break; - case '.node': - // If it gets here in the translators, the hooks must have already been invoked - // in the loader. Skip them in the synthetic module evaluation step. - return wrapModuleLoad(specifier, module, false, kShouldSkipModuleHooks); - default: - // fall through + // The CJS resolver works on file paths, so it cannot resolve a specifier whose + // referrer is not a file - that happens when the source of this module comes from + // a load hook that gave it a non-file URL. Leave the specifier alone in that case + // and let the ESM resolver below run the hooks on it. + let path; + try { + path = CJSModule._resolveFilename(specifier, module); + } catch (err) { + if (err?.code !== 'MODULE_NOT_FOUND' || StringPrototypeStartsWith(url, 'file:')) { + throw err; + } + } + if (path !== undefined) { + switch (extname(path)) { + case '.json': + importAttributes = { __proto__: null, type: 'json' }; + break; + case '.node': + // If it gets here in the translators, the hooks must have already been invoked + // in the loader. Skip them in the synthetic module evaluation step. + return wrapModuleLoad(specifier, module, false, kShouldSkipModuleHooks); + default: + // fall through + } + specifier = `${pathToFileURL(path)}`; } - specifier = `${pathToFileURL(path)}`; } // NOTE: This re-invented require() is only used on the loader-hook worker thread. diff --git a/test/es-module/test-esm-loader-cjs-source-relative-require.mjs b/test/es-module/test-esm-loader-cjs-source-relative-require.mjs new file mode 100644 index 000000000000..1ba64c9f704f --- /dev/null +++ b/test/es-module/test-esm-loader-cjs-source-relative-require.mjs @@ -0,0 +1,17 @@ +// Flags: --experimental-loader ./test/fixtures/es-module-loaders/non-file-cjs-source.mjs +import '../common/index.mjs'; +import assert from 'node:assert'; + +// When a load hook provides the source of a CommonJS module, the `require` calls +// that source makes are resolved by the ESM loader, so registered hooks apply to +// them. That has to keep working when the module has a URL that is not a file: +// the CJS resolver cannot resolve a relative specifier against such a referrer. + +const { default: fromHookedRequire } = await import('custom:entry'); +assert.strictEqual(fromHookedRequire, 'loaded through the hooks'); + +// When the hooks do not claim the specifier either, the failure has to come from +// the ESM resolver rather than from the CJS one. +await assert.rejects(import('custom:missing-dep'), { + code: 'ERR_UNSUPPORTED_RESOLVE_REQUEST', +}); diff --git a/test/fixtures/es-module-loaders/non-file-cjs-source.mjs b/test/fixtures/es-module-loaders/non-file-cjs-source.mjs new file mode 100644 index 000000000000..7436e7e30fd4 --- /dev/null +++ b/test/fixtures/es-module-loaders/non-file-cjs-source.mjs @@ -0,0 +1,25 @@ +// Serves CommonJS sources under a non-file URL scheme, so that the `require` +// calls they contain have a referrer that the CJS resolver cannot handle. + +const sources = { + 'custom:entry': 'module.exports = require("./dep");', + 'custom:dep': 'module.exports = "loaded through the hooks";', + 'custom:missing-dep': 'module.exports = require("./no-such-dep");', +}; + +export function resolve(specifier, context, next) { + if (specifier in sources) { + return { shortCircuit: true, url: specifier }; + } + if (specifier === './dep') { + return { shortCircuit: true, url: 'custom:dep' }; + } + return next(specifier, context); +} + +export function load(url, context, next) { + if (url in sources) { + return { shortCircuit: true, format: 'commonjs', source: sources[url] }; + } + return next(url, context); +}