Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: refine module type mismatch error cases #35426

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 1 addition & 23 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ const fs = require('fs');
const internalFS = require('internal/fs/utils');
const path = require('path');
const { sep } = path;
const { emitWarningSync } = require('internal/process/warning');
const { internalModuleStat } = internalBinding('fs');
const packageJsonReader = require('internal/modules/package_json_reader');
const { safeGetenv } = internalBinding('credentials');
Expand Down Expand Up @@ -114,6 +113,7 @@ const {
} = require('internal/util/types');

const asyncESM = require('internal/process/esm_loader');
const { enrichCJSError } = require('internal/modules/esm/translators');
const { kEvaluated } = internalBinding('module_wrap');
const {
encodedSepRegEx,
Expand All @@ -128,28 +128,6 @@ const relativeResolveCache = ObjectCreate(null);
let requireDepth = 0;
let statCache = null;

function enrichCJSError(err) {
const stack = err.stack.split('\n');

const lineWithErr = stack[1];

/*
The regular expression below targets the most common import statement
usage. However, some cases are not matching, cases like import statement
after a comment block and/or after a variable definition.
*/
if (err.message.startsWith('Unexpected token \'export\'') ||
(RegExpPrototypeTest(/^\s*import(?=[ {'"*])\s*(?![ (])/, lineWithErr))) {
// Emit the warning synchronously because we are in the middle of handling
// a SyntaxError that will throw and likely terminate the process before an
// asynchronous warning would be emitted.
emitWarningSync(
'To load an ES module, set "type": "module" in the package.json or use ' +
'the .mjs extension.'
);
}
}

function stat(filename) {
filename = path.toNamespacedPath(filename);
if (statCache !== null) {
Expand Down
39 changes: 37 additions & 2 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ const {
ObjectKeys,
PromisePrototypeCatch,
PromiseReject,
RegExpPrototypeTest,
SafeMap,
SafeSet,
StringPrototypeReplace,
StringPrototypeSplit,
StringPrototypeStartsWith,
} = primordials;

let _TYPES = null;
Expand Down Expand Up @@ -54,9 +57,11 @@ const experimentalImportMetaResolve =
getOptionValue('--experimental-import-meta-resolve');
const asyncESM = require('internal/process/esm_loader');
const cjsParse = require('internal/deps/cjs-module-lexer/lexer');
const { emitWarningSync } = require('internal/process/warning');

const translators = new SafeMap();
exports.translators = translators;
exports.enrichCJSError = enrichCJSError;

let DECODER = null;
function assertBufferSource(body, allowString, hookName) {
Expand Down Expand Up @@ -130,6 +135,25 @@ translators.set('module', async function moduleStrategy(url) {
return module;
});

function enrichCJSError(err) {
const stack = StringPrototypeSplit(err.stack, '\n');
/*
* The regular expression below targets the most common import statement
* usage. However, some cases are not matching, cases like import statement
* after a comment block and/or after a variable definition.
*/
if (StringPrototypeStartsWith(err.message, 'Unexpected token \'export\'') ||
RegExpPrototypeTest(/^\s*import(?=[ {'"*])\s*(?![ (])/, stack[1])) {
// Emit the warning synchronously because we are in the middle of handling
// a SyntaxError that will throw and likely terminate the process before an
// asynchronous warning would be emitted.
emitWarningSync(
'To load an ES module, set "type": "module" in the package.json or use ' +
'the .mjs extension.'
);
}
}

// Strategy for loading a node-style CommonJS module
const isWindows = process.platform === 'win32';
const winSepRegEx = /\//g;
Expand All @@ -152,7 +176,12 @@ translators.set('commonjs', async function commonjsStrategy(url, isMain) {
exports = asyncESM.ESMLoader.cjsCache.get(module);
asyncESM.ESMLoader.cjsCache.delete(module);
} else {
exports = CJSModule._load(filename, undefined, isMain);
try {
exports = CJSModule._load(filename, undefined, isMain);
} catch (err) {
enrichCJSError(err);
throw err;
}
}

for (const exportName of exportNames) {
Expand Down Expand Up @@ -190,7 +219,13 @@ function cjsPreparseModuleExports(filename) {
source = readFileSync(filename, 'utf8');
} catch {}

const { exports, reexports } = cjsParse(source || '');
let exports, reexports;
try {
({ exports, reexports } = cjsParse(source || ''));
} catch {
exports = [];
reexports = [];
}

const exportNames = new SafeSet(exports);

Expand Down
16 changes: 15 additions & 1 deletion test/es-module/test-esm-cjs-exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const assert = require('assert');

const entry = fixtures.path('/es-modules/cjs-exports.mjs');

const child = spawn(process.execPath, [entry]);
let child = spawn(process.execPath, [entry]);
child.stderr.setEncoding('utf8');
let stdout = '';
child.stdout.setEncoding('utf8');
Expand All @@ -19,3 +19,17 @@ child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(signal, null);
assert.strictEqual(stdout, 'ok\n');
}));

const entryInvalid = fixtures.path('/es-modules/cjs-exports-invalid.mjs');
child = spawn(process.execPath, [entryInvalid]);
let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
assert.ok(stderr.includes('Warning: To load an ES module'));
assert.ok(stderr.includes('Unexpected token \'export\''));
}));
1 change: 1 addition & 0 deletions test/fixtures/es-modules/cjs-exports-invalid.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import cjs from './invalid-cjs.js';
1 change: 1 addition & 0 deletions test/fixtures/es-modules/invalid-cjs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export var name = 5;