Skip to content

Commit e55283b

Browse files
bmeckbengl
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 1fe0b69 commit e55283b

File tree

5 files changed

+35
-13
lines changed

5 files changed

+35
-13
lines changed

Diff for: doc/api/esm.md

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

Diff for: lib/internal/errors.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -1596,9 +1596,13 @@ E('ERR_UNHANDLED_ERROR',
15961596
E('ERR_UNKNOWN_BUILTIN_MODULE', 'No such built-in module: %s', Error);
15971597
E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error);
15981598
E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
1599-
E('ERR_UNKNOWN_FILE_EXTENSION',
1600-
'Unknown file extension "%s" for %s',
1601-
TypeError);
1599+
E('ERR_UNKNOWN_FILE_EXTENSION', (ext, path, suggestion) => {
1600+
let msg = `Unknown file extension "${ext}" for ${path}`;
1601+
if (suggestion) {
1602+
msg += `. ${suggestion}`;
1603+
}
1604+
return msg;
1605+
}, TypeError);
16021606
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s for URL %s',
16031607
RangeError);
16041608
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);

Diff for: lib/internal/modules/esm/get_format.js

+18-4
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ const {
66
ObjectPrototypeHasOwnProperty,
77
PromisePrototypeThen,
88
PromiseResolve,
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

@@ -52,7 +53,8 @@ function getDataProtocolModuleFormat(parsed) {
5253
* @returns {string}
5354
*/
5455
function getFileProtocolModuleFormat(url, context, ignoreErrors) {
55-
const ext = extname(url.pathname);
56+
const filepath = fileURLToPath(url);
57+
const ext = extname(filepath);
5658
if (ext === '.js') {
5759
return getPackageType(url) === 'module' ? 'module' : 'commonjs';
5860
}
@@ -63,7 +65,19 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
6365
if (experimentalSpecifierResolution !== 'node') {
6466
// Explicit undefined return indicates load hook should rerun format check
6567
if (ignoreErrors) return undefined;
66-
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, fileURLToPath(url));
68+
let suggestion = '';
69+
if (getPackageType(url) === 'module' && ext === '') {
70+
const config = getPackageScopeConfig(url);
71+
const fileBasename = basename(filepath);
72+
const relativePath = StringPrototypeSlice(relative(config.pjsonPath, filepath), 1);
73+
suggestion = 'Loading extensionless files is not supported inside of ' +
74+
'"type":"module" package.json contexts. The package.json file ' +
75+
`${config.pjsonPath} caused this "type":"module" context. Try ` +
76+
`changing ${filepath} to have a file extension. Note the "bin" ` +
77+
'field of package.json can point to a file with an extension, for example ' +
78+
`{"type":"module","bin":{"${fileBasename}":"${relativePath}.js"}}`;
79+
}
80+
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
6781
}
6882

6983
return getLegacyExtensionFormat(ext) ?? null;

Diff for: 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

Diff for: 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)