loader: return package format from defaultResolve if known

This is a proposed modification of defaultResolve to return the package format in case it has been found during package resolution. The format will be returned as described in the documentation: https://nodejs.org/api/esm.html#resolvespecifier-context-defaultresolve There is one new unit test as well: test/es-module/test-esm-resolve-type.js PR-URL: #40980 Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Dec 13, 2021 · 85d4cd3 · 85d4cd3
1 parent e5c1fd7
commit 85d4cd3
Show file tree

Hide file tree

Showing 8 changed files with 372 additions and 50 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -457,7 +457,7 @@ function trySelf(parentPath, request) {
   try {
     return finalizeEsmResolution(packageExportsResolve(
       pathToFileURL(pkgPath + '/package.json'), expansion, pkg,
-      pathToFileURL(parentPath), cjsConditions), parentPath, pkgPath);
+      pathToFileURL(parentPath), cjsConditions).resolved, parentPath, pkgPath);
   } catch (e) {
     if (e.code === 'ERR_MODULE_NOT_FOUND')
       throw createEsmNotFoundErr(request, pkgPath + '/package.json');
@@ -481,7 +481,7 @@ function resolveExports(nmPath, request) {
     try {
       return finalizeEsmResolution(packageExportsResolve(
         pathToFileURL(pkgPath + '/package.json'), '.' + expansion, pkg, null,
-        cjsConditions), null, pkgPath);
+        cjsConditions).resolved, null, pkgPath);
     } catch (e) {
       if (e.code === 'ERR_MODULE_NOT_FOUND')
         throw createEsmNotFoundErr(request, pkgPath + '/package.json');

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -463,6 +463,23 @@ const patternRegEx = /\*/g;
 
 function resolvePackageTargetString(
   target, subpath, match, packageJSONUrl, base, pattern, internal, conditions) {
+
+  const composeResult = (resolved) => {
+    let format;
+    try {
+      format = getPackageType(resolved);
+    } catch (err) {
+      if (err.code === 'ERR_INVALID_FILE_URL_PATH') {
+        const invalidModuleErr = new ERR_INVALID_MODULE_SPECIFIER(
+          resolved, 'must not include encoded "/" or "\\" characters', base);
+        invalidModuleErr.cause = err;
+        throw invalidModuleErr;
+      }
+      throw err;
+    }
+    return { resolved, ...(format !== 'none') && { format } };
+  };
+
   if (subpath !== '' && !pattern && target[target.length - 1] !== '/')
     throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
 
@@ -478,7 +495,8 @@ function resolvePackageTargetString(
         const exportTarget = pattern ?
           RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) :
           target + subpath;
-        return packageResolve(exportTarget, packageJSONUrl, conditions);
+        return packageResolve(
+          exportTarget, packageJSONUrl, conditions);
       }
     }
     throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
