esm: make extension-less errors in type:module actionable

nodejs · nodejs-github-bot · Mar 15, 2022 · Mar 11, 2022 · Mar 11, 2022 · Mar 11, 2022
commit afde82a5f267e92ea20a9740b305dd9724e51c73
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1594,9 +1594,13 @@ E('ERR_UNHANDLED_ERROR',
 E('ERR_UNKNOWN_BUILTIN_MODULE', 'No such built-in module: %s', Error);
 E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error);
 E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
-E('ERR_UNKNOWN_FILE_EXTENSION',
-  'Unknown file extension "%s" for %s',
-  TypeError);
+E('ERR_UNKNOWN_FILE_EXTENSION', (ext, path, suggestion) => {
+  let msg = `Unknown file extension "${ext}" for ${path}`;
+  if (suggestion) {
+    msg += `. ${suggestion}`;
+  }
+  return msg;
+}, TypeError);
 E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s for URL %s',
   RangeError);
 E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);

diff --git a/lib/internal/modules/esm/formats.js b/lib/internal/modules/esm/formats.js
@@ -30,7 +30,7 @@ if (experimentalWasmModules) {
 }
 
 /**
- * @param {string} mime
+ * @param {string} mime 
  * @returns {string | null}
  */
 function mimeToFormat(mime) {

diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
@@ -7,7 +7,7 @@ const {
   PromisePrototypeThen,
   PromiseResolve,
 } = primordials;
-const { extname } = require('path');
+const { extname, basename } = require('path');
 const { getOptionValue } = require('internal/options');
 const { fetchModule } = require('internal/modules/esm/fetch_module');
 const {
@@ -20,7 +20,7 @@ const experimentalNetworkImports =
   getOptionValue('--experimental-network-imports');
 const experimentalSpecifierResolution =
   getOptionValue('--experimental-specifier-resolution');
-const { getPackageType } = require('internal/modules/esm/resolve');
+const { getPackageType, getPackageScopeConfig } = require('internal/modules/esm/resolve');
 const { URL, fileURLToPath } = require('internal/url');
 const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
 
@@ -33,7 +33,8 @@ const protocolHandlers = ObjectAssign(ObjectCreate(null), {
 });
 
 /**
- * @param {URL} parsed
+ * @param {URL} parsed 
+ * @param {{parentURL: string}} context
  * @returns {string | null}
  */
 function getDataProtocolModuleFormat(parsed) {
@@ -46,13 +47,14 @@ function getDataProtocolModuleFormat(parsed) {
 }
 
 /**
- * @param {URL} url
- * @param {{parentURL: string}} context
- * @param {boolean} ignoreErrors
+ * @param {URL} url 
+ * @param {{parentURL: string}} context 
+ * @param {boolean} ignoreErrors 
  * @returns {string}
  */
 function getFileProtocolModuleFormat(url, context, ignoreErrors) {
-  const ext = extname(url.pathname);
+  const filepath = fileURLToPath(url);
+  const ext = extname(filepath);
   if (ext === '.js') {
     return getPackageType(url) === 'module' ? 'module' : 'commonjs';
   }
@@ -63,14 +65,25 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
   if (experimentalSpecifierResolution !== 'node') {
     // Explicit undefined return indicates load hook should rerun format check
     if (ignoreErrors) return undefined;
-    throw new ERR_UNKNOWN_FILE_EXTENSION(ext, fileURLToPath(url));
+    let suggestion = '';
+    if (getPackageType(url) === 'module' && ext === '') {
+      const config = getPackageScopeConfig(url);
+      const fileBasename = basename(filepath);
+      suggestion = 'Extension-less files are disabled inside of ' +
+        '"type":"module" package.json contexts. The package.json file ' +
+        `${config.pjsonPath} caused this "type":"module" context. Try ` +
+        `changing ${filepath} to have a file extension. Note the "bin" ` +
+        'field  of package.json can point to a file with an extension E.G. ' +
+        `{"type":"module","bin":{"${fileBasename}":"${fileBasename}.js"}}`
+    }
+    throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
   }
 
   return getLegacyExtensionFormat(ext) ?? null;
 }
 
 /**
- * @param {URL} url
+ * @param {URL} url 
  * @param {{parentURL: string}} context
  * @returns {Promise<string> | undefined} only works when enabled
  */
@@ -86,7 +99,7 @@ function getHttpProtocolModuleFormat(url, context) {
 }
 
 /**
- * @param {URL | URL['href']} url
+ * @param {URL | URL['href']} url 
  * @param {{parentURL: string}} context
  * @returns {Promise<string> | string | undefined} only works when enabled
  */
@@ -98,7 +111,7 @@ function defaultGetFormatWithoutErrors(url, context) {
 }
 
 /**
- * @param {URL | URL['href']} url
+ * @param {URL | URL['href']} url 
  * @param {{parentURL: string}} context
  * @returns {Promise<string> | string | undefined} only works when enabled
  */

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -222,6 +222,10 @@ function getPackageConfig(path, specifier, base) {
   return packageConfig;
 }
 
+function getPackageScopePath(resolved) {
+
+}
+
 /**
  * @param {URL | string} resolved
  * @returns {PackageConfig}

diff --git a/test/es-module/test-esm-unknown-or-no-extension.js b/test/es-module/test-esm-unknown-or-no-extension.js
@@ -31,6 +31,10 @@ const assert = require('assert');
     assert.strictEqual(code, 1);
     assert.strictEqual(signal, null);
     assert.strictEqual(stdout, '');
-    assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1);
+    assert.ok(stderr.includes('ERR_UNKNOWN_FILE_EXTENSION'));
+    if (fixturePath.includes('noext')) {
+      // check for explaination to users
+      assert.ok(stderr.includes('Extension-less'));
+    }
   }));
 });