esm: convert resolve hook to synchronous

doc/api/esm.md

@@ -338,15 +338,15 @@ command flag enabled.
 * `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: {Promise}
+* Returns: {string}
 
 Provides a module-relative resolution function scoped to each module, returning
 the URL string.
 
 <!-- eslint-skip -->
 
 ```js
-const dependencyAsset = await import.meta.resolve('component-lib/asset.css');
+const dependencyAsset = import.meta.resolve('component-lib/asset.css');
 ```
 
 `import.meta.resolve` also accepts a second argument which is the parent module
@@ -355,7 +355,7 @@ from which to resolve from:
 <!-- eslint-skip -->
 
 ```js
-await import.meta.resolve('./dep', import.meta.url);
+import.meta.resolve('./dep', import.meta.url);
 ```
 
 This function is asynchronous because the ES module resolver in Node.js is
@@ -730,6 +730,9 @@ prevent unintentional breaks in the chain.
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43363
+    description: Convert hook from asynchronous to asynchronous.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/42623
     description: Add support for chaining resolve hooks. Each hook must either
@@ -789,7 +792,7 @@ Node.js module specifier resolution behavior_ when calling `defaultResolve`, the
 `context.conditions` array originally passed into the `resolve` hook.
 
 ```js
-export async function resolve(specifier, context, nextResolve) {
+export function resolve(specifier, context, nextResolve) {
   const { parentURL = null } = context;
 
   if (Math.random() > 0.5) { // Some condition.
@@ -1088,7 +1091,7 @@ const baseURL = pathToFileURL(`${cwd()}/`).href;
 // CoffeeScript files end in .coffee, .litcoffee, or .coffee.md.
 const extensionsRegex = /\.coffee$|\.litcoffee$|\.coffee\.md$/;
 
-export async function resolve(specifier, context, nextResolve) {
+export function resolve(specifier, context, nextResolve) {
   if (extensionsRegex.test(specifier)) {
     const { parentURL = baseURL } = context;
 

lib/internal/modules/esm/initialize_import_meta.js

@@ -3,21 +3,23 @@
 const { getOptionValue } = require('internal/options');
 const experimentalImportMetaResolve =
   getOptionValue('--experimental-import-meta-resolve');
-const {
-  PromisePrototypeThen,
-  PromiseReject,
-} = primordials;
 const asyncESM = require('internal/process/esm_loader');
 
 function createImportMetaResolve(defaultParentUrl) {
-  return async function resolve(specifier, parentUrl = defaultParentUrl) {
-    return PromisePrototypeThen(
-      asyncESM.esmLoader.resolve(specifier, parentUrl),
-      ({ url }) => url,
-      (error) => (
-        error.code === 'ERR_UNSUPPORTED_DIR_IMPORT' ?
-          error.url : PromiseReject(error))
-    );
+  return function resolve(specifier, parentUrl = defaultParentUrl) {
+    let url;
+
+    try {
+      ({ url } = asyncESM.esmLoader.resolve(specifier, parentUrl));
+    } catch (error) {
+      if (error.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
+        ({ url } = error);
+      } else {
+        throw error;
+      }
+    }
+
+    return url;
   };
 }
 

lib/internal/modules/esm/loader.js

@@ -15,6 +15,8 @@ const {
   ObjectDefineProperty,
   ObjectSetPrototypeOf,
   PromiseAll,
+  PromiseResolve,
+  PromisePrototypeThen,
   ReflectApply,
   RegExpPrototypeExec,
   SafeArrayIterator,
@@ -109,12 +111,14 @@ let emittedSpecifierResolutionWarning = false;
  * position in the hook chain.
  * @param {string} meta.hookName The kind of hook the chain is (ex 'resolve')
  * @param {boolean} meta.shortCircuited Whether a hook signaled a short-circuit.
- * @param {(hookErrIdentifier, hookArgs) => void} validate A wrapper function
+ * @param {object} validators A wrapper function
  *  containing all validation of a custom loader hook's intermediary output. Any
  *  validation within MUST throw.
+ * @param {(hookErrIdentifier, hookArgs) => void} validateArgs
+ * @param {(hookErrIdentifier, output) => void} validateOutput
  * @returns {function next<HookName>(...hookArgs)} The next hook in the chain.
  */
-function nextHookFactory(chain, meta, validate) {
+function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
   // First, prepare the current
   const { hookName } = meta;
   const {
@@ -137,7 +141,7 @@ function nextHookFactory(chain, meta, validate) {
     // factory generates the next link in the chain.
     meta.hookIndex--;
 
-    nextNextHook = nextHookFactory(chain, meta, validate);
+    nextNextHook = nextHookFactory(chain, meta, { validateArgs, validateOutput });
   } else {
     // eslint-disable-next-line func-name-matching
     nextNextHook = function chainAdvancedTooFar() {
@@ -148,21 +152,34 @@ function nextHookFactory(chain, meta, validate) {
   }
 
   return ObjectDefineProperty(
-    async (...args) => {
+    (...args) => {
       // Update only when hook is invoked to avoid fingering the wrong filePath
       meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`;
 
-      validate(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args);
+      validateArgs(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args);
+
+      const outputErrIdentifier = `${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`;
 
       // Set when next<HookName> is actually called, not just generated.
       if (generatedHookIndex === 0) { meta.chainFinished = true; }
 
       ArrayPrototypePush(args, nextNextHook);
-      const output = await ReflectApply(hook, undefined, args);
+      let output = ReflectApply(hook, undefined, args);
 
-      if (output?.shortCircuit === true) { meta.shortCircuited = true; }
-      return output;
+      validateOutput(outputErrIdentifier, output);
+
+      function checkShortCircuited(output) {
+        if (output?.shortCircuit === true) { meta.shortCircuited = true; }
+      }
 
+      if (meta.isChainAsync) {
+        output = PromiseResolve(output);
+        PromisePrototypeThen(output, checkShortCircuited);
+      } else {
+        checkShortCircuited(output);
+      }
+
+      return output;
     },
     'name',
     { __proto__: null, value: nextHookName },
@@ -421,8 +438,11 @@ class ESMLoader {
       );
     }
 
-    const { format, url } =
-      await this.resolve(specifier, parentURL, importAssertionsForResolve);
+    const { format, url } = this.resolve(
+      specifier,
+      parentURL,
+      importAssertionsForResolve,
+    );
 
     let job = this.moduleMap.get(url, importAssertions.type);
 
@@ -557,10 +577,11 @@ class ESMLoader {
       hookErrIdentifier: '',
       hookIndex: chain.length - 1,
       hookName: 'load',
+      isChainAsync: true,
       shortCircuited: false,
     };
 
-    const validate = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
+    const validateArgs = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
       if (typeof nextUrl !== 'string') {
         // non-strings can be coerced to a url string
         // validateString() throws a less-specific error
@@ -586,19 +607,22 @@ class ESMLoader {
 
       validateObject(ctx, `${hookErrIdentifier} context`);
     };
+    const validateOutput = (hookErrIdentifier, output) => {
+      if (typeof output !== 'object') { // [2]
+        throw new ERR_INVALID_RETURN_VALUE(
+          'an object',
+          hookErrIdentifier,
+          output,
+        );
+      }
+    };
 
-    const nextLoad = nextHookFactory(chain, meta, validate);
+    const nextLoad = nextHookFactory(chain, meta, { validateArgs, validateOutput });
 
     const loaded = await nextLoad(url, context);
     const { hookErrIdentifier } = meta; // Retrieve the value after all settled
 
-    if (typeof loaded !== 'object') { // [2]
-      throw new ERR_INVALID_RETURN_VALUE(
-        'an object',
-        hookErrIdentifier,
-        loaded,
-      );
-    }
+    validateOutput(hookErrIdentifier, loaded);
 
     if (loaded?.shortCircuit === true) { meta.shortCircuited = true; }
 
@@ -778,7 +802,7 @@ class ESMLoader {
    *                                              statement or expression.
    * @returns {{ format: string, url: URL['href'] }}
    */
-  async resolve(
+  resolve(
     originalSpecifier,
     parentURL,
     importAssertions = ObjectCreate(null)
@@ -802,36 +826,42 @@ class ESMLoader {
       hookErrIdentifier: '',
       hookIndex: chain.length - 1,
       hookName: 'resolve',
+      isChainAsync: false,
       shortCircuited: false,
     };
-
     const context = {
       conditions: DEFAULT_CONDITIONS,
       importAssertions,
       parentURL,
     };
-    const validate = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
 
+    const validateArgs = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
       validateString(
         suppliedSpecifier,
         `${hookErrIdentifier} specifier`,
       ); // non-strings can be coerced to a url string
 
       validateObject(ctx, `${hookErrIdentifier} context`);
     };
+    const validateOutput = (hookErrIdentifier, output) => {
+      if (
+        typeof output !== 'object' || // [2]
+        typeof output.then === 'function'
+      ) {
+        throw new ERR_INVALID_RETURN_VALUE(
+          'an object',
+          hookErrIdentifier,
+          output,
+        );
+      }
+    };
 
-    const nextResolve = nextHookFactory(chain, meta, validate);
+    const nextResolve = nextHookFactory(chain, meta, { validateArgs, validateOutput });
 
-    const resolution = await nextResolve(originalSpecifier, context);
+    const resolution = nextResolve(originalSpecifier, context);
     const { hookErrIdentifier } = meta; // Retrieve the value after all settled
 
-    if (typeof resolution !== 'object') { // [2]
-      throw new ERR_INVALID_RETURN_VALUE(
-        'an object',
-        hookErrIdentifier,
-        resolution,
-      );
-    }
+    validateOutput(hookErrIdentifier, resolution);
 
     if (resolution?.shortCircuit === true) { meta.shortCircuited = true; }
 

lib/internal/modules/esm/resolve.js

@@ -1081,7 +1081,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
   }
 }
 
-async function defaultResolve(specifier, context = {}) {
+function defaultResolve(specifier, context = {}) {
   let { parentURL, conditions } = context;
   if (parentURL && policy?.manifest) {
     const redirects = policy.manifest.getDependencyMapper(parentURL);
@@ -1227,11 +1227,11 @@ const {
 
 if (policy) {
   const $defaultResolve = defaultResolve;
-  module.exports.defaultResolve = async function defaultResolve(
+  module.exports.defaultResolve = function defaultResolve(
     specifier,
     context
   ) {
-    const ret = await $defaultResolve(specifier, context, $defaultResolve);
+    const ret = $defaultResolve(specifier, context);
     // This is a preflight check to avoid data exfiltration by query params etc.
     policy.manifest.mightAllow(ret.url, () =>
       new ERR_MANIFEST_DEPENDENCY_MISSING(

test/es-module/test-esm-import-meta-resolve.mjs

@@ -1,37 +1,49 @@
 // Flags: --experimental-import-meta-resolve
-import { mustCall } from '../common/index.mjs';
+import '../common/index.mjs';
 import assert from 'assert';
 
 const dirname = import.meta.url.slice(0, import.meta.url.lastIndexOf('/') + 1);
 const fixtures = dirname.slice(0, dirname.lastIndexOf('/', dirname.length - 2) +
     1) + 'fixtures/';
 
-(async () => {
-  assert.strictEqual(await import.meta.resolve('./test-esm-import-meta.mjs'),
-                     dirname + 'test-esm-import-meta.mjs');
-  try {
-    await import.meta.resolve('./notfound.mjs');
-    assert.fail();
-  } catch (e) {
-    assert.strictEqual(e.code, 'ERR_MODULE_NOT_FOUND');
-  }
-  assert.strictEqual(
-    await import.meta.resolve('../fixtures/empty-with-bom.txt'),
-    fixtures + 'empty-with-bom.txt');
-  assert.strictEqual(await import.meta.resolve('../fixtures/'), fixtures);
-  assert.strictEqual(
-    await import.meta.resolve('../fixtures/', import.meta.url),
-    fixtures);
-  assert.strictEqual(
-    await import.meta.resolve('../fixtures/', new URL(import.meta.url)),
-    fixtures);
-  await Promise.all(
-    [[], {}, Symbol(), 0, 1, 1n, 1.1, () => {}, true, false].map((arg) =>
-      assert.rejects(import.meta.resolve('../fixtures/', arg), {
-        code: 'ERR_INVALID_ARG_TYPE',
-      })
-    )
+assert.strictEqual(
+  import.meta.resolve('./test-esm-import-meta.mjs'),
+  dirname + 'test-esm-import-meta.mjs',
+);
+
+assert.throws(
+  () => import.meta.resolve('./notfound.mjs'),
+  { code: 'ERR_MODULE_NOT_FOUND' },
+);
+
+assert.strictEqual(
+  import.meta.resolve('../fixtures/empty-with-bom.txt'),
+  fixtures + 'empty-with-bom.txt',
+);
+
+assert.strictEqual(
+  import.meta.resolve('../fixtures/'),
+  fixtures,
+);
+
+assert.strictEqual(
+  import.meta.resolve('../fixtures/', import.meta.url),
+  fixtures,
+);
+
+assert.strictEqual(
+  import.meta.resolve('../fixtures/', new URL(import.meta.url)),
+  fixtures,
+);
+
+[[], {}, Symbol(), 0, 1, 1n, 1.1, () => {}, true, false].forEach((arg) => {
+  assert.throws(
+    () => import.meta.resolve('../fixtures/', arg),
+    { code: 'ERR_INVALID_ARG_TYPE' },
   );
-  assert.strictEqual(await import.meta.resolve('baz/', fixtures),
-                     fixtures + 'node_modules/baz/');
-})().then(mustCall());
+});
+
+assert.strictEqual(
+  import.meta.resolve('baz/', fixtures),
+  fixtures + 'node_modules/baz/',
+);