Skip to content

Commit

Permalink
File input; improve string input tests
Browse files Browse the repository at this point in the history
  • Loading branch information
GeoffreyBooth committed Oct 9, 2023
1 parent 0fa17a6 commit 3a37bc3
Show file tree
Hide file tree
Showing 15 changed files with 236 additions and 29 deletions.
64 changes: 53 additions & 11 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'); // Empty string if the flag is unset.
const defaultTypeFlagInputValue = getOptionValue('--experimental-default-type'); // Empty string if the flag is unset.
const detectModule = getOptionValue('--experimental-detect-module');
const { emitExperimentalWarning, kEmptyObject, setOwnProperty } = require('internal/util');
const {
ERR_UNKNOWN_BUILTIN_MODULE,
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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 under the following conditions:
* - 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;
Expand Down Expand Up @@ -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);
}
}
});

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ function isModuleSyntaxError(err) {
err.message === 'Cannot use import statement outside a module' ||
err.message === "Unexpected token 'export'" ||
err.message === "Cannot use 'import.meta' outside a module"
);
);
}

module.exports = {
Expand Down
14 changes: 13 additions & 1 deletion lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand Down
172 changes: 157 additions & 15 deletions test/es-module/test-esm-detect-ambiguous.mjs
Original file line number Diff line number Diff line change
@@ -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);
});
}
});
})
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import('./module.js');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './module.js';
2 changes: 2 additions & 0 deletions test/fixtures/es-modules/package-type-commonjs/module.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export default 'module';
console.log('executed');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import('./cjs.js');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './cjs.js';
2 changes: 2 additions & 0 deletions test/fixtures/es-modules/package-without-type/commonjs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module.exports = 'cjs';
console.log('executed');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import('./commonjs.js');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './commonjs.js';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import('./module.js');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './module.js';
1 change: 0 additions & 1 deletion test/fixtures/es-modules/package-without-type/module.js
Original file line number Diff line number Diff line change
@@ -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');

0 comments on commit 3a37bc3

Please sign in to comment.