diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index cd5c88dce8e021..d5b6058b7725b9 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/src/node_contextify.cc b/src/node_contextify.cc index 2b08aee16c8e43..75c94d6d5bad16 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1705,14 +1705,33 @@ 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()); + Utf8Value utf8Value(isolate, filename); + const char* filename_str = utf8Value.out(); + std::string contents; + int result = ReadFileSync(&contents, filename_str); + if (result != 0) { + // error reading file and no source available => undefined + args.GetReturnValue().SetUndefined(); + 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 +1748,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 +1760,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 80dbd885e25819..c7536f93e51567 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -772,6 +772,19 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => { 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'), + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); + it('should support source maps in commonjs translator', async () => { const readFile = async () => {}; const hook = ` diff --git a/test/es-module/test-esm-named-exports.js b/test/es-module/test-esm-named-exports.js deleted file mode 100644 index 2c6f67288aa57c..00000000000000 --- a/test/es-module/test-esm-named-exports.js +++ /dev/null @@ -1,11 +0,0 @@ -// Flags: --import ./test/fixtures/es-module-loaders/builtin-named-exports.mjs -'use strict'; - -require('../common'); -const { readFile, __fromLoader } = require('fs'); -const assert = require('assert'); - -assert.throws(() => require('../fixtures/es-modules/test-esm-ok.mjs'), { code: 'ERR_REQUIRE_ESM' }); - -assert(readFile); -assert(__fromLoader); 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"; diff --git a/test/fixtures/es-modules/require-esm-throws-with-loaders.js b/test/fixtures/es-modules/require-esm-throws-with-loaders.js new file mode 100644 index 00000000000000..79c5cf96c14141 --- /dev/null +++ b/test/fixtures/es-modules/require-esm-throws-with-loaders.js @@ -0,0 +1,8 @@ +'use strict'; +const { readFile, __fromLoader } = require('fs'); +const assert = require('assert'); + +assert.throws(() => require('./test-esm-ok.mjs'), { code: 'ERR_REQUIRE_ESM' }); + +assert(readFile); +assert(__fromLoader);