fix(node-resolve): Fix bug where JS import was converted to a TS import, resulting in an error when using export maps (#921)

Author: tjenkinson

packages/node-resolve/src/fs.js

@@ -16,3 +16,7 @@ export async function fileExists(filePath) {
     return false;
   }
 }
+
+export async function resolveSymlink(path) {
+  return (await fileExists(path)) ? realpath(path) : path;
+}

packages/node-resolve/src/index.js

@@ -121,30 +121,17 @@ export function nodeResolve(opts = {}) {
       return false;
     }
 
-    const importSpecifierList = [];
+    const importSpecifierList = [importee];
 
     if (importer === undefined && !importee[0].match(/^\.?\.?\//)) {
       // For module graph roots (i.e. when importer is undefined), we
       // need to handle 'path fragments` like `foo/bar` that are commonly
       // found in rollup config files. If importee doesn't look like a
       // relative or absolute path, we make it relative and attempt to
-      // resolve it. If we don't find anything, we try resolving it as we
-      // got it.
+      // resolve it.
       importSpecifierList.push(`./${importee}`);
     }
 
-    const importeeIsBuiltin = builtins.has(importee);
-
-    if (importeeIsBuiltin) {
-      // The `resolve` library will not resolve packages with the same
-      // name as a node built-in module. If we're resolving something
-      // that's a builtin, and we don't prefer to find built-ins, we
-      // first try to look up a local module with that name. If we don't
-      // find anything, we resolve the builtin which just returns back
-      // the built-in's name.
-      importSpecifierList.push(`${importee}/`);
-    }
-
     // TypeScript files may import '.js' to refer to either '.ts' or '.tsx'
     if (importer && importee.endsWith('.js')) {
       for (const ext of ['.ts', '.tsx']) {
@@ -154,8 +141,6 @@ export function nodeResolve(opts = {}) {
       }
     }
 
-    importSpecifierList.push(importee);
-
     const warn = (...args) => context.warn(...args);
     const isRequire =
       opts && opts.custom && opts.custom['node-resolve'] && opts.custom['node-resolve'].isRequire;
@@ -180,6 +165,7 @@ export function nodeResolve(opts = {}) {
       ignoreSideEffectsForRoot
     });
 
+    const importeeIsBuiltin = builtins.has(importee);
     const resolved =
       importeeIsBuiltin && preferBuiltins
         ? {

packages/node-resolve/src/package/resolvePackageImports.js

@@ -33,7 +33,7 @@ async function resolvePackageImports({
   }
 
   if (importSpecifier === '#' || importSpecifier.startsWith('#/')) {
-    throw new InvalidModuleSpecifierError(context, 'Invalid import specifier.');
+    throw new InvalidModuleSpecifierError(context, true, 'Invalid import specifier.');
   }
 
   return resolvePackageImportsExports(context, {

packages/node-resolve/src/package/utils.js

@@ -64,8 +64,8 @@ export class InvalidConfigurationError extends ResolveError {
 }
 
 export class InvalidModuleSpecifierError extends ResolveError {
-  constructor(context, internal) {
-    super(createErrorMsg(context, internal));
+  constructor(context, internal, reason) {
+    super(createErrorMsg(context, reason, internal));
   }
 }
 

packages/node-resolve/src/resolveImportSpecifiers.js

@@ -7,7 +7,7 @@ import { dirname } from 'path';
 import resolve from 'resolve';
 
 import { getPackageInfo, getPackageName } from './util';
-import { fileExists, realpath } from './fs';
+import { resolveSymlink } from './fs';
 import { isDirCached, isFileCached, readCachedFile } from './cache';
 import resolvePackageExports from './package/resolvePackageExports';
 import resolvePackageImports from './package/resolvePackageImports';
@@ -34,11 +34,8 @@ async function getPackageJson(importer, pkgName, resolveOptions, moduleDirectori
   }
 }
 
-async function resolveId({
-  importer,
+async function resolveIdClassic({
   importSpecifier,
-  exportConditions,
-  warn,
   packageInfoCache,
   extensions,
   mainFields,
@@ -85,21 +82,48 @@ async function resolveId({
   };
 
   let location;
+  try {
+    location = await resolveImportPath(importSpecifier, resolveOptions);
+  } catch (error) {
+    if (error.code !== 'MODULE_NOT_FOUND') {
+      throw error;
+    }
+    return null;
+  }
 
-  const pkgName = getPackageName(importSpecifier);
+  return {
+    location: preserveSymlinks ? location : await resolveSymlink(location),
+    hasModuleSideEffects,
+    hasPackageEntry,
+    packageBrowserField,
+    packageInfo
+  };
+}
+
+async function resolveWithExportMap({
+  importer,
+  importSpecifier,
+  exportConditions,
+  packageInfoCache,
+  extensions,
+  mainFields,
+  preserveSymlinks,
+  useBrowserOverrides,
+  baseDir,
+  moduleDirectories,
+  rootDir,
+  ignoreSideEffectsForRoot
+}) {
   if (importSpecifier.startsWith('#')) {
     // this is a package internal import, resolve using package imports field
     const resolveResult = await resolvePackageImports({
       importSpecifier,
       importer,
       moduleDirs: moduleDirectories,
       conditions: exportConditions,
-      resolveId(id, parent) {
-        return resolveId({
+      resolveId(id /* , parent*/) {
+        return resolveIdClassic({
           importSpecifier: id,
-          importer: parent,
-          exportConditions,
-          warn,
           packageInfoCache,
           extensions,
           mainFields,
@@ -110,71 +134,91 @@ async function resolveId({
         });
       }
     });
-    location = fileURLToPath(resolveResult);
-  } else if (pkgName) {
+
+    const location = fileURLToPath(resolveResult);
+    return {
+      location: preserveSymlinks ? location : await resolveSymlink(location),
+      hasModuleSideEffects: () => null,
+      hasPackageEntry: true,
+      packageBrowserField: false,
+      // eslint-disable-next-line no-undefined
+      packageInfo: undefined
+    };
+  }
+
+  const pkgName = getPackageName(importSpecifier);
+  if (pkgName) {
     // it's a bare import, find the package.json and resolve using package exports if available
+    let hasModuleSideEffects = () => null;
+    let hasPackageEntry = true;
+    let packageBrowserField = false;
+    let packageInfo;
+
+    const filter = (pkg, pkgPath) => {
+      const info = getPackageInfo({
+        cache: packageInfoCache,
+        extensions,
+        pkg,
+        pkgPath,
+        mainFields,
+        preserveSymlinks,
+        useBrowserOverrides,
+        rootDir,
+        ignoreSideEffectsForRoot
+      });
+
+      ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info);
+
+      return info.cachedPkg;
+    };
+
+    const resolveOptions = {
+      basedir: baseDir,
+      readFile: readCachedFile,
+      isFile: isFileCached,
+      isDirectory: isDirCached,
+      extensions,
+      includeCoreModules: false,
+      moduleDirectory: moduleDirectories,
+      preserveSymlinks,
+      packageFilter: filter
+    };
+
     const result = await getPackageJson(importer, pkgName, resolveOptions, moduleDirectories);
 
     if (result && result.pkgJson.exports) {
-      const { pkgJson, pkgJsonPath, pkgPath } = result;
-      try {
-        const subpath =
-          pkgName === importSpecifier ? '.' : `.${importSpecifier.substring(pkgName.length)}`;
-        const pkgURL = pathToFileURL(`${pkgPath}/`);
-
-        const context = {
-          importer,
-          importSpecifier,
-          moduleDirs: moduleDirectories,
-          pkgURL,
-          pkgJsonPath,
-          conditions: exportConditions
-        };
-        const resolvedPackageExport = await resolvePackageExports(
-          context,
-          subpath,
-          pkgJson.exports
-        );
-        location = fileURLToPath(resolvedPackageExport);
-      } catch (error) {
-        if (error instanceof ResolveError) {
-          return error;
-        }
-        throw error;
-      }
-    }
-  }
+      const { pkgJson, pkgJsonPath } = result;
+      const subpath =
+        pkgName === importSpecifier ? '.' : `.${importSpecifier.substring(pkgName.length)}`;
+      const pkgDr = pkgJsonPath.replace('package.json', '');
+      const pkgURL = pathToFileURL(pkgDr);
 
-  if (!location) {
-    // package has no imports or exports, use classic node resolve
-    try {
-      location = await resolveImportPath(importSpecifier, resolveOptions);
-    } catch (error) {
-      if (error.code !== 'MODULE_NOT_FOUND') {
-        throw error;
+      const context = {
+        importer,
+        importSpecifier,
+        moduleDirs: moduleDirectories,
+        pkgURL,
+        pkgJsonPath,
+        conditions: exportConditions
+      };
+      const resolvedPackageExport = await resolvePackageExports(context, subpath, pkgJson.exports);
+      const location = fileURLToPath(resolvedPackageExport);
+      if (location) {
+        return {
+          location: preserveSymlinks ? location : await resolveSymlink(location),
+          hasModuleSideEffects,
+          hasPackageEntry,
+          packageBrowserField,
+          packageInfo
+        };
       }
-      return null;
     }
   }
 
-  if (!preserveSymlinks) {
-    if (await fileExists(location)) {
-      location = await realpath(location);
-    }
-  }
-
-  return {
-    location,
-    hasModuleSideEffects,
-    hasPackageEntry,
-    packageBrowserField,
-    packageInfo
-  };
+  return null;
 }
 
-// Resolve module specifiers in order. Promise resolves to the first module that resolves
-// successfully, or the error that resulted from the last attempted module resolution.
-export default async function resolveImportSpecifiers({
+async function resolveWithClassic({
   importer,
   importSpecifierList,
   exportConditions,
@@ -189,11 +233,9 @@ export default async function resolveImportSpecifiers({
   rootDir,
   ignoreSideEffectsForRoot
 }) {
-  let lastResolveError;
-
   for (let i = 0; i < importSpecifierList.length; i++) {
     // eslint-disable-next-line no-await-in-loop
-    const result = await resolveId({
+    const result = await resolveIdClassic({
       importer,
       importSpecifier: importSpecifierList[i],
       exportConditions,
@@ -209,16 +251,71 @@ export default async function resolveImportSpecifiers({
       ignoreSideEffectsForRoot
     });
 
-    if (result instanceof ResolveError) {
-      lastResolveError = result;
-    } else if (result) {
+    if (result) {
       return result;
     }
   }
 
-  if (lastResolveError) {
-    // only log the last failed resolve error
-    warn(lastResolveError);
-  }
   return null;
 }
+
+// Resolves to the module if found or `null`.
+// The first import specificer will first be attempted with the exports algorithm.
+// If this is unsuccesful because export maps are not being used, then all of `importSpecifierList`
+// will be tried with the classic resolution algorithm
+export default async function resolveImportSpecifiers({
+  importer,
+  importSpecifierList,
+  exportConditions,
+  warn,
+  packageInfoCache,
+  extensions,
+  mainFields,
+  preserveSymlinks,
+  useBrowserOverrides,
+  baseDir,
+  moduleDirectories,
+  rootDir,
+  ignoreSideEffectsForRoot
+}) {
+  try {
+    const exportMapRes = await resolveWithExportMap({
+      importer,
+      importSpecifier: importSpecifierList[0],
+      exportConditions,
+      packageInfoCache,
+      extensions,
+      mainFields,
+      preserveSymlinks,
+      useBrowserOverrides,
+      baseDir,
+      moduleDirectories,
+      rootDir,
+      ignoreSideEffectsForRoot
+    });
+    if (exportMapRes) return exportMapRes;
+  } catch (error) {
+    if (error instanceof ResolveError) {
+      warn(error);
+      return null;
+    }
+    throw error;
+  }
+
+  // package has no imports or exports, use classic node resolve
+  return resolveWithClassic({
+    importer,
+    importSpecifierList,
+    exportConditions,
+    warn,
+    packageInfoCache,
+    extensions,
+    mainFields,
+    preserveSymlinks,
+    useBrowserOverrides,
+    baseDir,
+    moduleDirectories,
+    rootDir,
+    ignoreSideEffectsForRoot
+  });
+}