Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from Mar 15, 2022
2 changes: 1 addition & 1 deletion doc/api/esm.md
Expand Up @@ -1091,7 +1091,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 @@ -1594,9 +1594,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
22 changes: 18 additions & 4 deletions lib/internal/modules/esm/get_format.js
Expand Up @@ -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 {
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 @@ -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';
}
Expand All @@ -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;
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'));
}
}));
});