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, doc: fix and improve specifier resolution warning #42252

Merged
merged 2 commits into from
Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ of Node.js applications.

> Stability: 1 - Experimental

**Note: This API is currently being redesigned and will still change.**
> This API is currently being redesigned and will still change.

<!-- type=misc -->

Expand All @@ -700,7 +700,7 @@ changes:
description: Add support for import assertions.
-->

> Note: The loaders API is being redesigned. This hook may disappear or its
> The loaders API is being redesigned. This hook may disappear or its
> signature may change. Do not rely on the API described below.

* `specifier` {string}
Expand Down Expand Up @@ -773,10 +773,10 @@ export async function resolve(specifier, context, defaultResolve) {

#### `load(url, context, defaultLoad)`

> Note: The loaders API is being redesigned. This hook may disappear or its
> The loaders API is being redesigned. This hook may disappear or its
> signature may change. Do not rely on the API described below.

> Note: In a previous version of this API, this was split across 3 separate, now
> In a previous version of this API, this was split across 3 separate, now
> deprecated, hooks (`getFormat`, `getSource`, and `transformSource`).

* `url` {string}
Expand Down Expand Up @@ -814,7 +814,7 @@ overcome in the future.
> are incompatible. Attempting to use them together will result in an empty
> object from the import. This may be addressed in the future.

> Note: These types all correspond to classes defined in ECMAScript.
> These types all correspond to classes defined in ECMAScript.

* The specific [`ArrayBuffer`][] object is a [`SharedArrayBuffer`][].
* The specific [`TypedArray`][] object is a [`Uint8Array`][].
Expand Down Expand Up @@ -864,10 +864,10 @@ source to a supported one (see [Examples](#examples) below).

#### `globalPreload()`

> Note: The loaders API is being redesigned. This hook may disappear or its
> The loaders API is being redesigned. This hook may disappear or its
> signature may change. Do not rely on the API described below.

> Note: In a previous version of this API, this hook was named
> In a previous version of this API, this hook was named
> `getGlobalPreloadCode`.

* Returns: {string}
Expand Down Expand Up @@ -1456,6 +1456,10 @@ _internal_, _conditions_)

> 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
Expand Down Expand Up @@ -1488,6 +1492,7 @@ success!
[Import Assertions]: #import-assertions
[Import Assertions proposal]: https://github.com/tc39/proposal-import-assertions
[JSON modules]: #json-modules
[Loaders API]: #loaders
[Node.js Module Resolution Algorithm]: #resolver-algorithm-specification
[Terminology]: #terminology
[URL]: https://url.spec.whatwg.org/
Expand Down
12 changes: 0 additions & 12 deletions lib/internal/modules/esm/formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ const { getOptionValue } = require('internal/options');


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

const extensionFormatMap = {
'__proto__': null,
Expand Down Expand Up @@ -43,17 +41,7 @@ function mimeToFormat(mime) {
return null;
}

let experimentalSpecifierResolutionWarned = false;
function getLegacyExtensionFormat(ext) {
if (
experimentalSpecifierResolution === 'node' &&
!experimentalSpecifierResolutionWarned
) {
process.emitWarning(
'The Node.js specifier resolution in ESM is experimental.',
'ExperimentalWarning');
experimentalSpecifierResolutionWarned = true;
}
return legacyExtensionFormatMap[ext];
}

Expand Down
8 changes: 8 additions & 0 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ function resolveDirectoryEntry(search) {
}

const encodedSepRegEx = /%2F|%5C/i;
let experimentalSpecifierResolutionWarned = false;
/**
* @param {URL} resolved
* @param {string | URL | undefined} base
Expand All @@ -376,6 +377,13 @@ function finalizeResolution(resolved, base, preserveSymlinks) {

let path = fileURLToPath(resolved);
if (getOptionValue('--experimental-specifier-resolution') === 'node') {
if (!experimentalSpecifierResolutionWarned) {
process.emitWarning(
'The Node.js specifier resolution flag is experimental. It could change or be removed at any time.',
'ExperimentalWarning');
experimentalSpecifierResolutionWarned = true;
}
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved

let file = resolveExtensionsWithTryExactName(resolved);

// Directory
Expand Down
24 changes: 24 additions & 0 deletions test/es-module/test-esm-specifiers-legacy-flag-warning.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { mustCall } from '../common/index.mjs';
import { fileURL } from '../common/fixtures.mjs';
import { match, strictEqual } from 'assert';
import { spawn } from 'child_process';
import { execPath } from 'process';

// Verify experimental warning is printed
const child = spawn(execPath, [
'--experimental-specifier-resolution=node',
'--input-type=module',
'--eval',
`import ${JSON.stringify(fileURL('es-module-specifiers', 'package-type-module'))}`,
]);

let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', mustCall((code, signal) => {
strictEqual(code, 0);
strictEqual(signal, null);
match(stderr, /ExperimentalWarning: The Node\.js specifier resolution flag is experimental/);
}));