Skip to content

Commit

Permalink
module: add warning when import,export is detected in CJS context
Browse files Browse the repository at this point in the history
This will allow users to know how to change their project to support
ES modules.

PR-URL: #28950
Reviewed-By: Bradley Farias <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
  • Loading branch information
gntem authored and Trott committed Aug 13, 2019
1 parent a49b20d commit 427e534
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 18 deletions.
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)) {
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')

0 comments on commit 427e534

Please sign in to comment.