esm: unflag import.meta.resolve

PR-URL: #49028 Backport-PR-URL: #50669 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
nodejs · Nov 23, 2023 · c1a8439 · c1a8439
1 parent ef43f08
commit c1a8439
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 35 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -372,9 +372,18 @@ Expose the [Web Crypto API][] on the global scope.
 added:
   - v13.9.0
   - v12.16.2
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/49028
+    description: synchronous import.meta.resolve made available by default, with
+                 the flag retained for enabling the experimental second argument
+                 as previously supported.
 -->
 
-Enable experimental `import.meta.resolve()` support.
+Enable experimental `import.meta.resolve()` parent URL support, which allows
+passing a second `parentURL` argument for contextual resolution.
+
+Previously gated the entire `import.meta.resolve` feature.
 
 ### `--experimental-loader=module`
 

diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -321,7 +321,7 @@ import { readFileSync } from 'node:fs';
 const buffer = readFileSync(new URL('./data.proto', import.meta.url));
 ```
 
-### `import.meta.resolve(specifier[, parent])`
+### `import.meta.resolve(specifier)`
 
 <!--
 added:
@@ -336,36 +336,45 @@ changes:
       - v14.18.0
     pr-url: https://github.com/nodejs/node/pull/38587
     description: Add support for WHATWG `URL` object to `parentURL` parameter.
+  - version:
+      - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/49028
+    description: Unflag import.meta.resolve, with `parentURL` parameter still
+                 flagged.
 -->
 
-> Stability: 1 - Experimental
-
-This feature is only available with the `--experimental-import-meta-resolve`
-command flag enabled.
+> Stability: 1.2 - Release candidate
 
-* `specifier` {string} The module specifier to resolve relative to `parent`.
-* `parent` {string|URL} The absolute parent module URL to resolve from. If none
-  is specified, the value of `import.meta.url` is used as the default.
-* Returns: {string}
+* `specifier` {string} The module specifier to resolve relative to the
+  current module.
+* Returns: {string} The absolute (`file:`) URL string for the resolved module.
 
-Provides a module-relative resolution function scoped to each module, returning
-the URL string. In alignment with browser behavior, this now returns
-synchronously.
-
-> **Caveat** This can result in synchronous file-system operations, which
-> can impact performance similarly to `require.resolve`.
+[`import.meta.resolve`][] is a module-relative resolution function scoped to
+each module, returning the URL string.
 
 ```js
 const dependencyAsset = import.meta.resolve('component-lib/asset.css');
+// file:///app/node_modules/component-lib/asset.css
 ```
 
-`import.meta.resolve` also accepts a second argument which is the parent module
-from which to resolve:
+All features of the Node.js module resolution are supported. Dependency
+resolutions are subject to the permitted exports resolutions within the package.
 
 ```js
 import.meta.resolve('./dep', import.meta.url);
