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

packages/node-resolve/src/fs.js

@@ -15,3 +15,7 @@ export async function exists(filePath) {
     return false;
   }
 }
+
+export async function resolveSymlink(path) {
+  return (await exists(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

@@ -65,8 +65,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

@@ -5,7 +5,7 @@ import { fileURLToPath, pathToFileURL } from 'url';
 import resolve from 'resolve';
 
 import { getPackageInfo, getPackageName } from './util';
-import { exists, realpath } from './fs';
+import { resolveSymlink } from './fs';
 import { isDirCached, isFileCached, readCachedFile } from './cache';
 import resolvePackageExports from './package/resolvePackageExports';
 import resolvePackageImports from './package/resolvePackageImports';
@@ -32,11 +32,8 @@ async function getPackageJson(importer, pkgName, resolveOptions, moduleDirectori
   }
 }
 
-async function resolveId({
-  importer,
+async function resolveIdClassic({
   importSpecifier,
-  exportConditions,
-  warn,
   packageInfoCache,
   extensions,
   mainFields,
@@ -83,21 +80,49 @@ 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,
+  warn,
+  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,
@@ -108,9 +133,56 @@ 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) {
@@ -134,46 +206,30 @@ async function resolveId({
           subpath,
           pkgJson.exports
         );
-        location = fileURLToPath(resolvedPackageExport);
+        const location = fileURLToPath(resolvedPackageExport);
+        if (location) {
+          return {
+            location: preserveSymlinks ? location : await resolveSymlink(location),
+            hasModuleSideEffects,
+            hasPackageEntry,
+            packageBrowserField,
+            packageInfo
+          };
+        }
       } catch (error) {
         if (error instanceof ResolveError) {
-          return error;
+          warn(error);
+          return null;
         }
         throw error;
       }
     }
   }
 
-  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;
-      }
-      return null;
-    }
-  }
-
-  if (!preserveSymlinks) {
-    if (await exists(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,
@@ -188,11 +244,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,
@@ -208,16 +262,62 @@ 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;
 }
+
+// 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({
+  importer,
+  importSpecifierList,
+  exportConditions,
+  warn,
+  packageInfoCache,
+  extensions,
+  mainFields,
+  preserveSymlinks,
+  useBrowserOverrides,
+  baseDir,
+  moduleDirectories,
+  rootDir,
+  ignoreSideEffectsForRoot
+}) {
+  const exportMapRes = await resolveWithExportMap({
+    importer,
+    importSpecifier: importSpecifierList[0],
+    exportConditions,
+    warn,
+    packageInfoCache,
+    extensions,
+    mainFields,
+    preserveSymlinks,
+    useBrowserOverrides,
+    baseDir,
+    moduleDirectories,
+    rootDir,
+    ignoreSideEffectsForRoot
+  });
+  if (exportMapRes) return exportMapRes;
+
+  // 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
+  });
+}

packages/node-resolve/test/fixtures/ts-import-js-extension-for-js-file-export-map/import-js-with-js-extension.ts

@@ -0,0 +1,3 @@
+import { main } from "dependency/dist/something.js";
+
+export default main();

packages/node-resolve/test/package-entry-points.js

@@ -352,7 +352,7 @@ test('can override a star pattern using null', async (t) => {
 
 test('can self-import a package when using exports field', async (t) => {
   const bundle = await rollup({
-    input: 'self-package-import',
+    input: './self-package-import',
     onwarn: () => {
       t.fail('No warnings were expected');
     },