Skip to content

Commit

Permalink
esm: make extension-less errors in type:module actionable
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bmeck authored and danielleadams committed Apr 24, 2022
1 parent ebe2b6d commit c777cb3
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 14 deletions.
2 changes: 1 addition & 1 deletion doc/api/esm.md
Expand Up @@ -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 ?
Expand Down
10 changes: 7 additions & 3 deletions lib/internal/errors.js
Expand Up @@ -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);
Expand Down
24 changes: 19 additions & 5 deletions 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 {
Expand All @@ -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;

Expand Down Expand Up @@ -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';
}
Expand All @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/modules/esm/resolve.js
Expand Up @@ -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
*/

Expand Down
6 changes: 5 additions & 1 deletion test/es-module/test-esm-unknown-or-no-extension.js
Expand Up @@ -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'));
}
}));
});

0 comments on commit c777cb3

Please sign in to comment.