+// file:///app/dep
 ```
 
+> **Caveat** This can result in synchronous file-system operations, which
+> can impact performance similarly to `require.resolve`.
+
+Previously, Node.js implemented an asynchronous resolver which also permitted
+a second contextual argument. The implementation has since been updated to be
+synchronous, with the second contextual `parent` argument still accessible
+behind the `--experimental-import-meta-resolve` flag:
+
+* `parent` {string|URL} An optional absolute parent module URL to resolve from.
+
 ## Interoperability with CommonJS
 
 ### `import` statements
@@ -500,8 +509,8 @@ They can instead be loaded with [`module.createRequire()`][] or
 
 Relative resolution can be handled via `new URL('./local', import.meta.url)`.
 
-For a complete `require.resolve` replacement, there is a flagged experimental
-[`import.meta.resolve`][] API.
+For a complete `require.resolve` replacement, there is the
+[import.meta.resolve][] API.
 
 Alternatively `module.createRequire()` can be used.
 
@@ -1705,7 +1714,7 @@ success!
 [`data:` URLs]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs
 [`export`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export
 [`import()`]: #import-expressions
-[`import.meta.resolve`]: #importmetaresolvespecifier-parent
+[`import.meta.resolve`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta/resolve
 [`import.meta.url`]: #importmetaurl
 [`import`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import
 [`initialize`]: #initialize
@@ -1722,6 +1731,7 @@ success!
 [`util.TextDecoder`]: util.md#class-utiltextdecoder
 [cjs-module-lexer]: https://github.com/nodejs/cjs-module-lexer/tree/1.2.2
 [custom https loader]: #https-loader
+[import.meta.resolve]: #importmetaresolvespecifier
 [load hook]: #loadurl-context-nextload
 [percent-encoded]: url.md#percent-encoding-in-urls
 [special scheme]: https://url.spec.whatwg.org/#special-scheme

diff --git a/lib/internal/modules/esm/initialize_import_meta.js b/lib/internal/modules/esm/initialize_import_meta.js
@@ -7,11 +7,22 @@ const experimentalImportMetaResolve = getOptionValue('--experimental-import-meta
  * Generate a function to be used as import.meta.resolve for a particular module.
  * @param {string} defaultParentURL The default base to use for resolution
  * @param {typeof import('./loader.js').ModuleLoader} loader Reference to the current module loader
- * @returns {(specifier: string, parentURL?: string) => string} Function to assign to import.meta.resolve
+ * @param {bool} allowParentURL Whether to permit parentURL second argument for contextual resolution
+ * @returns {(specifier: string) => string} Function to assign to import.meta.resolve
  */
-function createImportMetaResolve(defaultParentURL, loader) {
+function createImportMetaResolve(defaultParentURL, loader, allowParentURL) {
+  /**
+   * @param {string} specifier
+   * @param {URL['href']} [parentURL] When `--experimental-import-meta-resolve` is specified, a
+   * second argument can be provided.
+   */
   return function resolve(specifier, parentURL = defaultParentURL) {
     let url;
+
+    if (!allowParentURL) {
+      parentURL = defaultParentURL;
+    }
+
     try {
       ({ url } = loader.resolveSync(specifier, parentURL));
       return url;
@@ -40,8 +51,8 @@ function initializeImportMeta(meta, context, loader) {
   const { url } = context;
 
   // Alphabetical
-  if (experimentalImportMetaResolve && loader.allowImportMetaResolve) {
-    meta.resolve = createImportMetaResolve(url, loader);
+  if (!loader || loader.allowImportMetaResolve) {
+    meta.resolve = createImportMetaResolve(url, loader, experimentalImportMetaResolve);
   }
 
   meta.url = url;

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -399,7 +399,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::experimental_wasm_modules,
             kAllowedInEnvvar);
   AddOption("--experimental-import-meta-resolve",
-            "experimental ES Module import.meta.resolve() support",
+            "experimental ES Module import.meta.resolve() parentURL support",
             &EnvironmentOptions::experimental_import_meta_resolve,
             kAllowedInEnvvar);
   AddOption("--experimental-policy",

diff --git a/test/es-module/test-esm-import-meta-resolve.mjs b/test/es-module/test-esm-import-meta-resolve.mjs
@@ -39,7 +39,6 @@ assert.strictEqual(import.meta.resolve('baz/', fixtures),
 
 {
   const cp = spawn(execPath, [
-    '--experimental-import-meta-resolve',
     '--input-type=module',
     '--eval', 'console.log(typeof import.meta.resolve)',
   ]);
@@ -48,7 +47,6 @@ assert.strictEqual(import.meta.resolve('baz/', fixtures),
 
 {
   const cp = spawn(execPath, [
-    '--experimental-import-meta-resolve',
     '--input-type=module',
   ]);
   cp.stdin.end('console.log(typeof import.meta.resolve)');
@@ -57,7 +55,6 @@ assert.strictEqual(import.meta.resolve('baz/', fixtures),
 
 {
   const cp = spawn(execPath, [
-    '--experimental-import-meta-resolve',
     '--input-type=module',
     '--eval', 'import "data:text/javascript,console.log(import.meta.resolve(%22node:os%22))"',
   ]);
@@ -66,7 +63,6 @@ assert.strictEqual(import.meta.resolve('baz/', fixtures),
 
 {
   const cp = spawn(execPath, [
-    '--experimental-import-meta-resolve',
     '--input-type=module',
   ]);
   cp.stdin.end('import "data:text/javascript,console.log(import.meta.resolve(%22node:os%22))"');

diff --git a/test/es-module/test-esm-import-meta.mjs b/test/es-module/test-esm-import-meta.mjs
@@ -3,7 +3,7 @@ import assert from 'assert';
 
 assert.strictEqual(Object.getPrototypeOf(import.meta), null);
 
-const keys = ['url'];
+const keys = ['resolve', 'url'];
 assert.deepStrictEqual(Reflect.ownKeys(import.meta), keys);
 
 const descriptors = Object.getOwnPropertyDescriptors(import.meta);

diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs
@@ -94,7 +94,6 @@ describe('Loader hooks', { concurrency: true }, () => {
     it('import.meta.resolve of a never-settling resolve', async () => {
       const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
         '--no-warnings',
-        '--experimental-import-meta-resolve',
         '--experimental-loader',
         fixtures.fileURL('es-module-loaders/never-settling-resolve-step/loader.mjs'),
         fixtures.path('es-module-loaders/never-settling-resolve-step/import.meta.never-resolve.mjs'),
@@ -155,7 +154,6 @@ describe('Loader hooks', { concurrency: true }, () => {
   it('should not leak internals or expose import.meta.resolve', async () => {
     const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
       '--no-warnings',
-      '--experimental-import-meta-resolve',
       '--experimental-loader',
       fixtures.fileURL('es-module-loaders/loader-edge-cases.mjs'),
       fixtures.path('empty.js'),
@@ -170,7 +168,6 @@ describe('Loader hooks', { concurrency: true }, () => {
   it('should be fine to call `process.exit` from a custom async hook', async () => {
     const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
       '--no-warnings',
-      '--experimental-import-meta-resolve',
       '--experimental-loader',
       'data:text/javascript,export function load(a,b,next){if(a==="data:exit")process.exit(42);return next(a,b)}',
       '--input-type=module',
@@ -187,7 +184,6 @@ describe('Loader hooks', { concurrency: true }, () => {
   it('should be fine to call `process.exit` from a custom sync hook', async () => {
     const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
       '--no-warnings',
-      '--experimental-import-meta-resolve',
       '--experimental-loader',
       'data:text/javascript,export function resolve(a,b,next){if(a==="exit:")process.exit(42);return next(a,b)}',
       '--input-type=module',