diff options
author | Guy Bedford <guybedford@gmail.com> | 2020-09-30 14:24:34 +0300 |
---|---|---|
committer | Guy Bedford <guybedford@gmail.com> | 2020-10-01 06:46:01 +0300 |
commit | 726143e683738b0e60580bdc247e6cfa60a0783a (patch) | |
tree | df52325923036ed0f27fb57655c953af1c687d21 | |
parent | 4acab22ac2dd4d066c7d50eb1916e34af555eccf (diff) |
module: refine module type mismatch error cases
PR-URL: https://github.com/nodejs/node/pull/35426
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
-rw-r--r-- | lib/internal/modules/cjs/loader.js | 24 | ||||
-rw-r--r-- | lib/internal/modules/esm/translators.js | 39 | ||||
-rw-r--r-- | test/es-module/test-esm-cjs-exports.js | 16 | ||||
-rw-r--r-- | test/fixtures/es-modules/cjs-exports-invalid.mjs | 1 | ||||
-rw-r--r-- | test/fixtures/es-modules/invalid-cjs.js | 1 |
5 files changed, 55 insertions, 26 deletions
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 6f47bbc7144..9b5752e1d26 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -72,7 +72,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'); @@ -115,6 +114,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, @@ -129,28 +129,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) { diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index efbb5d92d7e..94eb275a689 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -9,9 +9,12 @@ const { ObjectKeys, PromisePrototypeCatch, PromiseReject, + RegExpPrototypeTest, SafeMap, SafeSet, StringPrototypeReplace, + StringPrototypeSplit, + StringPrototypeStartsWith, } = primordials; let _TYPES = null; @@ -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) { @@ -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; @@ -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) { @@ -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); diff --git a/test/es-module/test-esm-cjs-exports.js b/test/es-module/test-esm-cjs-exports.js index 37aa70d3880..7db2c6fdb59 100644 --- a/test/es-module/test-esm-cjs-exports.js +++ b/test/es-module/test-esm-cjs-exports.js @@ -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'); @@ -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\'')); +})); diff --git a/test/fixtures/es-modules/cjs-exports-invalid.mjs b/test/fixtures/es-modules/cjs-exports-invalid.mjs new file mode 100644 index 00000000000..67428fc27c2 --- /dev/null +++ b/test/fixtures/es-modules/cjs-exports-invalid.mjs @@ -0,0 +1 @@ +import cjs from './invalid-cjs.js'; diff --git a/test/fixtures/es-modules/invalid-cjs.js b/test/fixtures/es-modules/invalid-cjs.js new file mode 100644 index 00000000000..15cae6690f1 --- /dev/null +++ b/test/fixtures/es-modules/invalid-cjs.js @@ -0,0 +1 @@ +export var name = 5; |