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

bmeck · bengl · commit e55283b978d4 · 2022-03-20T23:04:30.000-04:00
PR-URL: #42301 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -1084,7 +1084,7 @@ async function getPackageType(url) {
   // required by the spec
   // this simple truthy check for whether `url` contains a file extension will
   // work for most projects but does not cover some edge-cases (such as
-  // extension-less files or a url ending in a trailing space)
+  // extensionless files or a url ending in a trailing space)
   const isFilePath = !!extname(url);
   // If it is a file path, get the directory it's in
   const dir = isFilePath ?
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1596,9 +1596,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/get_format.js b/lib/internal/modules/esm/get_format.js
@@ -6,8 +6,9 @@ const {
   ObjectPrototypeHasOwnProperty,
   PromisePrototypeThen,
   PromiseResolve,
+  StringPrototypeSlice,
 } = primordials;
-const { extname } = require('path');
+const { basename, extname, relative } = require('path');
 const { getOptionValue } = require('internal/options');
 const { fetchModule } = require('internal/modules/esm/fetch_module');
 const {
@@ -20,7 +21,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;
 
@@ -52,7 +53,8 @@ function getDataProtocolModuleFormat(parsed) {
  * @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,7 +65,19 @@ 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);
+      const relativePath = StringPrototypeSlice(relative(config.pjsonPath, filepath), 1);
+      suggestion = 'Loading extensionless files is not supported 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, for example ' +
+        `{"type":"module","bin":{"${fileBasename}":"${relativePath}.js"}}`;
+    }
+    throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
   }
 
   return getLegacyExtensionFormat(ext) ?? null;
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -80,10 +80,10 @@ const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS);
  * @typedef {'module' | 'commonjs'} PackageType
  * @typedef {{
  *   pjsonPath: string,
- *   exports?: ExportConfig;
- *   name?: string;
- *   main?: string;
- *   type?: PackageType;
+ *   exports?: ExportConfig,
+ *   name?: string,
+ *   main?: string,
+ *   type?: PackageType,
  * }} 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 explanation to users
+      assert.ok(stderr.includes('extensionless'));
+    }
   }));
 });
