esm: convert resolve hook to synchronous

lib/internal/modules/esm/initialize_import_meta.js

@@ -3,21 +3,11 @@
 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 asyncESM.esmLoader.resolve(specifier, parentUrl);
   };
 }
 

lib/internal/modules/esm/loader.js

@@ -148,7 +148,7 @@ 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}'`;
 
@@ -158,11 +158,19 @@ function nextHookFactory(chain, meta, validate) {
       if (generatedHookIndex === 0) { meta.chainFinished = true; }
 
       ArrayPrototypePush(args, nextNextHook);
-      const output = await ReflectApply(hook, undefined, args);
+      const output = ReflectApply(hook, undefined, args);
 
-      if (output?.shortCircuit === true) { meta.shortCircuited = true; }
-      return output;
+      function checkShortCircuited(output) {
+        if (output?.shortCircuit === true) { meta.shortCircuited = true; }
+      }
+
+      if (output instanceof Promise) { // eslint-disable-line node-core/prefer-primordials
+        output?.then(checkShortCircuited);
+      } else {
+        checkShortCircuited(output);
+      }
 
+      return output;
     },
     'name',
     { __proto__: null, value: nextHookName },
@@ -421,8 +429,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);
 
@@ -778,7 +789,7 @@ class ESMLoader {
    *                                              statement or expression.
    * @returns {{ format: string, url: URL['href'] }}
    */
-  async resolve(
+  resolve(
     originalSpecifier,
     parentURL,
     importAssertions = ObjectCreate(null)
@@ -804,13 +815,22 @@ class ESMLoader {
       hookName: 'resolve',
       shortCircuited: false,
     };
-
     const context = {
       conditions: DEFAULT_CONDITIONS,
       importAssertions,
       parentURL,
     };
-    const validate = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
+
+    const validate = (hookErrIdentifier, output) => {
+      if (output instanceof Promise) { // eslint-disable-line node-core/prefer-primordials
+        throw ERR_INVALID_RETURN_VALUE(
+          'an object',
+          hookErrIdentifier,
+          output,
+        );
+      }
+
+      const { 0: suppliedSpecifier, 1: ctx } = output;
 
       validateString(
         suppliedSpecifier,
@@ -822,10 +842,13 @@ class ESMLoader {
 
     const nextResolve = nextHookFactory(chain, meta, validate);
 
-    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]
+    if (
+      typeof resolution !== 'object' || // [2]
+      resolution instanceof Promise // eslint-disable-line node-core/prefer-primordials
+    ) {
       throw new ERR_INVALID_RETURN_VALUE(
         'an object',
         hookErrIdentifier,

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

@@ -7,31 +7,31 @@ const fixtures = dirname.slice(0, dirname.lastIndexOf('/', dirname.length - 2) +
     1) + 'fixtures/';
 
 (async () => {
-  assert.strictEqual(await import.meta.resolve('./test-esm-import-meta.mjs'),
+  assert.strictEqual(import.meta.resolve('./test-esm-import-meta.mjs'),
                      dirname + 'test-esm-import-meta.mjs');
   try {
-    await import.meta.resolve('./notfound.mjs');
+    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'),
+    import.meta.resolve('../fixtures/empty-with-bom.txt'),
     fixtures + 'empty-with-bom.txt');
-  assert.strictEqual(await import.meta.resolve('../fixtures/'), fixtures);
+  assert.strictEqual(import.meta.resolve('../fixtures/'), fixtures);
   assert.strictEqual(
-    await import.meta.resolve('../fixtures/', import.meta.url),
+    import.meta.resolve('../fixtures/', import.meta.url),
     fixtures);
   assert.strictEqual(
-    await import.meta.resolve('../fixtures/', new URL(import.meta.url)),
+    import.meta.resolve('../fixtures/', new URL(import.meta.url)),
     fixtures);
-  await Promise.all(
+  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(await import.meta.resolve('baz/', fixtures),
+  assert.strictEqual(import.meta.resolve('baz/', fixtures),
                      fixtures + 'node_modules/baz/');
 })().then(mustCall());

test/es-module/test-esm-loader-chaining.mjs

@@ -165,6 +165,25 @@ const commonArgs = [
   assert.strictEqual(status, 0);
 }
 
+{ // Verify error thrown for an async resolve hook
+  const { status, stderr, stdout } = spawnSync(
+    process.execPath,
+    [
+      '--loader',
+      fixtures.fileURL('es-module-loaders', 'loader-resolve-async-fn.mjs'),
+      ...commonArgs,
+    ],
+    { encoding: 'utf8' },
+  );
+
+  assert.match(stderr, /ERR_INVALID_RETURN_VALUE/);
+  assert.match(stderr, /Promise/);
+  assert.match(stderr, /loader-resolve-async-fn\.mjs/);
+  assert.match(stderr, /'resolve'/);
+  assert.strictEqual(stdout, '');
+  assert.strictEqual(status, 1);
+}
+
 { // Verify error thrown for incomplete resolve chain, citing errant loader & hook
   const { status, stderr, stdout } = spawnSync(
     process.execPath,

test/es-module/test-esm-loader-search.js

@@ -10,7 +10,7 @@ const {
   defaultResolve: resolve
 } = require('internal/modules/esm/resolve');
 
-assert.rejects(
+assert.throws(
   resolve('target'),
   {
     code: 'ERR_MODULE_NOT_FOUND',

test/es-module/test-esm-resolve-type.js

@@ -41,10 +41,10 @@ try {
     [ '/es-modules/package-type-commonjs/index.js', 'commonjs' ],
     [ '/es-modules/package-without-type/index.js', 'commonjs' ],
     [ '/es-modules/package-without-pjson/index.js', 'commonjs' ],
-  ].forEach(async (testVariant) => {
+  ].forEach((testVariant) => {
     const [ testScript, expectedType ] = testVariant;
     const resolvedPath = path.resolve(fixtures.path(testScript));
-    const resolveResult = await resolve(url.pathToFileURL(resolvedPath));
+    const resolveResult = resolve(url.pathToFileURL(resolvedPath));
     assert.strictEqual(resolveResult.format, expectedType);
   });
 
@@ -59,7 +59,7 @@ try {
     [ 'test-module-mainmjs', 'mjs', 'module', 'module'],
     [ 'test-module-cjs', 'js', 'commonjs', 'commonjs'],
     [ 'test-module-ne', 'js', undefined, 'commonjs'],
-  ].forEach(async (testVariant) => {
+  ].forEach((testVariant) => {
     const [ moduleName,
             moduleExtenstion,
             moduleType,
@@ -89,7 +89,7 @@ try {
     fs.writeFileSync(script,
                      'export function esm-resolve-tester() {return 42}');
 
-    const resolveResult = await resolve(`${moduleName}`);
+    const resolveResult = resolve(`${moduleName}`);
     assert.strictEqual(resolveResult.format, expectedResolvedType);
 
     fs.rmSync(nmDir, { recursive: true, force: true });
@@ -102,7 +102,7 @@ try {
     }
   };
 
-  async function testDualPackageWithJsMainScriptAndModuleType() {
+  function testDualPackageWithJsMainScriptAndModuleType() {
     // Create a dummy dual package
     //
     /**
@@ -172,7 +172,7 @@ try {
     );
 
     // test the resolve
-    const resolveResult = await resolve(`${moduleName}`);
+    const resolveResult = resolve(`${moduleName}`);
     assert.strictEqual(resolveResult.format, 'module');
     assert.ok(resolveResult.url.includes('my-dual-package/es/index.js'));
   }
@@ -192,7 +192,7 @@ try {
     [ 'hmod', 'index.js', 'imp.js', 'commonjs', 'module', 'module', '#Key'],
     [ 'qhmod', 'index.js', 'imp.js', 'commonjs', 'module', 'module', '?k=v#h'],
     [ 'ts-mod-com', 'index.js', 'imp.ts', 'module', 'commonjs', undefined],
-  ].forEach(async (testVariant) => {
+  ].forEach((testVariant) => {
     const [
       moduleName,
       mainRequireScript,
@@ -240,7 +240,7 @@ try {
     );
 
     // test the resolve
-    const resolveResult = await resolve(`${moduleName}`);
+    const resolveResult = resolve(`${moduleName}`);
     assert.strictEqual(resolveResult.format, expectedResolvedFormat);
     assert.ok(resolveResult.url.endsWith(`${moduleName}/subdir/${mainImportScript}${mainSuffix}`));
   });

test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs

@@ -13,8 +13,8 @@ export function globalPreload() {
 `;
 }
 
-export async function resolve(specifier, context, next) {
-  const def = await next(specifier, context);
+export function resolve(specifier, context, next) {
+  const def = next(specifier, context);
 
   if (def.url.startsWith('node:')) {
     return {

test/fixtures/es-module-loaders/hook-resolve-type.mjs

@@ -6,8 +6,8 @@ export async function load(url, context, next) {
   return next(url, context, next);
 }
 
-export async function resolve(specifier, context, next) {
-  const nextResult = await next(specifier, context);
+export function resolve(specifier, context, next) {
+  const nextResult = next(specifier, context);
   const { format } = nextResult;
 
   if (format === 'module' || specifier.endsWith('.mjs')) {

test/fixtures/es-module-loaders/loader-invalid-format.mjs

@@ -1,4 +1,4 @@
-export async function resolve(specifier, { parentURL, importAssertions }, defaultResolve) {
+export function resolve(specifier, { parentURL, importAssertions }, defaultResolve) {
   if (parentURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') {
     return {
       shortCircuit: true,

test/fixtures/es-module-loaders/loader-invalid-url.mjs

@@ -1,10 +1,10 @@
-export async function resolve(specifier, { parentURL, importAssertions }, defaultResolve) {
+export function resolve(specifier, { parentURL, importAssertions }, nextResolve) {
   if (parentURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') {
     return {
       shortCircuit: true,
       url: specifier,
       importAssertions,
     };
   }
-  return defaultResolve(specifier, {parentURL, importAssertions}, defaultResolve);
+  return nextResolve(specifier, {parentURL, importAssertions});
 }

test/fixtures/es-module-loaders/loader-resolve-42.mjs

@@ -1,4 +1,4 @@
-export async function resolve(specifier, context, next) {
+export function resolve(specifier, context, next) {
   console.log('resolve 42'); // This log is deliberate
   console.log('next<HookName>:', next.name); // This log is deliberate
 

test/fixtures/es-module-loaders/loader-resolve-async-fn.mjs

@@ -0,0 +1,3 @@
+export async function resolve() {
+	return 'whatever';
+}

test/fixtures/es-module-loaders/loader-resolve-bad-next-context.mjs

@@ -1,3 +1,3 @@
-export async function resolve(specifier, context, next) {
+export function resolve(specifier, context, next) {
   return next(specifier, []);
 }

test/fixtures/es-module-loaders/loader-resolve-bad-next-specifier.mjs

@@ -1,3 +1,3 @@
-export async function resolve(specifier, context, next) {
+export function resolve(specifier, context, next) {
   return next([], context);
 }

test/fixtures/es-module-loaders/loader-resolve-foo.mjs

@@ -1,4 +1,4 @@
-export async function resolve(specifier, context, next) {
+export function resolve(specifier, context, next) {
   console.log('resolve foo'); // This log is deliberate
   return next('file:///foo.mjs', context);
 }

test/fixtures/es-module-loaders/loader-resolve-incomplete.mjs

@@ -1,4 +1,4 @@
-export async function resolve() {
+export function resolve() {
   return {
     url: 'file:///incomplete-resolve-chain.js',
   };

test/fixtures/es-module-loaders/loader-resolve-multiple-next-calls.mjs

@@ -1,6 +1,6 @@
-export async function resolve(specifier, context, next) {
-  const { url: first } = await next(specifier, context);
-  const { url: second } = await next(specifier, context);
+export function resolve(specifier, context, next) {
+  const { url: first } = next(specifier, context);
+  const { url: second } = next(specifier, context);
 
   return {
     format: 'module',

test/fixtures/es-module-loaders/loader-resolve-next-modified.mjs

@@ -1,8 +1,8 @@
-export async function resolve(url, context, next) {
+export function resolve(url, context, next) {
   const {
     format,
     url: nextUrl,
-  } = await next(url, context);
+  } = next(url, context);
 
   return {
     format,

test/fixtures/es-module-loaders/loader-resolve-passing-modified-context.mjs

@@ -1,4 +1,4 @@
-export async function resolve(specifier, context, next) {
+export function resolve(specifier, context, next) {
   return next(specifier, {
     ...context,
     foo: 'bar',

test/fixtures/es-module-loaders/loader-resolve-passthru.mjs

@@ -1,4 +1,4 @@
-export async function resolve(specifier, context, next) {
+export function resolve(specifier, context, next) {
   console.log('resolve passthru'); // This log is deliberate
   return next(specifier, context);
 }

test/fixtures/es-module-loaders/loader-resolve-receiving-modified-context.mjs

@@ -1,4 +1,4 @@
-export async function resolve(specifier, context, next) {
+export function resolve(specifier, context, next) {
   console.log(context.foo); // This log is deliberate
   return next(specifier, context);
 }