Skip to content

Commit

Permalink
esm: remove specifier resolution flag
Browse files Browse the repository at this point in the history
PR-URL: #44859
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
  • Loading branch information
GeoffreyBooth authored Oct 4, 2022
1 parent 417458d commit f594cc8
Show file tree
Hide file tree
Showing 26 changed files with 24 additions and 374 deletions.
18 changes: 0 additions & 18 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,23 +427,6 @@ added: REPLACEME

Use this flag to enable [ShadowRealm][] support.

### `--experimental-specifier-resolution=mode`

<!-- YAML
added:
- v13.4.0
- v12.16.0
-->

Sets the resolution algorithm for resolving ES module specifiers. Valid options
are `explicit` and `node`.

The default is `explicit`, which requires providing the full path to a
module. The `node` mode enables support for optional file extensions and
the ability to import a directory that has an index file.

See [customizing ESM specifier resolution][] for example usage.

### `--experimental-vm-modules`

<!-- YAML
Expand Down Expand Up @@ -2312,7 +2295,6 @@ done
[`worker_threads.threadId`]: worker_threads.md#workerthreadid
[conditional exports]: packages.md#conditional-exports
[context-aware]: addons.md#context-aware-addons
[customizing ESM specifier resolution]: esm.md#customizing-esm-specifier-resolution-algorithm
[debugger]: debugger.md
[debugging security implications]: https://nodejs.org/en/docs/guides/debugging-getting-started/#security-implications
[emit_warning]: process.md#processemitwarningwarning-options
Expand Down
29 changes: 4 additions & 25 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1518,31 +1518,9 @@ _isImports_, _conditions_)
### Customizing ESM specifier resolution algorithm
> Stability: 1 - Experimental
> Do not rely on this flag. We plan to remove it once the
> [Loaders API][] has advanced to the point that equivalent functionality can
> be achieved via custom loaders.
The current specifier resolution does not support all default behavior of
the CommonJS loader. One of the behavior differences is automatic resolution
of file extensions and the ability to import directories that have an index
file.
The `--experimental-specifier-resolution=[mode]` flag can be used to customize
the extension resolution algorithm. The default mode is `explicit`, which
requires the full path to a module be provided to the loader. To enable the
automatic extension resolution and importing from directories that include an
index file use the `node` mode.
```console
$ node index.mjs
success!
$ node index # Failure!
Error: Cannot find module
$ node --experimental-specifier-resolution=node index
success!
```
The [Loaders API][] provides a mechanism for customizing the ESM specifier
resolution algorithm. An example loader that provides CommonJS-style resolution
for ESM specifiers is [commonjs-extension-resolution-loader][].
<!-- Note: The cjs-module-lexer link should be kept in-sync with the deps version -->
Expand Down Expand Up @@ -1583,6 +1561,7 @@ success!
[`string`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String
[`util.TextDecoder`]: util.md#class-utiltextdecoder
[cjs-module-lexer]: https://github.com/nodejs/cjs-module-lexer/tree/1.2.2
[commonjs-extension-resolution-loader]: https://github.com/nodejs/loaders-test/tree/main/commonjs-extension-resolution-loader
[custom https loader]: #https-loader
[load hook]: #loadurl-context-nextload
[percent-encoded]: url.md#percent-encoding-in-urls
Expand Down
3 changes: 0 additions & 3 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,6 @@ Disable exposition of the Web Crypto API on the global scope.
.It Fl -no-experimental-repl-await
Disable top-level await keyword support in REPL.
.
.It Fl -experimental-specifier-resolution
Select extension resolution algorithm for ES Modules; either 'explicit' (default) or 'node'.
.
.It Fl -experimental-vm-modules
Enable experimental ES module support in VM module.
.
Expand Down
18 changes: 1 addition & 17 deletions lib/internal/modules/esm/formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const {
} = primordials;
const { getOptionValue } = require('internal/options');


const experimentalWasmModules = getOptionValue('--experimental-wasm-modules');

const extensionFormatMap = {
Expand All @@ -16,17 +15,8 @@ const extensionFormatMap = {
'.mjs': 'module',
};

const legacyExtensionFormatMap = {
'__proto__': null,
'.cjs': 'commonjs',
'.js': 'commonjs',
'.json': 'commonjs',
'.mjs': 'module',
'.node': 'commonjs',
};

if (experimentalWasmModules) {
extensionFormatMap['.wasm'] = legacyExtensionFormatMap['.wasm'] = 'wasm';
extensionFormatMap['.wasm'] = 'wasm';
}

/**
Expand All @@ -45,13 +35,7 @@ function mimeToFormat(mime) {
return null;
}

function getLegacyExtensionFormat(ext) {
return legacyExtensionFormatMap[ext];
}

module.exports = {
extensionFormatMap,
getLegacyExtensionFormat,
legacyExtensionFormatMap,
mimeToFormat,
};
35 changes: 14 additions & 21 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,11 @@ const { getOptionValue } = require('internal/options');
const { fetchModule } = require('internal/modules/esm/fetch_module');
const {
extensionFormatMap,
getLegacyExtensionFormat,
mimeToFormat,
} = require('internal/modules/esm/formats');

const experimentalNetworkImports =
getOptionValue('--experimental-network-imports');
const experimentalSpecifierResolution =
getOptionValue('--experimental-specifier-resolution');
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 @@ -61,25 +58,21 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
const format = extensionFormatMap[ext];
if (format) return format;

if (experimentalSpecifierResolution !== 'node') {
// Explicit undefined return indicates load hook should rerun format check
if (ignoreErrors) return undefined;
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);
// Explicit undefined return indicates load hook should rerun format check
if (ignoreErrors) { return undefined; }
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"}}`;
}

return getLegacyExtensionFormat(ext) ?? null;
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
}

/**
Expand Down
12 changes: 0 additions & 12 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ const { getOptionValue } = require('internal/options');

// [2] `validate...()`s throw the wrong error

let emittedSpecifierResolutionWarning = false;

/**
* A utility function to iterate through a hook chain, track advancement in the
* chain, and generate and supply the `next<HookName>` argument to the custom
Expand Down Expand Up @@ -241,16 +239,6 @@ class ESMLoader {
if (getOptionValue('--experimental-network-imports')) {
emitExperimentalWarning('Network Imports');
}
if (
!emittedSpecifierResolutionWarning &&
getOptionValue('--experimental-specifier-resolution') === 'node'
) {
process.emitWarning(
'The Node.js specifier resolution flag is experimental. It could change or be removed at any time.',
'ExperimentalWarning'
);
emittedSpecifierResolutionWarning = true;
}
}

/**
Expand Down
68 changes: 2 additions & 66 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const {
ArrayPrototypeConcat,
ArrayPrototypeJoin,
ArrayPrototypeShift,
JSONParse,
JSONStringify,
ObjectFreeze,
ObjectGetOwnPropertyNames,
Expand Down Expand Up @@ -37,7 +36,7 @@ const { getOptionValue } = require('internal/options');
const policy = getOptionValue('--experimental-policy') ?
require('internal/process/policy') :
null;
const { sep, relative, resolve } = require('path');
const { sep, relative } = require('path');
const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const experimentalNetworkImports =
Expand All @@ -60,7 +59,6 @@ const {
} = require('internal/errors').codes;

const { Module: CJSModule } = require('internal/modules/cjs/loader');
const packageJsonReader = require('internal/modules/package_json_reader');
const { getPackageConfig, getPackageScopeConfig } = require('internal/modules/esm/package_config');

/**
Expand Down Expand Up @@ -234,50 +232,6 @@ function legacyMainResolve(packageJSONUrl, packageConfig, base) {
fileURLToPath(new URL('.', packageJSONUrl)), fileURLToPath(base));
}

/**
* @param {URL} search
* @returns {URL | undefined}
*/
function resolveExtensionsWithTryExactName(search) {
if (fileExists(search)) return search;
return resolveExtensions(search);
}

const extensions = ['.js', '.json', '.node', '.mjs'];

/**
* @param {URL} search
* @returns {URL | undefined}
*/
function resolveExtensions(search) {
for (let i = 0; i < extensions.length; i++) {
const extension = extensions[i];
const guess = new URL(`${search.pathname}${extension}`, search);
if (fileExists(guess)) return guess;
}
return undefined;
}

/**
* @param {URL} search
* @returns {URL | undefined}
*/
function resolveDirectoryEntry(search) {
const dirPath = fileURLToPath(search);
const pkgJsonPath = resolve(dirPath, 'package.json');
if (fileExists(pkgJsonPath)) {
const pkgJson = packageJsonReader.read(pkgJsonPath);
if (pkgJson.containsKeys) {
const { main } = JSONParse(pkgJson.string);
if (main != null) {
const mainUrl = pathToFileURL(resolve(dirPath, main));
return resolveExtensionsWithTryExactName(mainUrl);
}
}
}
return resolveExtensions(new URL('index', search));
}

const encodedSepRegEx = /%2F|%5C/i;
/**
* @param {URL} resolved
Expand All @@ -291,25 +245,7 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
resolved.pathname, 'must not include encoded "/" or "\\" characters',
fileURLToPath(base));

let path = fileURLToPath(resolved);
if (getOptionValue('--experimental-specifier-resolution') === 'node') {
let file = resolveExtensionsWithTryExactName(resolved);

// Directory
if (file === undefined) {
file = StringPrototypeEndsWith(path, '/') ?
(resolveDirectoryEntry(resolved) || resolved) : resolveDirectoryEntry(new URL(`${resolved}/`));

if (file === resolved) return file;

if (file === undefined) {
throw new ERR_MODULE_NOT_FOUND(
resolved.pathname, fileURLToPath(base), 'module');
}
}

path = file;
}
const path = fileURLToPath(resolved);

const stats = tryStatSync(StringPrototypeEndsWith(path, '/') ?
StringPrototypeSlice(path, -1) : path);
Expand Down
4 changes: 0 additions & 4 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ function shouldUseESMLoader(mainPath) {
const userImports = getOptionValue('--import');
if (userLoaders.length > 0 || userImports.length > 0)
return true;
const esModuleSpecifierResolution =
getOptionValue('--experimental-specifier-resolution');
if (esModuleSpecifierResolution === 'node')
return true;
// Determine the module format of the main
if (mainPath && StringPrototypeEndsWith(mainPath, '.mjs'))
return true;
Expand Down
6 changes: 1 addition & 5 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ const {
const history = require('internal/repl/history');
const {
extensionFormatMap,
legacyExtensionFormatMap,
} = require('internal/modules/esm/formats');

let nextREPLResourceNumber = 1;
Expand Down Expand Up @@ -1377,10 +1376,7 @@ function complete(line, callback) {
if (this.allowBlockingCompletions) {
const subdir = match[2] || '';
// File extensions that can be imported:
const extensions = ObjectKeys(
getOptionValue('--experimental-specifier-resolution') === 'node' ?
legacyExtensionFormatMap :
extensionFormatMap);
const extensions = ObjectKeys(extensionFormatMap);

// Only used when loading bare module specifiers from `node_modules`:
const indexes = ArrayPrototypeMap(extensions, (ext) => `index${ext}`);
Expand Down
15 changes: 2 additions & 13 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,6 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
}
}

if (!experimental_specifier_resolution.empty()) {
if (experimental_specifier_resolution != "node" &&
experimental_specifier_resolution != "explicit") {
errors->push_back(
"invalid value for --experimental-specifier-resolution");
}
}

if (syntax_check_only && has_eval_string) {
errors->push_back("either --check or --eval can be used, not both");
}
Expand Down Expand Up @@ -444,11 +436,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"set module type for string input",
&EnvironmentOptions::module_type,
kAllowedInEnvironment);
AddOption("--experimental-specifier-resolution",
"Select extension resolution algorithm for es modules; "
"either 'explicit' (default) or 'node'",
&EnvironmentOptions::experimental_specifier_resolution,
kAllowedInEnvironment);
AddOption(
"--experimental-specifier-resolution", "", NoOp{}, kAllowedInEnvironment);
AddAlias("--es-module-specifier-resolution",
"--experimental-specifier-resolution");
AddOption("--deprecation",
Expand Down
1 change: 0 additions & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ class EnvironmentOptions : public Options {
bool experimental_global_customevent = false;
bool experimental_global_web_crypto = true;
bool experimental_https_modules = false;
std::string experimental_specifier_resolution;
bool experimental_wasm_modules = false;
bool experimental_import_meta_resolve = false;
std::string module_type;
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-experimental-warnings.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ describe('ESM: warn for obsolete hooks provided', { concurrency: true }, () => {
const [experiment, arg] of [
[/Custom ESM Loaders/, `--experimental-loader=${fileURL('es-module-loaders', 'hooks-custom.mjs')}`],
[/Network Imports/, '--experimental-network-imports'],
[/specifier resolution/, '--experimental-specifier-resolution=node'],
]
) {
it(`should print for ${experiment.toString().replaceAll('/', '')}`, async () => {
Expand Down
Loading

0 comments on commit f594cc8

Please sign in to comment.