Revert "esm: convert resolve hook to synchronous"

This reverts commit 90b634a.
PR-URL: #43526
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Author: JakobJingleheimer

doc/api/esm.md

@@ -324,9 +324,6 @@ added:
  - v13.9.0
  - v12.16.2
 changes:
- - version: REPLACEME
- pr-url: https://github.com/nodejs/node/pull/43363
- description: Convert from asynchronous to synchronous.
  - version:
  - v16.2.0
  - v14.18.0
@@ -342,19 +339,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: {string}
+* Returns: {Promise}
 
 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`.
+the URL string.
 
 <!-- eslint-skip -->
 
 ```js
-constdependencyAsset=import.meta.resolve('component-lib/asset.css');
+constdependencyAsset=awaitimport.meta.resolve('component-lib/asset.css');
 ```
 
 `import.meta.resolve` also accepts a second argument which is the parent module
@@ -363,11 +356,11 @@ from which to resolve from:
 <!-- eslint-skip -->
 
 ```js
-import.meta.resolve('./dep', import.meta.url);
+awaitimport.meta.resolve('./dep', import.meta.url);
 ```
 
-This function is synchronous because the ES module resolver in Node.js is
-synchronous.
+This function is asynchronous because the ES module resolver in Node.js is
+allowed to be asynchronous.
 
 ## Interoperability with CommonJS
 
@@ -738,9 +731,6 @@ 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 synchronous.
  - version: REPLACEME
  pr-url: https://github.com/nodejs/node/pull/42623
  description: Add support for chaining resolve hooks. Each hook must either
@@ -774,9 +764,6 @@ changes:
  terminate the chain of `resolve` hooks. **Default:** `false`
  * `url` {string} The absolute URL to which this input resolves
 
-> **Caveat** A resolve hook can contain synchronous file-system operations
-> (as `defaultResolveHook()` does), which can impact performance.
-
 The `resolve` hook chain is responsible for resolving file URL for a given
 module specifier and parent URL, and optionally its format (such as `'module'`)
 as a hint to the `load` hook. If a format is specified, the `load` hook is
@@ -803,7 +790,7 @@ Node.js module specifier resolution behavior_ when calling `defaultResolve`, the
 `context.conditions` array originally passed into the `resolve` hook.
 
 ```js
