Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

Commit

Permalink
esm: fix errors for --type / "type" mismatches
Browse files Browse the repository at this point in the history
  • Loading branch information
GeoffreyBooth authored and nodejs-ci committed Mar 3, 2019
1 parent 9ef172b commit 325886d
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 26 deletions.
4 changes: 1 addition & 3 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,7 @@ PACKAGE_MAIN_RESOLVE(_packageURL_, _pjson_)
> 1. Return _"commonjs"_.
> 1. If _pjson.type_ exists and is _"module"_, then
> 1. If _url_ ends in _".cjs"_, then
> 1. Throw a _Type Mismatch_ error.
> 1. If _url_ does not end in _".js"_ or _".mjs"_, then
> 1. Throw an _Unsupported File Extension_ error.
> 1. Return _"commonjs"_.
> 1. Return _"module"_.
> 1. Otherwise,
> 1. If _url_ ends in _".mjs"_, then
Expand Down
24 changes: 11 additions & 13 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -956,21 +956,19 @@ E('ERR_TRANSFORM_ALREADY_TRANSFORMING',
E('ERR_TRANSFORM_WITH_LENGTH_0',
'Calling transform done when writableState.length != 0', Error);
E('ERR_TTY_INIT_FAILED', 'TTY initialization failed', SystemError);
E('ERR_TYPE_MISMATCH', (filename, moduleType, scopeConflict, extConflict) => {
const typeValue = moduleType ? 'module' : 'commonjs';
E('ERR_TYPE_MISMATCH', (filename, ext, typeFlag, conflict) => {
const typeString =
typeFlag === 'module' ? '--type=module or -m' : '--type=commonjs';
// --type mismatches file extension
if (!scopeConflict && extConflict)
return `--type=${typeValue + (typeValue === 'module' ? ' / -m' : '')}` +
` flag conflicts with the file extension loading ${filename}`;
if (conflict === 'extension')
return `Extension ${ext} is not supported for ` +
`${typeString} loading ${filename}`;
// --type mismatches package.json "type"
else if (scopeConflict && !extConflict)
return `--type=${typeValue + (typeValue === 'module' ? ' / ` -m' : '')}` +
' flag conflicts with the package.json "type": "' +
(moduleType ? 'commonjs' : 'module') + `" loading ${filename}`;
// package.json "type" mismatches file extension
else if (extConflict && scopeConflict)
return `"type": "${typeValue}" in package.json conflicts with the file ` +
`extension loading ${filename}`;
else if (conflict === 'scope')
return `Cannot use ${typeString} because nearest parent package.json ` +
((typeFlag === 'module') ?
'includes "type": "commonjs"' : 'includes "type": "module",') +
` which controls the type to use for ${filename}`;
}, TypeError);
E('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET',
'`process.setupUncaughtExceptionCapture()` was called while a capture ' +
Expand Down
12 changes: 6 additions & 6 deletions lib/internal/modules/esm/default_resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ function getModuleFormat(url, isMain, parentURL) {

const ext = extname(url.pathname);

if (!legacy && ext === '.cjs')
throw new ERR_TYPE_MISMATCH(true, true, fileURLToPath(url));

let format = (legacy ? legacyExtensionFormatMap : extensionFormatMap)[ext];

if (!format) {
Expand All @@ -89,14 +86,17 @@ function getModuleFormat(url, isMain, parentURL) {
}

// Check for mismatch between --type and file extension,
// and between --type and package.json.
// and between --type and the "type" field in package.json.
if (isMain && format !== 'module' && asyncESM.typeFlag === 'module') {
// Conflict between package scope type and --type
if (ext === '.js') {
throw new ERR_TYPE_MISMATCH(fileURLToPath(url), true, true, false);
if (pcfg && pcfg.type)
throw new ERR_TYPE_MISMATCH(
fileURLToPath(url), ext, asyncESM.typeFlag, 'scope');
// Conflict between explicit extension (.mjs, .cjs) and --type
} else {
throw new ERR_TYPE_MISMATCH(fileURLToPath(url), true, false, true);
throw new ERR_TYPE_MISMATCH(
fileURLToPath(url), ext, asyncESM.typeFlag, 'extension');
}
}

Expand Down
9 changes: 5 additions & 4 deletions test/es-module/test-esm-type-flag-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ expect('', packageTypeModuleMain, 'package-type-module');
expect('', packageTypeCommonJsMain, 'package-type-commonjs');
expect('', packageWithoutTypeMain, 'package-without-type');

// Check that running with --type and no package.json "type" works
expect('--type=commonjs', packageWithoutTypeMain, 'package-without-type');
expect('--type=module', packageWithoutTypeMain, 'package-without-type');
expect('-m', packageWithoutTypeMain, 'package-without-type');

// Check that running with conflicting --type flags throws errors
expect('--type=commonjs', mjsFile, 'ERR_REQUIRE_ESM', true);
expect('--type=module', cjsFile, 'ERR_TYPE_MISMATCH', true);
Expand All @@ -29,10 +34,6 @@ expect('--type=module', packageTypeCommonJsMain,
'ERR_TYPE_MISMATCH', true);
expect('-m', packageTypeCommonJsMain,
'ERR_TYPE_MISMATCH', true);
expect('--type=module', packageWithoutTypeMain,
'ERR_TYPE_MISMATCH', true);
expect('-m', packageWithoutTypeMain,
'ERR_TYPE_MISMATCH', true);

function expect(opt = '', inputFile, want, wantsError = false) {
// TODO: Remove when --experimental-modules is unflagged
Expand Down

0 comments on commit 325886d

Please sign in to comment.