@@ -494,18 +512,21 @@ function resolvePackageTargetString(
   if (!StringPrototypeStartsWith(resolvedPath, packagePath))
     throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
 
-  if (subpath === '') return resolved;
+  if (subpath === '') return composeResult(resolved);
 
   if (RegExpPrototypeTest(invalidSegmentRegEx, subpath)) {
     const request = pattern ?
       StringPrototypeReplace(match, '*', () => subpath) : match + subpath;
     throwInvalidSubpath(request, packageJSONUrl, internal, base);
   }
 
-  if (pattern)
-    return new URL(RegExpPrototypeSymbolReplace(patternRegEx, resolved.href,
-                                                () => subpath));
-  return new URL(subpath, resolved);
+  if (pattern) {
+    return composeResult(new URL(RegExpPrototypeSymbolReplace(patternRegEx,
+                                                              resolved.href,
+                                                              () => subpath)));
+  }
+
+  return composeResult(new URL(subpath, resolved));
 }
 
 /**
@@ -531,9 +552,9 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
     let lastException;
     for (let i = 0; i < target.length; i++) {
       const targetItem = target[i];
-      let resolved;
+      let resolveResult;
       try {
-        resolved = resolvePackageTarget(
+        resolveResult = resolvePackageTarget(
           packageJSONUrl, targetItem, subpath, packageSubpath, base, pattern,
           internal, conditions);
       } catch (e) {
@@ -542,13 +563,13 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
           continue;
         throw e;
       }
-      if (resolved === undefined)
+      if (resolveResult === undefined)
         continue;
-      if (resolved === null) {
+      if (resolveResult === null) {
         lastException = null;
         continue;
       }
-      return resolved;
+      return resolveResult;
     }
     if (lastException === undefined || lastException === null)
       return lastException;
@@ -567,12 +588,12 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
       const key = keys[i];
       if (key === 'default' || conditions.has(key)) {
         const conditionalTarget = target[key];
-        const resolved = resolvePackageTarget(
+        const resolveResult = resolvePackageTarget(
           packageJSONUrl, conditionalTarget, subpath, packageSubpath, base,
           pattern, internal, conditions);
-        if (resolved === undefined)
+        if (resolveResult === undefined)
           continue;
-        return resolved;
+        return resolveResult;
       }
     }
     return undefined;
@@ -631,12 +652,15 @@ function packageExportsResolve(
       !StringPrototypeIncludes(packageSubpath, '*') &&
       !StringPrototypeEndsWith(packageSubpath, '/')) {
     const target = exports[packageSubpath];
-    const resolved = resolvePackageTarget(
+    const resolveResult = resolvePackageTarget(
       packageJSONUrl, target, '', packageSubpath, base, false, false, conditions
     );
-    if (resolved === null || resolved === undefined)
+
+    if (resolveResult == null) {
       throwExportsNotFound(packageSubpath, packageJSONUrl, base);
-    return resolved;
+    }
+
+    return resolveResult;
   }
 
   let bestMatch = '';
@@ -672,12 +696,20 @@ function packageExportsResolve(
 
   if (bestMatch) {
     const target = exports[bestMatch];
-    const resolved = resolvePackageTarget(packageJSONUrl, target,
-                                          bestMatchSubpath, bestMatch, base,
-                                          true, false, conditions);
-    if (resolved === null || resolved === undefined)
+    const resolveResult = resolvePackageTarget(
+      packageJSONUrl,
+      target,
+      bestMatchSubpath,
+      bestMatch,
+      base,
+      true,
+      false,
+      conditions);
+
+    if (resolveResult == null) {
       throwExportsNotFound(packageSubpath, packageJSONUrl, base);
-    return resolved;
+    }
+    return resolveResult;
   }
 
   throwExportsNotFound(packageSubpath, packageJSONUrl, base);
@@ -717,11 +749,12 @@ function packageImportsResolve(name, base, conditions) {
     if (imports) {
       if (ObjectPrototypeHasOwnProperty(imports, name) &&
           !StringPrototypeIncludes(name, '*')) {
-        const resolved = resolvePackageTarget(
+        const resolveResult = resolvePackageTarget(
           packageJSONUrl, imports[name], '', name, base, false, true, conditions
         );
-        if (resolved !== null && resolved !== undefined)
-          return resolved;
+        if (resolveResult != null) {
+          return resolveResult.resolved;
+        }
       } else {
         let bestMatch = '';
         let bestMatchSubpath;
@@ -747,11 +780,13 @@ function packageImportsResolve(name, base, conditions) {
 
         if (bestMatch) {
           const target = imports[bestMatch];
-          const resolved = resolvePackageTarget(packageJSONUrl, target,
-                                                bestMatchSubpath, bestMatch,
-                                                base, true, true, conditions);
-          if (resolved !== null && resolved !== undefined)
-            return resolved;
+          const resolveResult = resolvePackageTarget(packageJSONUrl, target,
+                                                     bestMatchSubpath,
+                                                     bestMatch, base, true,
+                                                     true, conditions);
+          if (resolveResult != null) {
+            return resolveResult.resolved;
+          }
         }
       }
     }
@@ -810,11 +845,11 @@ function parsePackageName(specifier, base) {
  * @param {string} specifier
  * @param {string | URL | undefined} base
  * @param {Set<string>} conditions
- * @returns {URL}
+ * @returns {resolved: URL, format? : string}
  */
 function packageResolve(specifier, base, conditions) {
   if (NativeModule.canBeRequiredByUsers(specifier))
-    return new URL('node:' + specifier);
+    return { resolved: new URL('node:' + specifier) };
 
   const { packageName, packageSubpath, isScoped } =
     parsePackageName(specifier, base);
@@ -826,8 +861,7 @@ function packageResolve(specifier, base, conditions) {
     if (packageConfig.name === packageName &&
         packageConfig.exports !== undefined && packageConfig.exports !== null) {
       return packageExportsResolve(
-        packageJSONUrl, packageSubpath, packageConfig, base, conditions
-      );
+        packageJSONUrl, packageSubpath, packageConfig, base, conditions);
     }
   }
 
