From c777cb3368c922c8c49b9ac3f765a084b74dbce5 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Mon, 14 Mar 2022 19:59:32 -0500 Subject: [PATCH] esm: make extension-less errors in type:module actionable PR-URL: https://github.com/nodejs/node/pull/42301 Reviewed-By: James M Snell Reviewed-By: Jacob Smith Reviewed-By: Geoffrey Booth Reviewed-By: Antoine du Hamel --- doc/api/esm.md | 2 +- lib/internal/errors.js | 10 +++++--- lib/internal/modules/esm/get_format.js | 24 +++++++++++++++---- lib/internal/modules/esm/resolve.js | 8 +++---- .../test-esm-unknown-or-no-extension.js | 6 ++++- 5 files changed, 36 insertions(+), 14 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 4886123f8632e6..a45ec7927264f1 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1079,7 +1079,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 index 2cbac1735e2b29..04a752ea6522dc 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1593,9 +1593,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 index ec3ab0c6c68527..5ae0e17dcfb5e2 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -1,13 +1,14 @@ 'use strict'; const { - RegExpPrototypeExec, ObjectAssign, ObjectCreate, ObjectPrototypeHasOwnProperty, PromisePrototypeThen, PromiseResolve, + RegExpPrototypeExec, + 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; @@ -48,7 +49,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'; } @@ -59,7 +61,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 index 4ac3c7a9763fe8..a8bc02aeb0a739 100644 --- 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 index 3b1802a4dcedbd..40f840ad670cf3 100644 --- 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')); + } })); });