From 0c15920da70373e7f35df682237b325564fc94a1 Mon Sep 17 00:00:00 2001
From: Geoffrey Booth <webmaster@geoffreybooth.com>
Date: Wed, 27 Feb 2019 11:15:41 -0800
Subject: [PATCH] esm: fix errors for --type / "type" mismatches

---
 doc/api/esm.md                              |  4 +---
 lib/internal/errors.js                      | 24 ++++++++++-----------
 lib/internal/modules/esm/default_resolve.js | 12 +++++------
 test/es-module/test-esm-type-flag-errors.js |  9 ++++----
 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/doc/api/esm.md b/doc/api/esm.md
index 7c5081d716..37f3d12df1 100644
--- a/doc/api/esm.md
+++ b/doc/api/esm.md
@@ -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
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index e91887a43f..fa7b233978 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -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 ' +
diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js
index f7414a5985..5471ee629a 100644
--- a/lib/internal/modules/esm/default_resolve.js
+++ b/lib/internal/modules/esm/default_resolve.js
@@ -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) {
@@ -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');
     }
   }
 
diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js
index de1fa1a276..84d67a8681 100644
--- a/test/es-module/test-esm-type-flag-errors.js
+++ b/test/es-module/test-esm-type-flag-errors.js
@@ -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);
@@ -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