@@ -849,13 +883,24 @@ function packageResolve(specifier, base, conditions) {
 
     // Package match.
     const packageConfig = getPackageConfig(packageJSONPath, specifier, base);
-    if (packageConfig.exports !== undefined && packageConfig.exports !== null)
+    if (packageConfig.exports !== undefined && packageConfig.exports !== null) {
       return packageExportsResolve(
-        packageJSONUrl, packageSubpath, packageConfig, base, conditions
-      );
-    if (packageSubpath === '.')
-      return legacyMainResolve(packageJSONUrl, packageConfig, base);
-    return new URL(packageSubpath, packageJSONUrl);
+        packageJSONUrl, packageSubpath, packageConfig, base, conditions);
+    }
+    if (packageSubpath === '.') {
+      return {
+        resolved: legacyMainResolve(
+          packageJSONUrl,
+          packageConfig,
+          base),
+        ...(packageConfig.type !== 'none') && { format: packageConfig.type }
+      };
+    }
+
+    return {
+      resolved: new URL(packageSubpath, packageJSONUrl),
+      ...(packageConfig.type !== 'none') && { format: packageConfig.type }
+    };
     // Cross-platform root check.
   } while (packageJSONPath.length !== lastPath.length);
 
@@ -893,12 +938,13 @@ function shouldBeTreatedAsRelativeOrAbsolutePath(specifier) {
  * @param {string | URL | undefined} base
  * @param {Set<string>} conditions
  * @param {boolean} preserveSymlinks
- * @returns {URL}
+ * @returns {url: URL, format?: string}
  */
 function moduleResolve(specifier, base, conditions, preserveSymlinks) {
   // Order swapped from spec for minor perf gain.
   // Ok since relative URLs cannot parse as URLs.
   let resolved;
+  let format;
   if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
     resolved = new URL(specifier, base);
   } else if (specifier[0] === '#') {
@@ -907,12 +953,19 @@ function moduleResolve(specifier, base, conditions, preserveSymlinks) {
     try {
       resolved = new URL(specifier);
     } catch {
-      resolved = packageResolve(specifier, base, conditions);
+      ({ resolved, format } = packageResolve(specifier, base, conditions));
     }
   }
-  if (resolved.protocol !== 'file:')
-    return resolved;
-  return finalizeResolution(resolved, base, preserveSymlinks);
+  if (resolved.protocol !== 'file:') {
+    return {
+      url: resolved
+    };
+  }
+
+  return {
+    url: finalizeResolution(resolved, base, preserveSymlinks),
+    ...(format != null) && { format }
+  };
 }
 
 /**
@@ -1001,9 +1054,15 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {
 
   conditions = getConditionsSet(conditions);
   let url;
+  let format;
   try {
-    url = moduleResolve(specifier, parentURL, conditions,
-                        isMain ? preserveSymlinksMain : preserveSymlinks);
+    ({ url, format } =
+      moduleResolve(
+        specifier,
+        parentURL,
+        conditions,
+        isMain ? preserveSymlinksMain : preserveSymlinks
+      ));
   } catch (error) {
     // Try to give the user a hint of what would have been the
     // resolved CommonJS module
@@ -1031,7 +1090,10 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {
       url.protocol !== 'node:')
     throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(url);
 
-  return { url: `${url}` };
+  return {
+    url: `${url}`,
+    ...(format != null) && { format }
+  };
 }
 
 module.exports = {

diff --git a/test/es-module/test-esm-loader-resolve-type.mjs b/test/es-module/test-esm-loader-resolve-type.mjs
@@ -0,0 +1,41 @@
+// Flags: --loader ./test/fixtures/es-module-loaders/hook-resolve-type.mjs
+import { allowGlobals } from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import { strict as assert } from 'assert';
+import * as fs from 'fs';
+
+allowGlobals(global.getModuleTypeStats);
+
+const basePath =
+  new URL('./node_modules/', import.meta.url);
+
+const rel = (file) => new URL(file, basePath);
+const createDir = (path) => {
+  if (!fs.existsSync(path)) {
+    fs.mkdirSync(path);
+  }
+};
+
+const moduleName = 'module-counter-by-type';
+
+const moduleDir = rel(`${moduleName}`);
+createDir(basePath);
+createDir(moduleDir);
+fs.cpSync(
+  fixtures.path('es-modules', moduleName),
+  moduleDir,
+  { recursive: true }
+);
+
+const { importedESM: importedESMBefore,
+        importedCJS: importedCJSBefore } = global.getModuleTypeStats();
+
+import(`${moduleName}`).finally(() => {
+  fs.rmSync(basePath, { recursive: true, force: true });
+});
+
+const { importedESM: importedESMAfter,
+        importedCJS: importedCJSAfter } = global.getModuleTypeStats();
+
+assert.strictEqual(importedESMBefore + 1, importedESMAfter);
+assert.strictEqual(importedCJSBefore, importedCJSAfter);