From dc43188c23c75130e61e1752cddfbf80bf1117aa Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 8 Oct 2023 21:02:55 -0700 Subject: [PATCH] File input; improve string input tests --- lib/internal/modules/esm/translators.js | 64 +++++-- lib/internal/modules/run_main.js | 14 +- test/es-module/test-esm-detect-ambiguous.mjs | 170 ++++++++++++++++-- .../package-type-commonjs/imports-esm.js | 1 + .../package-type-commonjs/imports-esm.mjs | 1 + .../package-type-commonjs/module.js | 2 + .../package-type-module/imports-commonjs.cjs | 1 + .../package-type-module/imports-commonjs.mjs | 1 + .../package-without-type/commonjs.js | 2 + .../package-without-type/imports-commonjs.cjs | 1 + .../package-without-type/imports-commonjs.mjs | 1 + .../package-without-type/imports-esm.js | 1 + .../package-without-type/imports-esm.mjs | 1 + .../es-modules/package-without-type/module.js | 1 - 14 files changed, 234 insertions(+), 27 deletions(-) create mode 100644 test/fixtures/es-modules/package-type-commonjs/imports-esm.js create mode 100644 test/fixtures/es-modules/package-type-commonjs/imports-esm.mjs create mode 100644 test/fixtures/es-modules/package-type-commonjs/module.js create mode 100644 test/fixtures/es-modules/package-type-module/imports-commonjs.cjs create mode 100644 test/fixtures/es-modules/package-type-module/imports-commonjs.mjs create mode 100644 test/fixtures/es-modules/package-without-type/commonjs.js create mode 100644 test/fixtures/es-modules/package-without-type/imports-commonjs.cjs create mode 100644 test/fixtures/es-modules/package-without-type/imports-commonjs.mjs create mode 100644 test/fixtures/es-modules/package-without-type/imports-esm.js create mode 100644 test/fixtures/es-modules/package-without-type/imports-esm.mjs diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index cf9afb741aab85..39d3bf1bcbf5f8 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -36,15 +36,21 @@ const { hasEsmSyntax, loadBuiltinModule, stripBOM, + isModuleSyntaxError, } = require('internal/modules/helpers'); const { Module: CJSModule, cjsParseCache, } = require('internal/modules/cjs/loader'); +const { getPackageType } = require('internal/modules/esm/resolve'); const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { debug = fn; }); +const { getOptionValue } = require('internal/options'); +const inputTypeFlagInputValue = getOptionValue('--input-type'); // Will be an empty string if flag is not set. +const defaultTypeFlagInputValue = getOptionValue('--experimental-default-type'); // Will be an empty string if flag is not set. +const detectModule = getOptionValue('--experimental-detect-module'); const { emitExperimentalWarning, kEmptyObject, setOwnProperty } = require('internal/util'); const { ERR_UNKNOWN_BUILTIN_MODULE, @@ -146,7 +152,7 @@ async function importModuleDynamically(specifier, { url }, assertions) { } // Strategy for loading a standard JavaScript module. -translators.set('module', async function moduleStrategy(url, source, isMain) { +async function moduleStrategy(url, source, isMain) { assertBufferSource(source, true, 'load'); source = stringify(source); maybeCacheSourceMap(url, source); @@ -159,16 +165,18 @@ translators.set('module', async function moduleStrategy(url, source, isMain) { importModuleDynamically, }); return module; -}); +}; +translators.set('module', moduleStrategy); /** - * Provide a more informative error for CommonJS imports. - * @param {Error | any} err + * Provide a more informative error for CommonJS erroring because of ESM syntax, + * when we aren't retrying CommonJS modules as ES modules. + * @param {Error | unknown} err * @param {string} [content] Content of the file, if known. * @param {string} [filename] Useful only if `content` is unknown. */ function enrichCJSError(err, content, filename) { - if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype && + if (!detectModule && err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype && hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) { // Emit the warning synchronously because we are in the middle of handling // a SyntaxError that will throw and likely terminate the process before an @@ -264,7 +272,8 @@ const cjsCache = new SafeMap(); function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) { debug(`Translating CJSModule ${url}`); - const filename = StringPrototypeStartsWith(url, 'file://') ? fileURLToPath(url) : url; + const isFileURL = StringPrototypeStartsWith(url, 'file://'); + const filename = isFileURL ? fileURLToPath(url) : url; source = stringify(source); const { exportNames, module } = cjsPreparseModuleExports(filename, source); @@ -276,11 +285,37 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) { setOwnProperty(process, 'mainModule', module); } - return new ModuleWrap(url, undefined, namesWithDefault, function() { - debug(`Loading CJSModule ${url}`); - - if (!module.loaded) { + let moduleLoadingError; + debug(`Loading CJSModule ${url}`); + if (!module.loaded) { + try { loadCJS(module, source, url, filename); + } catch (err) { + /** + * If this module contained ESM syntax, we will retry loading it as an ES module if the following conditions are met: + * - For file input: + * - It is in a package scope with no defined `type`: neither `module` nor `commonjs`. + * - It does not have an explicit .cjs or .mjs extension. + * - For string input: + * - `--input-type` is unset. + * - `--experimental-default-type` is unset. + */ + if (detectModule && isModuleSyntaxError(err) && ( + (isFileURL && getPackageType(url) === 'none' && !filename.endsWith('.mjs') && !filename.endsWith('.cjs')) || + (!isFileURL && inputTypeFlagInputValue === '' && defaultTypeFlagInputValue === '') + )) { + // We need to catch this error here, rather than during the ModuleWrap callback, in order to bubble this up so that we + // can retry loading the module as an ES module. + throw err; + } + moduleLoadingError = err; + } + } + + return new ModuleWrap(url, undefined, namesWithDefault, function() { + // The non-detection flow expects module loading errors to be thrown here. + if (moduleLoadingError) { + throw moduleLoadingError; } let exports; @@ -344,8 +379,15 @@ translators.set('commonjs', async function commonjsStrategy(url, source, } catch { // Continue regardless of error. } - return createCJSModuleWrap(url, source, isMain, cjsLoader); + try { + return createCJSModuleWrap(url, source, isMain, cjsLoader); + } catch (err) { + if (detectModule && isModuleSyntaxError(err)) { + debug(`Retrying evaluating module ${url} as ESM`); + return moduleStrategy(url, source, isMain); + } + } }); /** diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index a9828286a9c0e0..220f5332527d0a 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -119,7 +119,19 @@ function executeUserEntryPoint(main = process.argv[1]) { } else { // Module._load is the monkey-patchable CJS module loader. const { Module } = require('internal/modules/cjs/loader'); - Module._load(main, null, true); + try { + Module._load(main, null, true); + } catch (err) { + if (getOptionValue('--experimental-detect-module')) { + const { isModuleSyntaxError } = require('internal/modules/helpers'); + if (isModuleSyntaxError(err)) { + // If the error is a module syntax error, we should use the ESM loader instead. + runMainESM(resolvedMain || main); + return; + } + } + throw err; + } } } diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs index cf0805c7fe46ba..6b373a3e20f253 100644 --- a/test/es-module/test-esm-detect-ambiguous.mjs +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -1,18 +1,160 @@ import { spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { spawn } from 'node:child_process'; import { describe, it } from 'node:test'; -import { strictEqual } from 'node:assert'; - -describe('--experimental-detect-module', () => { - it('permits ESM syntax in --eval input without requiring --input-type=module', async () => { - const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ - '--experimental-detect-module', - '--eval', - 'import { version } from "node:process"; console.log(version);', - ]); - - strictEqual(stderr, ''); - strictEqual(stdout, `${process.version}\n`); - strictEqual(code, 0); - strictEqual(signal, null); +import { strictEqual, match } from 'node:assert'; + +describe('--experimental-detect-module', { concurrency: true }, () => { + describe('string input', { concurrency: true }, () => { + it('permits ESM syntax in --eval input without requiring --input-type=module', async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--eval', + 'import { version } from "node:process"; console.log(version);', + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, `${process.version}\n`); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + // ESM is unsupported for --print via --input-type=module + + it('permits ESM syntax in STDIN input without requiring --input-type=module', async () => { + const child = spawn(process.execPath, [ + '--experimental-detect-module', + ]); + child.stdin.end('console.log(typeof import.meta.resolve)'); + + match((await child.stdout.toArray()).toString(), /^function\r?\n$/); + }); + + it('should be overridden by --input-type', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--input-type=commonjs', + '--eval', + 'import.meta.url', + ]); + + match(stderr, /SyntaxError: Cannot use 'import\.meta' outside a module/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + + it('should be overridden by --experimental-default-type', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + '--experimental-default-type=commonjs', + '--eval', + 'import.meta.url', + ]); + + match(stderr, /SyntaxError: Cannot use 'import\.meta' outside a module/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + }); + + describe('file input in a typeless package', { concurrency: true }, () => { + for (const { testName, entryPath } of [ + { + testName: 'permits CommonJS syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-without-type/commonjs.js'), + }, + { + testName: 'permits ESM syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-without-type/module.js'), + }, + { + testName: 'permits CommonJS syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-commonjs.cjs'), + }, + { + testName: 'permits ESM syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-esm.js'), + }, + { + testName: 'permits CommonJS syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-commonjs.mjs'), + }, + { + testName: 'permits ESM syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-without-type/imports-esm.mjs'), + }, + ]) { + it(testName, async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + entryPath, + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, 'executed\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + } + }); + + describe('file input in a "type": "commonjs" package', { concurrency: true }, () => { + for (const { testName, entryPath } of [ + { + testName: 'disallows ESM syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-type-commonjs/module.js'), + }, + { + testName: 'disallows ESM syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-type-commonjs/imports-esm.js'), + }, + { + testName: 'disallows ESM syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-type-commonjs/imports-esm.mjs'), + }, + ]) { + it(testName, async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + entryPath, + ]); + + match(stderr, /SyntaxError: Unexpected token 'export'/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + } + }); + + describe('file input in a "type": "module" package', { concurrency: true }, () => { + for (const { testName, entryPath } of [ + { + testName: 'disallows CommonJS syntax in a .js entry point', + entryPath: fixtures.path('es-modules/package-type-module/cjs.js'), + }, + { + testName: 'disallows CommonJS syntax in a .js file imported by a CommonJS entry point', + entryPath: fixtures.path('es-modules/package-type-module/imports-commonjs.cjs'), + }, + { + testName: 'disallows CommonJS syntax in a .js file imported by an ESM entry point', + entryPath: fixtures.path('es-modules/package-type-module/imports-commonjs.mjs'), + }, + ]) { + it(testName, async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + '--experimental-detect-module', + entryPath, + ]); + + match(stderr, /ReferenceError: module is not defined in ES module scope/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + } }); }) diff --git a/test/fixtures/es-modules/package-type-commonjs/imports-esm.js b/test/fixtures/es-modules/package-type-commonjs/imports-esm.js new file mode 100644 index 00000000000000..d2f5d5fee76ef7 --- /dev/null +++ b/test/fixtures/es-modules/package-type-commonjs/imports-esm.js @@ -0,0 +1 @@ +import('./module.js'); diff --git a/test/fixtures/es-modules/package-type-commonjs/imports-esm.mjs b/test/fixtures/es-modules/package-type-commonjs/imports-esm.mjs new file mode 100644 index 00000000000000..d3eb2fba6a8ee7 --- /dev/null +++ b/test/fixtures/es-modules/package-type-commonjs/imports-esm.mjs @@ -0,0 +1 @@ +import './module.js'; diff --git a/test/fixtures/es-modules/package-type-commonjs/module.js b/test/fixtures/es-modules/package-type-commonjs/module.js new file mode 100644 index 00000000000000..251d6e538a1fcf --- /dev/null +++ b/test/fixtures/es-modules/package-type-commonjs/module.js @@ -0,0 +1,2 @@ +export default 'module'; +console.log('executed'); diff --git a/test/fixtures/es-modules/package-type-module/imports-commonjs.cjs b/test/fixtures/es-modules/package-type-module/imports-commonjs.cjs new file mode 100644 index 00000000000000..7dbbf0d97a46ba --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/imports-commonjs.cjs @@ -0,0 +1 @@ +import('./cjs.js'); diff --git a/test/fixtures/es-modules/package-type-module/imports-commonjs.mjs b/test/fixtures/es-modules/package-type-module/imports-commonjs.mjs new file mode 100644 index 00000000000000..df53dcd2bd4885 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/imports-commonjs.mjs @@ -0,0 +1 @@ +import './cjs.js'; diff --git a/test/fixtures/es-modules/package-without-type/commonjs.js b/test/fixtures/es-modules/package-without-type/commonjs.js new file mode 100644 index 00000000000000..9b4d39fa21ada2 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/commonjs.js @@ -0,0 +1,2 @@ +module.exports = 'cjs'; +console.log('executed'); diff --git a/test/fixtures/es-modules/package-without-type/imports-commonjs.cjs b/test/fixtures/es-modules/package-without-type/imports-commonjs.cjs new file mode 100644 index 00000000000000..b247f42a15ceb8 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-commonjs.cjs @@ -0,0 +1 @@ +import('./commonjs.js'); diff --git a/test/fixtures/es-modules/package-without-type/imports-commonjs.mjs b/test/fixtures/es-modules/package-without-type/imports-commonjs.mjs new file mode 100644 index 00000000000000..c2f8171fd1deda --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-commonjs.mjs @@ -0,0 +1 @@ +import './commonjs.js'; diff --git a/test/fixtures/es-modules/package-without-type/imports-esm.js b/test/fixtures/es-modules/package-without-type/imports-esm.js new file mode 100644 index 00000000000000..d2f5d5fee76ef7 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-esm.js @@ -0,0 +1 @@ +import('./module.js'); diff --git a/test/fixtures/es-modules/package-without-type/imports-esm.mjs b/test/fixtures/es-modules/package-without-type/imports-esm.mjs new file mode 100644 index 00000000000000..d3eb2fba6a8ee7 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/imports-esm.mjs @@ -0,0 +1 @@ +import './module.js'; diff --git a/test/fixtures/es-modules/package-without-type/module.js b/test/fixtures/es-modules/package-without-type/module.js index 69147a3b8ca027..251d6e538a1fcf 100644 --- a/test/fixtures/es-modules/package-without-type/module.js +++ b/test/fixtures/es-modules/package-without-type/module.js @@ -1,3 +1,2 @@ -// This file can be run or imported only if `--experimental-default-type=module` is set. export default 'module'; console.log('executed');