diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index cd5c88dce8e021..d779bdf427553d 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -82,6 +82,17 @@ function underNodeModules(url) { return StringPrototypeIncludes(url.pathname, '/node_modules/'); } +/** + * Determine whether the given source contains CJS or ESM module syntax. + * @param {string | Buffer | undefined} source + * @param {URL} url + */ +function detectModuleFormat(source, url) { + const moduleSource = source ? `${source}` : undefined; + const modulePath = fileURLToPath(url); + return containsModuleSyntax(moduleSource, modulePath, url) ? 'module' : 'commonjs' +} + let typelessPackageJsonFilesWarnedAbout; /** * @param {URL} url @@ -113,9 +124,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE // `source` is undefined when this is called from `defaultResolve`; // but this gets called again from `defaultLoad`/`defaultLoadSync`. if (getOptionValue('--experimental-detect-module')) { - const format = source ? - (containsModuleSyntax(`${source}`, fileURLToPath(url), url) ? 'module' : 'commonjs') : - null; + const format = detectModuleFormat(source, url); if (format === 'module') { // This module has a .js extension, a package.json with no `type` field, and ESM syntax. // Warn about the missing `type` field so that the user can avoid the performance penalty of detection. @@ -155,12 +164,8 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE } default: { // The user did not pass `--experimental-default-type`. if (getOptionValue('--experimental-detect-module')) { - if (!source) { return null; } const format = getFormatOfExtensionlessFile(url); - if (format === 'module') { - return containsModuleSyntax(`${source}`, fileURLToPath(url), url) ? 'module' : 'commonjs'; - } - return format; + return (format === 'module') ? detectModuleFormat(source, url) : format; } return 'commonjs'; } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 231f7308e652a6..969b72a508b48b 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -329,7 +329,7 @@ class ModuleLoader { * @returns {ModuleJobBase} */ getModuleJobForRequire(specifier, parentURL, importAttributes) { - assert(getOptionValue('--experimental-require-module')); + assert(getOptionValue('--experimental-require-module') || getOptionValue('--experimental-detect-module')); const parsed = URLParse(specifier); if (parsed != null) { diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 2b08aee16c8e43..3aaf045ce5f07c 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1705,14 +1705,32 @@ static void ContainsModuleSyntax(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 2); - // Argument 1: source code - CHECK(args[0]->IsString()); - Local code = args[0].As(); - // Argument 2: filename CHECK(args[1]->IsString()); Local filename = args[1].As(); + // Argument 1: source code; if undefined, read from filename in argument 2 + Local code; + if (args[0]->IsUndefined()) { + CHECK(!filename.IsEmpty()); + const char* filename_str = Utf8Value(isolate, filename).out(); + std::string contents; + int result = ReadFileSync(&contents, filename_str); + if (result != 0) { + isolate->ThrowException( + ERR_MODULE_NOT_FOUND(isolate, "Cannot read file %s", filename_str)); + return; + } + code = String::NewFromUtf8(isolate, + contents.c_str(), + v8::NewStringType::kNormal, + contents.length()) + .ToLocalChecked(); + } else { + CHECK(args[0]->IsString()); + code = args[0].As(); + } + // Argument 3: resource name (URL for ES module). Local resource_name = filename; if (args[2]->IsString()) { @@ -1729,6 +1747,7 @@ static void ContainsModuleSyntax(const FunctionCallbackInfo& args) { Local fn; TryCatchScope try_catch(env); ShouldNotAbortOnUncaughtScope no_abort_scope(env); + if (CompileFunctionForCJSLoader( env, context, code, filename, &cache_rejected, cjs_var) .ToLocal(&fn)) { @@ -1740,7 +1759,13 @@ static void ContainsModuleSyntax(const FunctionCallbackInfo& args) { } bool result = ShouldRetryAsESM(realm, message, code, resource_name); - args.GetReturnValue().Set(result); + if (result) { + // successfully parsed as ESM after failing to parse as CJS => ESM syntax + args.GetReturnValue().Set(result); + return; + } + + args.GetReturnValue().SetUndefined(); } static void StartSigintWatchdog(const FunctionCallbackInfo& args) { diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs index a58872e661c4f7..84bb1c06416c71 100644 --- a/test/es-module/test-esm-detect-ambiguous.mjs +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -168,7 +168,7 @@ describe('--experimental-detect-module', { concurrency: !process.env.TEST_PARALL }); } - it('should not hint wrong format in resolve hook', async () => { + it('should hint format correctly for the resolve hook for extensionless modules', async () => { let writeSync; const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ '--experimental-detect-module', @@ -185,7 +185,7 @@ describe('--experimental-detect-module', { concurrency: !process.env.TEST_PARALL ]); strictEqual(stderr, ''); - strictEqual(stdout, 'null\nexecuted\n'); + strictEqual(stdout, 'module\nexecuted\n'); strictEqual(code, 0); strictEqual(signal, null); @@ -385,6 +385,65 @@ describe('--experimental-detect-module', { concurrency: !process.env.TEST_PARALL strictEqual(signal, null); }); }); + + describe('should work with module customization hooks', { concurrency: true }, () => { + it('should not break basic hooks functionality of substituting a module', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--import', + fixtures.fileURL('es-module-loaders/builtin-named-exports.mjs'), + fixtures.path('es-modules/require-esm-throws-with-loaders.js'), + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, ''); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should detect the syntax of the source as returned by a custom load hook', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--no-warnings', + '--experimental-detect-module', + '--import', + `data:text/javascript,${encodeURIComponent( + 'import { register } from "node:module";' + + 'import { pathToFileURL } from "node:url";' + + 'register("./transpile-esm-to-cjs.mjs", pathToFileURL("./"));' + )}`, + fixtures.path('es-modules/package-without-type/module.js'), + ], { cwd: fixtures.fileURL('es-module-loaders/') }); + + strictEqual(stderr, ''); + strictEqual(stdout, ` + Resolved format: module + Loaded original format: module + executed + Evaluated format: commonjs + `.replace(/^\s+/gm, '')); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should throw the usual error for a missing file', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--no-warnings', + '--experimental-detect-module', + '--import', + `data:text/javascript,${encodeURIComponent( + 'import { register } from "node:module";' + + 'import { pathToFileURL } from "node:url";' + + 'register("./transpile-esm-to-cjs.mjs", pathToFileURL("./"));' + )}`, + fixtures.path('es-modules/package-without-type/imports-nonexistent.js'), + ], { cwd: fixtures.fileURL('es-module-loaders/') }); + + match(stderr, /ERR_MODULE_NOT_FOUND.+does-not-exist\.js/); + strictEqual(stdout, 'Resolved format: module\nLoaded original format: module\n'); + strictEqual(code, 1); + strictEqual(signal, null); + }); + }); }); // Validate temporarily disabling `--abort-on-uncaught-exception` diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index c7536f93e51567..a049350772aef7 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -744,45 +744,41 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => { assert.strictEqual(signal, null); }); - it('should use ESM loader to respond to require.resolve calls when opting in', async () => { - const readFile = async () => {}; - const fileURLToPath = () => {}; - const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ - '--no-warnings', - '--experimental-loader', - `data:text/javascript,import{readFile}from"node:fs/promises";import{fileURLToPath}from"node:url";export ${ - async function load(u, c, n) { - const r = await n(u, c); - if (u.endsWith('/common/index.js')) { - r.source = '"use strict";module.exports=require("node:module").createRequire(' + - `${JSON.stringify(u)})(${JSON.stringify(fileURLToPath(u))});\n`; - } else if (c.format === 'commonjs') { - r.source = await readFile(new URL(u)); - } - return r; - }}`, - '--experimental-loader', - fixtures.fileURL('es-module-loaders/loader-resolve-passthru.mjs'), - fixtures.path('require-resolve.js'), - ]); - - assert.strictEqual(stderr, ''); - assert.strictEqual(stdout, 'resolve passthru\n'.repeat(10)); - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); - }); - describe('should use hooks', async () => { - const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--import', - fixtures.fileURL('es-module-loaders/builtin-named-exports.mjs'), - fixtures.path('es-modules/require-esm-throws-with-loaders.js'), - ]); + describe('should use ESM loader to respond to require.resolve calls when opting in', () => { + for (const { testConfigName, additionalOptions } of [ + { testConfigName: 'without --experimental-detect-module', additionalOptions: [] }, + { testConfigName: 'with --experimental-detect-module', additionalOptions: ['--experimental-detect-module'] }, + ]) { + it(testConfigName, async () => { + const readFile = async () => {}; + const fileURLToPath = () => {}; + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + ...additionalOptions, + '--no-warnings', + '--experimental-loader', + `data:text/javascript,import{readFile}from"node:fs/promises";import{fileURLToPath}from"node:url";export ${ + async function load(u, c, n) { + const r = await n(u, c); + if (u.endsWith('/common/index.js')) { + r.source = '"use strict";module.exports=require("node:module").createRequire(' + + `${JSON.stringify(u)})(${JSON.stringify(fileURLToPath(u))});\n`; + } else if (c.format === 'commonjs') { + r.source = await readFile(new URL(u)); + } + return r; + }}`, + '--experimental-loader', + fixtures.fileURL('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.path('require-resolve.js'), + ]); - assert.strictEqual(stderr, ''); - assert.strictEqual(stdout, ''); - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, 'resolve passthru\n'.repeat(10)); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); + } }); it('should support source maps in commonjs translator', async () => { diff --git a/test/fixtures/es-module-loaders/transpile-esm-to-cjs.mjs b/test/fixtures/es-module-loaders/transpile-esm-to-cjs.mjs new file mode 100644 index 00000000000000..c08008053a2cbf --- /dev/null +++ b/test/fixtures/es-module-loaders/transpile-esm-to-cjs.mjs @@ -0,0 +1,28 @@ +import { writeSync } from "node:fs"; + +export async function resolve(specifier, context, next) { + const result = await next(specifier, context); + if (specifier.startsWith("file://")) { + writeSync(1, `Resolved format: ${result.format}\n`); + } + return result; +} + +export async function load(url, context, next) { + const output = await next(url, context); + writeSync(1, `Loaded original format: ${output.format}\n`); + + let source = `${output.source}` + + // This is a very incomplete and naively done implementation for testing purposes only + if (source?.includes('export default')) { + source = source.replace('export default', 'module.exports ='); + + source += '\nconsole.log(`Evaluated format: ${this === undefined ? "module" : "commonjs"}`);'; + + output.source = source; + output.format = 'commonjs'; + } + + return output; +} diff --git a/test/fixtures/es-modules/package-without-type/imports-nonexistent.js b/test/fixtures/es-modules/package-without-type/imports-nonexistent.js new file mode 100644 index 00000000000000..1136a7d8c2787d --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-nonexistent.js @@ -0,0 +1 @@ +import "./does-not-exist.js";