-exportfunctionresolve(specifier, context, nextResolve) {
+exportasyncfunctionresolve(specifier, context, nextResolve) {
 const { parentURL=null } = context;
 
 if (Math.random() >0.5) { // Some condition.
@@ -1102,7 +1089,7 @@ const baseURL = pathToFileURL(`${cwd()}/`).href;
 // CoffeeScript files end in .coffee, .litcoffee, or .coffee.md.
 const extensionsRegex = /\.coffee$|\.litcoffee$|\.coffee\.md$/;
 
-export function resolve(specifier, context, nextResolve) {
+export async function resolve(specifier, context, nextResolve) {
  if (extensionsRegex.test(specifier)) {
  const { parentURL = baseURL } = context;
 

lib/internal/modules/esm/initialize_import_meta.js

@@ -3,23 +3,21 @@
 const{ getOptionValue }=require('internal/options');
 constexperimentalImportMetaResolve=
 getOptionValue('--experimental-import-meta-resolve');
+const{
+ PromisePrototypeThen,
+ PromiseReject,
+}=primordials;
 constasyncESM=require('internal/process/esm_loader');
 
 functioncreateImportMetaResolve(defaultParentUrl){
-returnfunctionresolve(specifier,parentUrl=defaultParentUrl){
-leturl;
-
-try{
-({ url }=asyncESM.esmLoader.resolve(specifier,parentUrl));
-}catch(error){
-if(error.code==='ERR_UNSUPPORTED_DIR_IMPORT'){
-({ url }=error);
-}else{
-throwerror;
-}
-}
-
-returnurl;
+returnasyncfunctionresolve(specifier,parentUrl=defaultParentUrl){
+returnPromisePrototypeThen(
+asyncESM.esmLoader.resolve(specifier,parentUrl),
+({ url })=>url,
+(error)=>(
+error.code==='ERR_UNSUPPORTED_DIR_IMPORT' ?
+error.url : PromiseReject(error))
+);
 };
 }
 

lib/internal/modules/esm/loader.js

@@ -15,8 +15,6 @@ const {
  ObjectDefineProperty,
  ObjectSetPrototypeOf,
  PromiseAll,
- PromiseResolve,
- PromisePrototypeThen,
  ReflectApply,
  RegExpPrototypeExec,
  SafeArrayIterator,
@@ -111,14 +109,12 @@ 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 {object} validators A wrapper function
+ * @param {(hookErrIdentifier, hookArgs) => void} validate A wrapper function
  * containing all validation of a custom loader hook's intermediary output. Any
  * validation within MUST throw.
- * @param {(hookErrIdentifier, hookArgs) => void} validators.validateArgs
- * @param {(hookErrIdentifier, output) => void} validators.validateOutput
  * @returns {function next<HookName>(...hookArgs)} The next hook in the chain.
  */
-functionnextHookFactory(chain,meta,{ validateArgs, validateOutput }){
+functionnextHookFactory(chain,meta,validate){
 // First, prepare the current
 const{ hookName }=meta;
 const{
@@ -141,7 +137,7 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
 // factory generates the next link in the chain.
 meta.hookIndex--;
 
-nextNextHook=nextHookFactory(chain,meta,{ validateArgs, validateOutput });
+nextNextHook=nextHookFactory(chain,meta,validate);
 }else{
 // eslint-disable-next-line func-name-matching
 nextNextHook=functionchainAdvancedTooFar(){
@@ -152,36 +148,21 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
 }
 
 returnObjectDefineProperty(
-(...args)=>{
+async(...args)=>{
 // Update only when hook is invoked to avoid fingering the wrong filePath
 meta.hookErrIdentifier=`${hookFilePath} '${hookName}'`;
 
-validateArgs(`${meta.hookErrIdentifier} hook's ${nextHookName}()`,args);
-
-constoutputErrIdentifier=`${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`;
+validate(`${meta.hookErrIdentifier} hook's ${nextHookName}()`,args);
 
 // Set when next<HookName> is actually called, not just generated.
 if(generatedHookIndex===0){meta.chainFinished=true;}
 
 ArrayPrototypePush(args,nextNextHook);
-constoutput=ReflectApply(hook,undefined,args);
-
-validateOutput(outputErrIdentifier,output);
+constoutput=awaitReflectApply(hook,undefined,args);
 
-functioncheckShortCircuited(output){
-if(output?.shortCircuit===true){meta.shortCircuited=true;}
-
-returnoutput;
-}
-
-if(meta.isChainAsync){
-returnPromisePrototypeThen(
-PromiseResolve(output),
-checkShortCircuited,
-);
-}
+if(output?.shortCircuit===true){meta.shortCircuited=true;}
+returnoutput;
 
-returncheckShortCircuited(output);
 },
 'name',
 {__proto__: null,value: nextHookName},
@@ -440,11 +421,8 @@ class ESMLoader {
 );
 }
 
-const{ format, url }=this.resolve(
-specifier,
-parentURL,
-importAssertionsForResolve,
-);
+const{ format, url }=
+awaitthis.resolve(specifier,parentURL,importAssertionsForResolve);
 
 letjob=this.moduleMap.get(url,importAssertions.type);
 
@@ -579,11 +557,10 @@ class ESMLoader {
 hookErrIdentifier: '',
 hookIndex: chain.length-1,
 hookName: 'load',
-isChainAsync: true,
 shortCircuited: false,
 };
 
-constvalidateArgs=(hookErrIdentifier,{0: nextUrl,1: ctx})=>{
+constvalidate=(hookErrIdentifier,{0: nextUrl,1: ctx})=>{
 if(typeofnextUrl!=='string'){
 // non-strings can be coerced to a url string
 // validateString() throws a less-specific error
@@ -609,22 +586,19 @@ class ESMLoader {
 
 validateObject(ctx,`${hookErrIdentifier} context`);
 };
-constvalidateOutput=(hookErrIdentifier,output)=>{
-if(typeofoutput!=='object'){// [2]
-thrownewERR_INVALID_RETURN_VALUE(
-'an object',
-hookErrIdentifier,
-output,
-);
-}
-};
 
-constnextLoad=nextHookFactory(chain,meta,{ validateArgs, validateOutput });
+constnextLoad=nextHookFactory(chain,meta,validate);
 
 constloaded=awaitnextLoad(url,context);
 const{ hookErrIdentifier }=meta;// Retrieve the value after all settled
 
-validateOutput(hookErrIdentifier,loaded);
+if(typeofloaded!=='object'){// [2]
+thrownewERR_INVALID_RETURN_VALUE(
+'an object',
+hookErrIdentifier,
+loaded,
+);
+}
 
 if(loaded?.shortCircuit===true){meta.shortCircuited=true;}
 
@@ -804,7 +778,7 @@ class ESMLoader {
  * statement or expression.
  * @returns {{ format: string, url: URL['href'] }}
  */
-resolve(
+asyncresolve(
 originalSpecifier,
 parentURL,
 importAssertions=ObjectCreate(null)
@@ -828,43 +802,36 @@ class ESMLoader {
 hookErrIdentifier: '',
 hookIndex: chain.length-1,
 hookName: 'resolve',
-isChainAsync: false,
 shortCircuited: false,
 };
+
 constcontext={
 conditions: DEFAULT_CONDITIONS,
  importAssertions,
  parentURL,
 };
+constvalidate=(hookErrIdentifier,{0: suppliedSpecifier,1: ctx})=>{
 
-constvalidateArgs=(hookErrIdentifier,{0: suppliedSpecifier,1: ctx})=>{
 validateString(
 suppliedSpecifier,
 `${hookErrIdentifier} specifier`,
 );// non-strings can be coerced to a url string
 
 validateObject(ctx,`${hookErrIdentifier} context`);
 };
-constvalidateOutput=(hookErrIdentifier,output)=>{
-if(
-typeofoutput!=='object'||// [2]
-output===null||
-(output.url==null&&typeofoutput.then==='function')
-){
-thrownewERR_INVALID_RETURN_VALUE(
-'an object',
-hookErrIdentifier,
-output,
-);
-}
-};
 
-constnextResolve=nextHookFactory(chain,meta,{ validateArgs, validateOutput });
+constnextResolve=nextHookFactory(chain,meta,validate);
 
-constresolution=nextResolve(originalSpecifier,context);
+constresolution=awaitnextResolve(originalSpecifier,context);
 const{ hookErrIdentifier }=meta;// Retrieve the value after all settled
 
-validateOutput(hookErrIdentifier,resolution);
+if(typeofresolution!=='object'){// [2]
+thrownewERR_INVALID_RETURN_VALUE(
+'an object',
+hookErrIdentifier,
+resolution,
+);
+}
 
 if(resolution?.shortCircuit===true){meta.shortCircuited=true;}
 

lib/internal/modules/esm/resolve.js

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