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: add note when cjs loader throws error #28950

Closed
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
66 changes: 48 additions & 18 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,29 @@ const relativeResolveCache = Object.create(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') ||
(/^\s*import(?=[ {'"*])\s*(?![ (])/).test(lineWithErr)) {
gntem marked this conversation as resolved.
Show resolved Hide resolved
process.emitWarning(
'To load an ES module, set "type": "module" in the package.json or use ' +
'the .mjs extension.',
undefined,
undefined,
undefined,
true);
}
}

function stat(filename) {
filename = path.toNamespacedPath(filename);
if (statCache !== null) {
Expand Down Expand Up @@ -828,24 +851,31 @@ function wrapSafe(filename, content) {
} : undefined,
});
}

const compiled = compileFunction(
content,
filename,
0,
0,
undefined,
false,
undefined,
[],
[
'exports',
'require',
'module',
'__filename',
'__dirname',
]
);
let compiled;
try {
compiled = compileFunction(
content,
filename,
0,
0,
undefined,
false,
undefined,
[],
[
'exports',
'require',
'module',
'__filename',
'__dirname',
]
);
} catch (err) {
if (experimentalModules) {
enrichCJSError(err);
}
throw err;
}

if (experimentalModules) {
const { callbackMap } = internalBinding('module_wrap');
Expand Down
136 changes: 136 additions & 0 deletions test/es-module/test-esm-cjs-load-error-note.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// Flags: --experimental-modules

import { mustCall } from '../common/index.mjs';
import assert from 'assert';
import fixtures from '../common/fixtures.js';
import { spawn } from 'child_process';

const Export1 = fixtures.path('/es-modules/es-note-unexpected-export-1.cjs');
const Export2 = fixtures.path('/es-modules/es-note-unexpected-export-2.cjs');
const Import1 = fixtures.path('/es-modules/es-note-unexpected-import-1.cjs');
const Import2 = fixtures.path('/es-modules/es-note-promiserej-import-2.cjs');
const Import3 = fixtures.path('/es-modules/es-note-unexpected-import-3.cjs');
const Import4 = fixtures.path('/es-modules/es-note-unexpected-import-4.cjs');
const Import5 = fixtures.path('/es-modules/es-note-unexpected-import-5.cjs');

// Expect note to be included in the error output
const expectedNote = 'To load an ES module, ' +
'set "type": "module" in the package.json ' +
'or use the .mjs extension.';

const expectedCode = 1;

const pExport1 = spawn(process.execPath, ['--experimental-modules', Export1]);
let pExport1Stderr = '';
pExport1.stderr.setEncoding('utf8');
pExport1.stderr.on('data', (data) => {
pExport1Stderr += data;
});
pExport1.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pExport1Stderr.includes(expectedNote),
`${expectedNote} not found in ${pExport1Stderr}`);
}));


const pExport2 = spawn(process.execPath, ['--experimental-modules', Export2]);
let pExport2Stderr = '';
pExport2.stderr.setEncoding('utf8');
pExport2.stderr.on('data', (data) => {
pExport2Stderr += data;
});
pExport2.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pExport2Stderr.includes(expectedNote),
`${expectedNote} not found in ${pExport2Stderr}`);
}));
// The flag --experimental-modules is not used here
// the note must not be included in the output
const pExport3 = spawn(process.execPath, [Export1]);
let pExport3Stderr = '';
pExport3.stderr.setEncoding('utf8');
pExport3.stderr.on('data', (data) => {
pExport3Stderr += data;
});
pExport3.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(!pExport3Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pExport3Stderr}`);
}));

const pImport1 = spawn(process.execPath, ['--experimental-modules', Import1]);
let pImport1Stderr = '';
pImport1.stderr.setEncoding('utf8');
pImport1.stderr.on('data', (data) => {
pImport1Stderr += data;
});
pImport1.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pImport1Stderr.includes(expectedNote),
`${expectedNote} not found in ${pExport1Stderr}`);
}));

// Note this test shouldn't include the note
const pImport2 = spawn(process.execPath, ['--experimental-modules', Import2]);
let pImport2Stderr = '';
pImport2.stderr.setEncoding('utf8');
pImport2.stderr.on('data', (data) => {
pImport2Stderr += data;
});
pImport2.on('close', mustCall((code) => {
// As this exits normally we expect 0
assert.strictEqual(code, 0);
assert.ok(!pImport2Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport2Stderr}`);
}));

const pImport3 = spawn(process.execPath, ['--experimental-modules', Import3]);
let pImport3Stderr = '';
pImport3.stderr.setEncoding('utf8');
pImport3.stderr.on('data', (data) => {
pImport3Stderr += data;
});
pImport3.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pImport3Stderr.includes(expectedNote),
`${expectedNote} not found in ${pImport3Stderr}`);
}));


const pImport4 = spawn(process.execPath, ['--experimental-modules', Import4]);
let pImport4Stderr = '';
pImport4.stderr.setEncoding('utf8');
pImport4.stderr.on('data', (data) => {
pImport4Stderr += data;
});
pImport4.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pImport4Stderr.includes(expectedNote),
`${expectedNote} not found in ${pImport4Stderr}`);
}));

// Must exit with zero and show note
const pImport5 = spawn(process.execPath, ['--experimental-modules', Import5]);
let pImport5Stderr = '';
pImport5.stderr.setEncoding('utf8');
pImport5.stderr.on('data', (data) => {
pImport5Stderr += data;
});
pImport5.on('close', mustCall((code) => {
assert.strictEqual(code, 0);
assert.ok(!pImport5Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport5Stderr}`);
}));

// Must exit with zero and not show note
const pImport6 = spawn(process.execPath, [Import1]);
let pImport6Stderr = '';
pImport6.stderr.setEncoding('utf8');
pImport6.stderr.on('data', (data) => {
pImport6Stderr += data;
});
pImport6.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(!pImport6Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport6Stderr}`);
}));
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-promiserej-import-2.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import('valid');
2 changes: 2 additions & 0 deletions test/fixtures/es-modules/es-note-unexpected-export-1.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const example = 10;
export default example;
4 changes: 4 additions & 0 deletions test/fixtures/es-modules/es-note-unexpected-export-2.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const config = 10;
export {
config
};
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-unexpected-import-1.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "invalid";
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-unexpected-import-3.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import x, { y } from 'xyz'
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-unexpected-import-4.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import * as coordinates from 'xyz'
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-unexpected-import-5.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import ('invalid')