fix(check): attempt to resolve types from pkg before `@types` pkg (#24152)

I've been meaning to fix this for ages, but I finally ran into it here:


https://github.com/dsherret/ts-ast-viewer/actions/runs/9432038675/job/25981325408

We need to resolve the `@types` package as a fallback instead of eagerly
resolving it.

7 files changed, 7 insertions, 96 deletions

diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs
index bfb9bed7f..ec3fb00e9 100644
--- a/cli/npm/byonm.rs
+++ b/cli/npm/byonm.rs
@@ -12,7 +12,6 @@ use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::NodePermissions;
-use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::NpmResolver;
 use deno_runtime::deno_node::PackageJson;
 use deno_semver::package::PackageReq;
@@ -23,7 +22,6 @@ use crate::args::NpmProcessStateKind;
 use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs;
 use deno_runtime::fs_util::specifier_to_file_path;
 
-use super::common::types_package_name;
 use super::CliNpmResolver;
 use super::InnerCliNpmResolverRef;
 
@@ -97,20 +95,13 @@ impl NpmResolver for ByonmCliNpmResolver {
     &self,
     name: &str,
     referrer: &ModuleSpecifier,
-    mode: NodeResolutionMode,
   ) -> Result<PathBuf, AnyError> {
     fn inner(
       fs: &dyn FileSystem,
       name: &str,
       referrer: &ModuleSpecifier,
-      mode: NodeResolutionMode,
     ) -> Result<PathBuf, AnyError> {
       let referrer_file = specifier_to_file_path(referrer)?;
-      let types_pkg_name = if mode.is_types() && !name.starts_with("@types/") {
-        Some(types_package_name(name))
-      } else {
-        None
-      };
       let mut current_folder = referrer_file.parent().unwrap();
       loop {
         let node_modules_folder = if current_folder.ends_with("node_modules") {
@@ -119,14 +110,6 @@ impl NpmResolver for ByonmCliNpmResolver {
           Cow::Owned(current_folder.join("node_modules"))
         };
 
-        // attempt to resolve the types package first, then fallback to the regular package
-        if let Some(types_pkg_name) = &types_pkg_name {
-          let sub_dir = join_package_name(&node_modules_folder, types_pkg_name);
-          if fs.is_dir_sync(&sub_dir) {
-            return Ok(sub_dir);
-          }
-        }
-
         let sub_dir = join_package_name(&node_modules_folder, name);
         if fs.is_dir_sync(&sub_dir) {
           return Ok(sub_dir);
@@ -146,7 +129,7 @@ impl NpmResolver for ByonmCliNpmResolver {
       );
     }
 
-    let path = inner(&*self.fs, name, referrer, mode)?;
+    let path = inner(&*self.fs, name, referrer)?;
     Ok(self.fs.realpath_sync(&path)?)
   }
 
diff --git a/cli/npm/common.rs b/cli/npm/common.rs
index bf45aa5b8..c55f73cd5 100644
--- a/cli/npm/common.rs
+++ b/cli/npm/common.rs
@@ -3,14 +3,6 @@
 use deno_npm::npm_rc::RegistryConfig;
 use reqwest::header;
 
-/// Gets the corresponding @types package for the provided package name.
-pub fn types_package_name(package_name: &str) -> String {
-  debug_assert!(!package_name.starts_with("@types/"));
-  // Scoped packages will get two underscores for each slash
-  // https://github.com/DefinitelyTyped/DefinitelyTyped/tree/15f1ece08f7b498f4b9a2147c2a46e94416ca777#what-about-scoped-packages
-  format!("@types/{}", package_name.replace('/', "__"))
-}
-
 // TODO(bartlomieju): support more auth methods besides token and basic auth
 pub fn maybe_auth_header_for_npm_registry(
   registry_config: &RegistryConfig,
@@ -31,17 +23,3 @@ pub fn maybe_auth_header_for_npm_registry(
 
   None
 }
-
-#[cfg(test)]
-mod test {
-  use super::types_package_name;
-
-  #[test]
-  fn test_types_package_name() {
-    assert_eq!(types_package_name("name"), "@types/name");
-    assert_eq!(
-      types_package_name("@scoped/package"),
-      "@types/@scoped__package"
-    );
-  }
-}
diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
index 2298ea9bd..d3045a1c9 100644
--- a/cli/npm/managed/mod.rs
+++ b/cli/npm/managed/mod.rs
@@ -23,7 +23,6 @@ use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::NodePermissions;
-use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::NpmResolver;
 use deno_semver::package::PackageNv;
 use deno_semver::package::PackageReq;
@@ -531,11 +530,10 @@ impl NpmResolver for ManagedCliNpmResolver {
     &self,
     name: &str,
     referrer: &ModuleSpecifier,
-    mode: NodeResolutionMode,
   ) -> Result<PathBuf, AnyError> {
     let path = self
       .fs_resolver
-      .resolve_package_folder_from_package(name, referrer, mode)?;
+      .resolve_package_folder_from_package(name, referrer)?;
     let path =
       canonicalize_path_maybe_not_exists_with_fs(&path, self.fs.as_ref())?;
     log::debug!("Resolved {} from {} to {}", name, referrer, path.display());
diff --git a/cli/npm/managed/resolvers/common.rs b/cli/npm/managed/resolvers/common.rs
index 767b3f9c6..35f368cb5 100644
--- a/cli/npm/managed/resolvers/common.rs
+++ b/cli/npm/managed/resolvers/common.rs
@@ -19,7 +19,6 @@ use deno_npm::NpmPackageId;
 use deno_npm::NpmResolutionPackage;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::NodePermissions;
-use deno_runtime::deno_node::NodeResolutionMode;
 
 use crate::npm::managed::cache::TarballCache;
 
@@ -41,7 +40,6 @@ pub trait NpmPackageFsResolver: Send + Sync {
     &self,
     name: &str,
     referrer: &ModuleSpecifier,
-    mode: NodeResolutionMode,
   ) -> Result<PathBuf, AnyError>;
 
   fn resolve_package_cache_folder_id_from_specifier(
diff --git a/cli/npm/managed/resolvers/global.rs b/cli/npm/managed/resolvers/global.rs
index bfd2f4de3..d10b33e7d 100644
--- a/cli/npm/managed/resolvers/global.rs
+++ b/cli/npm/managed/resolvers/global.rs
@@ -11,16 +11,12 @@ use deno_ast::ModuleSpecifier;
 use deno_core::anyhow::bail;
 use deno_core::error::AnyError;
 use deno_core::url::Url;
-use deno_npm::resolution::PackageNotFoundFromReferrerError;
 use deno_npm::NpmPackageCacheFolderId;
 use deno_npm::NpmPackageId;
-use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::NodePermissions;
-use deno_runtime::deno_node::NodeResolutionMode;
 
-use super::super::super::common::types_package_name;
 use super::super::cache::NpmCache;
 use super::super::cache::TarballCache;
 use super::super::resolution::NpmResolution;
@@ -57,17 +53,6 @@ impl GlobalNpmPackageResolver {
       system_info,
     }
   }
-
-  fn resolve_types_package(
-    &self,
-    package_name: &str,
-    referrer_pkg_id: &NpmPackageCacheFolderId,
-  ) -> Result<NpmResolutionPackage, Box<PackageNotFoundFromReferrerError>> {
-    let types_name = types_package_name(package_name);
-    self
-      .resolution
-      .resolve_package_from_package(&types_name, referrer_pkg_id)
-  }
 }
 
 #[async_trait(?Send)]
@@ -92,7 +77,6 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
     &self,
     name: &str,
     referrer: &ModuleSpecifier,
-    mode: NodeResolutionMode,
   ) -> Result<PathBuf, AnyError> {
     let Some(referrer_pkg_id) = self
       .cache
@@ -100,19 +84,9 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
     else {
       bail!("could not find npm package for '{}'", referrer);
     };
-    let pkg = if mode.is_types() && !name.starts_with("@types/") {
-      // attempt to resolve the types package first, then fallback to the regular package
-      match self.resolve_types_package(name, &referrer_pkg_id) {
-        Ok(pkg) => pkg,
-        Err(_) => self
-          .resolution
-          .resolve_package_from_package(name, &referrer_pkg_id)?,
-      }
-    } else {
-      self
-        .resolution
-        .resolve_package_from_package(name, &referrer_pkg_id)?
-    };
+    let pkg = self
+      .resolution
+      .resolve_package_from_package(name, &referrer_pkg_id)?;
     self.package_folder(&pkg.id)
   }
 
diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index e93d400e1..d338720b6 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -39,14 +39,12 @@ use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_node::NodePermissions;
-use deno_runtime::deno_node::NodeResolutionMode;
 use deno_semver::package::PackageNv;
 use serde::Deserialize;
 use serde::Serialize;
 
 use crate::npm::cache_dir::mixed_case_package_name_encode;
 
-use super::super::super::common::types_package_name;
 use super::super::cache::NpmCache;
 use super::super::cache::TarballCache;
 use super::super::resolution::NpmResolution;
@@ -175,7 +173,6 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
     &self,
     name: &str,
     referrer: &ModuleSpecifier,
-    mode: NodeResolutionMode,
   ) -> Result<PathBuf, AnyError> {
     let Some(local_path) = self.resolve_folder_for_specifier(referrer)? else {
       bail!("could not find npm package for '{}'", referrer);
@@ -190,15 +187,6 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
         Cow::Owned(current_folder.join("node_modules"))
       };
 
-      // attempt to resolve the types package first, then fallback to the regular package
-      if mode.is_types() && !name.starts_with("@types/") {
-        let sub_dir =
-          join_package_name(&node_modules_folder, &types_package_name(name));
-        if self.fs.is_dir_sync(&sub_dir) {
-          return Ok(sub_dir);
-        }
-      }
-
       let sub_dir = join_package_name(&node_modules_folder, name);
       if self.fs.is_dir_sync(&sub_dir) {
         return Ok(sub_dir);
diff --git a/cli/resolver.rs b/cli/resolver.rs
index 3edc6f429..994f03f36 100644
--- a/cli/resolver.rs
+++ b/cli/resolver.rs
@@ -514,14 +514,11 @@ impl CliGraphResolver {
     &self,
     specifier: &str,
     referrer: &ModuleSpecifier,
-    mode: NodeResolutionMode,
     original_err: AnyError,
     resolver: &ByonmCliNpmResolver,
   ) -> Result<(), AnyError> {
     if let Ok((pkg_name, _, _)) = parse_npm_pkg_name(specifier, referrer) {
-      match resolver
-        .resolve_package_folder_from_package(&pkg_name, referrer, mode)
-      {
+      match resolver.resolve_package_folder_from_package(&pkg_name, referrer) {
         Ok(_) => {
           return Err(original_err);
         }
@@ -650,7 +647,6 @@ impl Resolver for CliGraphResolver {
                     .check_surface_byonm_node_error(
                       specifier,
                       referrer,
-                      to_node_mode(mode),
                       anyhow!("Cannot find \"{}\"", specifier),
                       resolver,
                     )
@@ -659,11 +655,7 @@ impl Resolver for CliGraphResolver {
                 Err(err) => {
                   self
                     .check_surface_byonm_node_error(
-                      specifier,
-                      referrer,
-                      to_node_mode(mode),
-                      err,
-                      resolver,
+                      specifier, referrer, err, resolver,
                     )
                     .map_err(ResolveError::Other)?;
                 }