From 427e5348a267b7c88879b47f8d942fc3e84b30e9 Mon Sep 17 00:00:00 2001 From: Giorgos Ntemiris Date: Sat, 27 Jul 2019 20:23:25 +0200 Subject: [PATCH] module: add warning when import,export is detected in CJS context This will allow users to know how to change their project to support ES modules. PR-URL: https://github.com/nodejs/node/pull/28950 Reviewed-By: Bradley Farias Reviewed-By: Rich Trott Reviewed-By: Guy Bedford --- lib/internal/modules/cjs/loader.js | 66 ++++++--- .../test-esm-cjs-load-error-note.mjs | 136 ++++++++++++++++++ .../es-note-promiserej-import-2.cjs | 1 + .../es-note-unexpected-export-1.cjs | 2 + .../es-note-unexpected-export-2.cjs | 4 + .../es-note-unexpected-import-1.cjs | 1 + .../es-note-unexpected-import-3.cjs | 1 + .../es-note-unexpected-import-4.cjs | 1 + .../es-note-unexpected-import-5.cjs | 1 + 9 files changed, 195 insertions(+), 18 deletions(-) create mode 100644 test/es-module/test-esm-cjs-load-error-note.mjs create mode 100644 test/fixtures/es-modules/es-note-promiserej-import-2.cjs create mode 100644 test/fixtures/es-modules/es-note-unexpected-export-1.cjs create mode 100644 test/fixtures/es-modules/es-note-unexpected-export-2.cjs create mode 100644 test/fixtures/es-modules/es-note-unexpected-import-1.cjs create mode 100644 test/fixtures/es-modules/es-note-unexpected-import-3.cjs create mode 100644 test/fixtures/es-modules/es-note-unexpected-import-4.cjs create mode 100644 test/fixtures/es-modules/es-note-unexpected-import-5.cjs diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 5d5767e0cffa48..3d440202c28b5b 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -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) { @@ -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'); diff --git a/test/es-module/test-esm-cjs-load-error-note.mjs b/test/es-module/test-esm-cjs-load-error-note.mjs new file mode 100644 index 00000000000000..ce0d1d796969d7 --- /dev/null +++ b/test/es-module/test-esm-cjs-load-error-note.mjs @@ -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}`); +})); diff --git a/test/fixtures/es-modules/es-note-promiserej-import-2.cjs b/test/fixtures/es-modules/es-note-promiserej-import-2.cjs new file mode 100644 index 00000000000000..e9da32086f526c --- /dev/null +++ b/test/fixtures/es-modules/es-note-promiserej-import-2.cjs @@ -0,0 +1 @@ +import('valid'); diff --git a/test/fixtures/es-modules/es-note-unexpected-export-1.cjs b/test/fixtures/es-modules/es-note-unexpected-export-1.cjs new file mode 100644 index 00000000000000..3e2612ed35aff8 --- /dev/null +++ b/test/fixtures/es-modules/es-note-unexpected-export-1.cjs @@ -0,0 +1,2 @@ +const example = 10; +export default example; diff --git a/test/fixtures/es-modules/es-note-unexpected-export-2.cjs b/test/fixtures/es-modules/es-note-unexpected-export-2.cjs new file mode 100644 index 00000000000000..637bac96163752 --- /dev/null +++ b/test/fixtures/es-modules/es-note-unexpected-export-2.cjs @@ -0,0 +1,4 @@ +const config = 10; +export { + config +}; diff --git a/test/fixtures/es-modules/es-note-unexpected-import-1.cjs b/test/fixtures/es-modules/es-note-unexpected-import-1.cjs new file mode 100644 index 00000000000000..232259b06f7fae --- /dev/null +++ b/test/fixtures/es-modules/es-note-unexpected-import-1.cjs @@ -0,0 +1 @@ +import "invalid"; diff --git a/test/fixtures/es-modules/es-note-unexpected-import-3.cjs b/test/fixtures/es-modules/es-note-unexpected-import-3.cjs new file mode 100644 index 00000000000000..5d2d3029e31840 --- /dev/null +++ b/test/fixtures/es-modules/es-note-unexpected-import-3.cjs @@ -0,0 +1 @@ +import x, { y } from 'xyz' diff --git a/test/fixtures/es-modules/es-note-unexpected-import-4.cjs b/test/fixtures/es-modules/es-note-unexpected-import-4.cjs new file mode 100644 index 00000000000000..5e265ba0543746 --- /dev/null +++ b/test/fixtures/es-modules/es-note-unexpected-import-4.cjs @@ -0,0 +1 @@ +import * as coordinates from 'xyz' diff --git a/test/fixtures/es-modules/es-note-unexpected-import-5.cjs b/test/fixtures/es-modules/es-note-unexpected-import-5.cjs new file mode 100644 index 00000000000000..f38c3ee1a5c887 --- /dev/null +++ b/test/fixtures/es-modules/es-note-unexpected-import-5.cjs @@ -0,0 +1 @@ +import ('invalid')