Skip to content

Commit c777cb3

Browse files
bmeckdanielleadams
authored andcommitted
esm: make extension-less errors in type:module actionable
PR-URL: #42301 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent ebe2b6d commit c777cb3

File tree

5 files changed

+36
-14
lines changed

5 files changed

+36
-14
lines changed

doc/api/esm.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,7 @@ async function getPackageType(url) {
10791079
// required by the spec
10801080
// this simple truthy check for whether `url` contains a file extension will
10811081
// work for most projects but does not cover some edge-cases (such as
1082-
// extension-less files or a url ending in a trailing space)
1082+
// extensionless files or a url ending in a trailing space)
10831083
const isFilePath = !!extname(url);
10841084
// If it is a file path, get the directory it's in
10851085
const dir = isFilePath ?

lib/internal/errors.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -1593,9 +1593,13 @@ E('ERR_UNHANDLED_ERROR',
15931593
E('ERR_UNKNOWN_BUILTIN_MODULE', 'No such built-in module: %s', Error);
15941594
E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error);
15951595
E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
1596-
E('ERR_UNKNOWN_FILE_EXTENSION',
1597-
'Unknown file extension "%s" for %s',
1598-
TypeError);
1596+
E('ERR_UNKNOWN_FILE_EXTENSION', (ext, path, suggestion) => {
1597+
let msg = `Unknown file extension "${ext}" for ${path}`;
1598+
if (suggestion) {
1599+
msg += `. ${suggestion}`;
1600+
}
1601+
return msg;
1602+
}, TypeError);
15991603
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s for URL %s',
16001604
RangeError);
16011605
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);

lib/internal/modules/esm/get_format.js

+19-5
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
'use strict';
22
const {
3-
RegExpPrototypeExec,
43
ObjectAssign,
54
ObjectCreate,
65
ObjectPrototypeHasOwnProperty,
76
PromisePrototypeThen,
87
PromiseResolve,
8+
RegExpPrototypeExec,
9+
StringPrototypeSlice,
910
} = primordials;
10-
const { extname } = require('path');
11+
const { basename, extname, relative } = require('path');
1112
const { getOptionValue } = require('internal/options');
1213
const { fetchModule } = require('internal/modules/esm/fetch_module');
1314
const {
@@ -20,7 +21,7 @@ const experimentalNetworkImports =
2021
getOptionValue('--experimental-network-imports');
2122
const experimentalSpecifierResolution =
2223
getOptionValue('--experimental-specifier-resolution');
23-
const { getPackageType } = require('internal/modules/esm/resolve');
24+
const { getPackageType, getPackageScopeConfig } = require('internal/modules/esm/resolve');
2425
const { URL, fileURLToPath } = require('internal/url');
2526
const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
2627

@@ -48,7 +49,8 @@ function getDataProtocolModuleFormat(parsed) {
4849
* @returns {string}
4950
*/
5051
function getFileProtocolModuleFormat(url, context, ignoreErrors) {
51-
const ext = extname(url.pathname);
52+
const filepath = fileURLToPath(url);
53+
const ext = extname(filepath);
5254
if (ext === '.js') {
5355
return getPackageType(url) === 'module' ? 'module' : 'commonjs';
5456
}
@@ -59,7 +61,19 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
5961
if (experimentalSpecifierResolution !== 'node') {
6062
// Explicit undefined return indicates load hook should rerun format check
6163
if (ignoreErrors) return undefined;
62-
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, fileURLToPath(url));
64+
let suggestion = '';
65+
if (getPackageType(url) === 'module' && ext === '') {
66+
const config = getPackageScopeConfig(url);
67+
const fileBasename = basename(filepath);
68+
const relativePath = StringPrototypeSlice(relative(config.pjsonPath, filepath), 1);
69+
suggestion = 'Loading extensionless files is not supported inside of ' +
70+
'"type":"module" package.json contexts. The package.json file ' +
71+
`${config.pjsonPath} caused this "type":"module" context. Try ` +
72+
`changing ${filepath} to have a file extension. Note the "bin" ` +
73+
'field of package.json can point to a file with an extension, for example ' +
74+
`{"type":"module","bin":{"${fileBasename}":"${relativePath}.js"}}`;
75+
}
76+
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
6377
}
6478

6579
return getLegacyExtensionFormat(ext) ?? null;

lib/internal/modules/esm/resolve.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@ const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS);
8080
* @typedef {'module' | 'commonjs'} PackageType
8181
* @typedef {{
8282
* pjsonPath: string,
83-
* exports?: ExportConfig;
84-
* name?: string;
85-
* main?: string;
86-
* type?: PackageType;
83+
* exports?: ExportConfig,
84+
* name?: string,
85+
* main?: string,
86+
* type?: PackageType,
8787
* }} PackageConfig
8888
*/
8989

test/es-module/test-esm-unknown-or-no-extension.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ const assert = require('assert');
3131
assert.strictEqual(code, 1);
3232
assert.strictEqual(signal, null);
3333
assert.strictEqual(stdout, '');
34-
assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1);
34+
assert.ok(stderr.includes('ERR_UNKNOWN_FILE_EXTENSION'));
35+
if (fixturePath.includes('noext')) {
36+
// Check for explanation to users
37+
assert.ok(stderr.includes('extensionless'));
38+
}
3539
}));
3640
});

0 commit comments

Comments
 (0)