refactor: move ByonmNpmResolver to deno_resolver (#25937)

Some more slow progress on moving all the resolution code into
deno_resolver.

7 files changed, 40 insertions, 359 deletions

diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs
index 83c406765..ceef68135 100644
--- a/cli/npm/byonm.rs
+++ b/cli/npm/byonm.rs
@@ -1,276 +1,36 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use std::borrow::Cow;
 use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
 
-use deno_ast::ModuleSpecifier;
-use deno_core::anyhow::bail;
 use deno_core::error::AnyError;
 use deno_core::serde_json;
-use deno_package_json::PackageJsonDepValue;
-use deno_path_util::url_to_file_path;
-use deno_runtime::deno_fs::FileSystem;
-use deno_runtime::deno_node::DenoPkgJsonFsAdapter;
+use deno_core::url::Url;
+use deno_resolver::npm::ByonmNpmResolver;
+use deno_resolver::npm::ByonmNpmResolverCreateOptions;
 use deno_runtime::deno_node::NodePermissions;
 use deno_runtime::deno_node::NodeRequireResolver;
-use deno_runtime::deno_node::PackageJson;
 use deno_runtime::ops::process::NpmProcessStateProvider;
 use deno_semver::package::PackageReq;
-use deno_semver::Version;
-use node_resolver::errors::PackageFolderResolveError;
-use node_resolver::errors::PackageFolderResolveIoError;
-use node_resolver::errors::PackageJsonLoadError;
-use node_resolver::errors::PackageNotFoundError;
-use node_resolver::load_pkg_json;
 use node_resolver::NpmResolver;
 
 use crate::args::NpmProcessState;
 use crate::args::NpmProcessStateKind;
-use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs;
+use crate::resolver::CliDenoResolverFs;
 
-use super::managed::normalize_pkg_name_for_node_modules_deno_folder;
 use super::CliNpmResolver;
 use super::InnerCliNpmResolverRef;
 
-pub struct CliNpmResolverByonmCreateOptions {
-  pub fs: Arc<dyn FileSystem>,
-  // todo(dsherret): investigate removing this
-  pub root_node_modules_dir: Option<PathBuf>,
-}
-
-pub fn create_byonm_npm_resolver(
-  options: CliNpmResolverByonmCreateOptions,
-) -> Arc<dyn CliNpmResolver> {
-  Arc::new(ByonmCliNpmResolver {
-    fs: options.fs,
-    root_node_modules_dir: options.root_node_modules_dir,
-  })
-}
+pub type CliByonmNpmResolverCreateOptions =
+  ByonmNpmResolverCreateOptions<CliDenoResolverFs>;
+pub type CliByonmNpmResolver = ByonmNpmResolver<CliDenoResolverFs>;
 
+// todo(dsherret): the services hanging off `CliNpmResolver` doesn't seem ideal. We should probably decouple.
 #[derive(Debug)]
-pub struct ByonmCliNpmResolver {
-  fs: Arc<dyn FileSystem>,
-  root_node_modules_dir: Option<PathBuf>,
-}
-
-impl ByonmCliNpmResolver {
-  fn load_pkg_json(
-    &self,
-    path: &Path,
-  ) -> Result<Option<Arc<PackageJson>>, PackageJsonLoadError> {
-    load_pkg_json(&DenoPkgJsonFsAdapter(self.fs.as_ref()), path)
-  }
-
-  /// Finds the ancestor package.json that contains the specified dependency.
-  pub fn find_ancestor_package_json_with_dep(
-    &self,
-    dep_name: &str,
-    referrer: &ModuleSpecifier,
-  ) -> Option<Arc<PackageJson>> {
-    let referrer_path = referrer.to_file_path().ok()?;
-    let mut current_folder = referrer_path.parent()?;
-    loop {
-      let pkg_json_path = current_folder.join("package.json");
-      if let Ok(Some(pkg_json)) = self.load_pkg_json(&pkg_json_path) {
-        if let Some(deps) = &pkg_json.dependencies {
-          if deps.contains_key(dep_name) {
-            return Some(pkg_json);
-          }
-        }
-        if let Some(deps) = &pkg_json.dev_dependencies {
-          if deps.contains_key(dep_name) {
-            return Some(pkg_json);
-          }
-        }
-      }
-
-      if let Some(parent) = current_folder.parent() {
-        current_folder = parent;
-      } else {
-        return None;
-      }
-    }
-  }
-
-  fn resolve_pkg_json_and_alias_for_req(
-    &self,
-    req: &PackageReq,
-    referrer: &ModuleSpecifier,
-  ) -> Result<Option<(Arc<PackageJson>, String)>, AnyError> {
-    fn resolve_alias_from_pkg_json(
-      req: &PackageReq,
-      pkg_json: &PackageJson,
-    ) -> Option<String> {
-      let deps = pkg_json.resolve_local_package_json_deps();
-      for (key, value) in deps {
-        if let Ok(value) = value {
-          match value {
-            PackageJsonDepValue::Req(dep_req) => {
-              if dep_req.name == req.name
-                && dep_req.version_req.intersects(&req.version_req)
-              {
-                return Some(key);
-              }
-            }
-            PackageJsonDepValue::Workspace(_workspace) => {
-              if key == req.name && req.version_req.tag() == Some("workspace") {
-                return Some(key);
-              }
-            }
-          }
-        }
-      }
-      None
-    }
-
-    // attempt to resolve the npm specifier from the referrer's package.json,
-    if let Ok(file_path) = url_to_file_path(referrer) {
-      let mut current_path = file_path.as_path();
-      while let Some(dir_path) = current_path.parent() {
-        let package_json_path = dir_path.join("package.json");
-        if let Some(pkg_json) = self.load_pkg_json(&package_json_path)? {
-          if let Some(alias) =
-            resolve_alias_from_pkg_json(req, pkg_json.as_ref())
-          {
-            return Ok(Some((pkg_json, alias)));
-          }
-        }
-        current_path = dir_path;
-      }
-    }
-
-    // otherwise, fall fallback to the project's package.json
-    if let Some(root_node_modules_dir) = &self.root_node_modules_dir {
-      let root_pkg_json_path =
-        root_node_modules_dir.parent().unwrap().join("package.json");
-      if let Some(pkg_json) = self.load_pkg_json(&root_pkg_json_path)? {
-        if let Some(alias) = resolve_alias_from_pkg_json(req, pkg_json.as_ref())
-        {
-          return Ok(Some((pkg_json, alias)));
-        }
-      }
-    }
-
-    Ok(None)
-  }
-
-  fn resolve_folder_in_root_node_modules(
-    &self,
-    req: &PackageReq,
-  ) -> Option<PathBuf> {
-    // now check if node_modules/.deno/ matches this constraint
-    let root_node_modules_dir = self.root_node_modules_dir.as_ref()?;
-    let node_modules_deno_dir = root_node_modules_dir.join(".deno");
-    let Ok(entries) = self.fs.read_dir_sync(&node_modules_deno_dir) else {
-      return None;
-    };
-    let search_prefix = format!(
-      "{}@",
-      normalize_pkg_name_for_node_modules_deno_folder(&req.name)
-    );
-    let mut best_version = None;
-
-    // example entries:
-    // - @denotest+add@1.0.0
-    // - @denotest+add@1.0.0_1
-    for entry in entries {
-      if !entry.is_directory {
-        continue;
-      }
-      let Some(version_and_copy_idx) = entry.name.strip_prefix(&search_prefix)
-      else {
-        continue;
-      };
-      let version = version_and_copy_idx
-        .rsplit_once('_')
-        .map(|(v, _)| v)
-        .unwrap_or(version_and_copy_idx);
-      let Ok(version) = Version::parse_from_npm(version) else {
-        continue;
-      };
-      if req.version_req.matches(&version) {
-        if let Some((best_version_version, _)) = &best_version {
-          if version > *best_version_version {
-            best_version = Some((version, entry.name));
-          }
-        } else {
-          best_version = Some((version, entry.name));
-        }
-      }
-    }
-
-    best_version.map(|(_version, entry_name)| {
-      join_package_name(
-        &node_modules_deno_dir.join(entry_name).join("node_modules"),
-        &req.name,
-      )
-    })
-  }
-}
-
-impl NpmResolver for ByonmCliNpmResolver {
-  fn resolve_package_folder_from_package(
-    &self,
-    name: &str,
-    referrer: &ModuleSpecifier,
-  ) -> Result<PathBuf, PackageFolderResolveError> {
-    fn inner(
-      fs: &dyn FileSystem,
-      name: &str,
-      referrer: &ModuleSpecifier,
-    ) -> Result<PathBuf, PackageFolderResolveError> {
-      let maybe_referrer_file = url_to_file_path(referrer).ok();
-      let maybe_start_folder =
-        maybe_referrer_file.as_ref().and_then(|f| f.parent());
-      if let Some(start_folder) = maybe_start_folder {
-        for current_folder in start_folder.ancestors() {
-          let node_modules_folder = if current_folder.ends_with("node_modules")
-          {
-            Cow::Borrowed(current_folder)
-          } else {
-            Cow::Owned(current_folder.join("node_modules"))
-          };
+struct CliByonmWrapper(Arc<CliByonmNpmResolver>);
 
-          let sub_dir = join_package_name(&node_modules_folder, name);
-          if fs.is_dir_sync(&sub_dir) {
-            return Ok(sub_dir);
-          }
-        }
-      }
-
-      Err(
-        PackageNotFoundError {
-          package_name: name.to_string(),
-          referrer: referrer.clone(),
-          referrer_extra: None,
-        }
-        .into(),
-      )
-    }
-
-    let path = inner(&*self.fs, name, referrer)?;
-    self.fs.realpath_sync(&path).map_err(|err| {
-      PackageFolderResolveIoError {
-        package_name: name.to_string(),
-        referrer: referrer.clone(),
-        source: err.into_io_error(),
-      }
-      .into()
-    })
-  }
-
-  fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool {
-    specifier.scheme() == "file"
-      && specifier
-        .path()
-        .to_ascii_lowercase()
-        .contains("/node_modules/")
-  }
-}
-
-impl NodeRequireResolver for ByonmCliNpmResolver {
+impl NodeRequireResolver for CliByonmWrapper {
   fn ensure_read_permission(
     &self,
     permissions: &mut dyn NodePermissions,
@@ -286,110 +46,54 @@ impl NodeRequireResolver for ByonmCliNpmResolver {
   }
 }
 
-impl NpmProcessStateProvider for ByonmCliNpmResolver {
+impl NpmProcessStateProvider for CliByonmWrapper {
   fn get_npm_process_state(&self) -> String {
     serde_json::to_string(&NpmProcessState {
       kind: NpmProcessStateKind::Byonm,
       local_node_modules_path: self
-        .root_node_modules_dir
-        .as_ref()
+        .0
+        .root_node_modules_dir()
         .map(|p| p.to_string_lossy().to_string()),
     })
     .unwrap()
   }
 }
 
-impl CliNpmResolver for ByonmCliNpmResolver {
+impl CliNpmResolver for CliByonmNpmResolver {
   fn into_npm_resolver(self: Arc<Self>) -> Arc<dyn NpmResolver> {
     self
   }
 
   fn into_require_resolver(self: Arc<Self>) -> Arc<dyn NodeRequireResolver> {
-    self
+    Arc::new(CliByonmWrapper(self))
   }
 
   fn into_process_state_provider(
     self: Arc<Self>,
   ) -> Arc<dyn NpmProcessStateProvider> {
-    self
+    Arc::new(CliByonmWrapper(self))
   }
 
   fn clone_snapshotted(&self) -> Arc<dyn CliNpmResolver> {
-    Arc::new(Self {
-      fs: self.fs.clone(),
-      root_node_modules_dir: self.root_node_modules_dir.clone(),
-    })
+    Arc::new(self.clone())
   }
 
   fn as_inner(&self) -> InnerCliNpmResolverRef {
     InnerCliNpmResolverRef::Byonm(self)
   }
 
-  fn root_node_modules_path(&self) -> Option<&PathBuf> {
-    self.root_node_modules_dir.as_ref()
+  fn root_node_modules_path(&self) -> Option<&Path> {
+    self.root_node_modules_dir()
   }
 
   fn resolve_pkg_folder_from_deno_module_req(
     &self,
     req: &PackageReq,
-    referrer: &ModuleSpecifier,
+    referrer: &Url,
   ) -> Result<PathBuf, AnyError> {
-    fn node_resolve_dir(
-      fs: &dyn FileSystem,
-      alias: &str,
-      start_dir: &Path,
-    ) -> Result<Option<PathBuf>, AnyError> {
-      for ancestor in start_dir.ancestors() {
-        let node_modules_folder = ancestor.join("node_modules");
-        let sub_dir = join_package_name(&node_modules_folder, alias);
-        if fs.is_dir_sync(&sub_dir) {
-          return Ok(Some(canonicalize_path_maybe_not_exists_with_fs(
-            &sub_dir, fs,
-          )?));
-        }
-      }
-      Ok(None)
-    }
-
-    // now attempt to resolve if it's found in any package.json
-    let maybe_pkg_json_and_alias =
-      self.resolve_pkg_json_and_alias_for_req(req, referrer)?;
-    match maybe_pkg_json_and_alias {
-      Some((pkg_json, alias)) => {
-        // now try node resolution
-        if let Some(resolved) =
-          node_resolve_dir(self.fs.as_ref(), &alias, pkg_json.dir_path())?
-        {
-          return Ok(resolved);
-        }
-
-        bail!(
-          concat!(
-            "Could not find \"{}\" in a node_modules folder. ",
-            "Deno expects the node_modules/ directory to be up to date. ",
-            "Did you forget to run `deno install`?"
-          ),
-          alias,
-        );
-      }
-      None => {
-        // now check if node_modules/.deno/ matches this constraint
-        if let Some(folder) = self.resolve_folder_in_root_node_modules(req) {
-          return Ok(folder);
-        }
-
-        bail!(
-          concat!(
-            "Could not find a matching package for 'npm:{}' in the node_modules ",
-            "directory. Ensure you have all your JSR and npm dependencies listed ",
-            "in your deno.json or package.json, then run `deno install`. Alternatively, ",
-            r#"turn on auto-install by specifying `"nodeModulesDir": "auto"` in your "#,
-            "deno.json file."
-          ),
-          req,
-        );
-      }
-    }
+    ByonmNpmResolver::resolve_pkg_folder_from_deno_module_req(
+      self, req, referrer,
+    )
   }
 
   fn check_state_hash(&self) -> Option<u64> {
@@ -398,12 +102,3 @@ impl CliNpmResolver for ByonmCliNpmResolver {
     None
   }
 }
-
-fn join_package_name(path: &Path, package_name: &str) -> PathBuf {
-  let mut path = path.to_path_buf();
-  // ensure backslashes are used on windows
-  for part in package_name.split('/') {
-    path = path.join(part);
-  }
-  path
-}
diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
index e3ac5e1af..62af3e4aa 100644
--- a/cli/npm/managed/mod.rs
+++ b/cli/npm/managed/mod.rs
@@ -47,7 +47,6 @@ use self::cache::NpmCache;
 use self::registry::CliNpmRegistryApi;
 use self::resolution::NpmResolution;
 use self::resolvers::create_npm_fs_resolver;
-pub use self::resolvers::normalize_pkg_name_for_node_modules_deno_folder;
 use self::resolvers::NpmPackageFsResolver;
 
 use super::CliNpmResolver;
@@ -575,7 +574,7 @@ impl NpmProcessStateProvider for ManagedCliNpmResolver {
   fn get_npm_process_state(&self) -> String {
     npm_process_state(
       self.resolution.serialized_valid_snapshot(),
-      self.fs_resolver.node_modules_path().map(|p| p.as_path()),
+      self.fs_resolver.node_modules_path(),
     )
   }
 }
@@ -632,7 +631,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
     InnerCliNpmResolverRef::Managed(self)
   }
 
-  fn root_node_modules_path(&self) -> Option<&PathBuf> {
+  fn root_node_modules_path(&self) -> Option<&Path> {
     self.fs_resolver.node_modules_path()
   }
 
diff --git a/cli/npm/managed/resolvers/common.rs b/cli/npm/managed/resolvers/common.rs
index 620daf4b3..8df4debc5 100644
--- a/cli/npm/managed/resolvers/common.rs
+++ b/cli/npm/managed/resolvers/common.rs
@@ -33,7 +33,7 @@ pub trait NpmPackageFsResolver: Send + Sync {
   fn root_dir_url(&self) -> &Url;
 
   /// The local node_modules folder if it is applicable to the implementation.
-  fn node_modules_path(&self) -> Option<&PathBuf>;
+  fn node_modules_path(&self) -> Option<&Path>;
 
   fn maybe_package_folder(&self, package_id: &NpmPackageId) -> Option<PathBuf>;
 
diff --git a/cli/npm/managed/resolvers/global.rs b/cli/npm/managed/resolvers/global.rs
index 187e6b277..ca5722c86 100644
--- a/cli/npm/managed/resolvers/global.rs
+++ b/cli/npm/managed/resolvers/global.rs
@@ -73,7 +73,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
     self.cache.root_dir_url()
   }
 
-  fn node_modules_path(&self) -> Option<&PathBuf> {
+  fn node_modules_path(&self) -> Option<&Path> {
     None
   }
 
diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index 297fcab23..59ba27d05 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -20,7 +20,6 @@ use crate::colors;
 use async_trait::async_trait;
 use deno_ast::ModuleSpecifier;
 use deno_cache_dir::npm::mixed_case_package_name_decode;
-use deno_cache_dir::npm::mixed_case_package_name_encode;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::futures::stream::FuturesUnordered;
@@ -32,6 +31,7 @@ use deno_npm::NpmPackageCacheFolderId;
 use deno_npm::NpmPackageId;
 use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
+use deno_resolver::npm::normalize_pkg_name_for_node_modules_deno_folder;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_node::NodePermissions;
 use deno_semver::package::PackageNv;
@@ -159,8 +159,8 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
     &self.root_node_modules_url
   }
 
-  fn node_modules_path(&self) -> Option<&PathBuf> {
-    Some(&self.root_node_modules_path)
+  fn node_modules_path(&self) -> Option<&Path> {
+    Some(self.root_node_modules_path.as_ref())
   }
 
   fn maybe_package_folder(&self, id: &NpmPackageId) -> Option<PathBuf> {
@@ -920,20 +920,6 @@ impl SetupCache {
   }
 }
 
-/// Normalizes a package name for use at `node_modules/.deno/<pkg-name>@<version>[_<copy_index>]`
-pub fn normalize_pkg_name_for_node_modules_deno_folder(name: &str) -> Cow<str> {
-  let name = if name.to_lowercase() == name {
-    Cow::Borrowed(name)
-  } else {
-    Cow::Owned(format!("_{}", mixed_case_package_name_encode(name)))
-  };
-  if name.starts_with('@') {
-    name.replace('/', "+").into()
-  } else {
-    name
-  }
-}
-
 fn get_package_folder_id_folder_name(
   folder_id: &NpmPackageCacheFolderId,
 ) -> String {
diff --git a/cli/npm/managed/resolvers/mod.rs b/cli/npm/managed/resolvers/mod.rs
index 234a6e4db..36d795ee7 100644
--- a/cli/npm/managed/resolvers/mod.rs
+++ b/cli/npm/managed/resolvers/mod.rs
@@ -15,7 +15,6 @@ use crate::args::NpmInstallDepsProvider;
 use crate::util::progress_bar::ProgressBar;
 
 pub use self::common::NpmPackageFsResolver;
-pub use self::local::normalize_pkg_name_for_node_modules_deno_folder;
 
 use self::global::GlobalNpmPackageResolver;
 use self::local::LocalNpmPackageResolver;
diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs
index 2c9ee20bc..1e3c752ae 100644
--- a/cli/npm/mod.rs
+++ b/cli/npm/mod.rs
@@ -4,6 +4,7 @@ mod byonm;
 mod common;
 mod managed;
 
+use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
 
@@ -12,6 +13,7 @@ use deno_ast::ModuleSpecifier;
 use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_npm::registry::NpmPackageInfo;
+use deno_resolver::npm::ByonmNpmResolver;
 use deno_runtime::deno_node::NodeRequireResolver;
 use deno_runtime::ops::process::NpmProcessStateProvider;
 use deno_semver::package::PackageNv;
@@ -21,15 +23,15 @@ use node_resolver::NpmResolver;
 use crate::args::npm_registry_url;
 use crate::file_fetcher::FileFetcher;
 
-pub use self::byonm::ByonmCliNpmResolver;
-pub use self::byonm::CliNpmResolverByonmCreateOptions;
+pub use self::byonm::CliByonmNpmResolver;
+pub use self::byonm::CliByonmNpmResolverCreateOptions;
 pub use self::managed::CliNpmResolverManagedCreateOptions;
 pub use self::managed::CliNpmResolverManagedSnapshotOption;
 pub use self::managed::ManagedCliNpmResolver;
 
 pub enum CliNpmResolverCreateOptions {
   Managed(CliNpmResolverManagedCreateOptions),
-  Byonm(CliNpmResolverByonmCreateOptions),
+  Byonm(CliByonmNpmResolverCreateOptions),
 }
 
 pub async fn create_cli_npm_resolver_for_lsp(
@@ -40,7 +42,7 @@ pub async fn create_cli_npm_resolver_for_lsp(
     Managed(options) => {
       managed::create_managed_npm_resolver_for_lsp(options).await
     }
-    Byonm(options) => byonm::create_byonm_npm_resolver(options),
+    Byonm(options) => Arc::new(ByonmNpmResolver::new(options)),
   }
 }
 
@@ -50,14 +52,14 @@ pub async fn create_cli_npm_resolver(
   use CliNpmResolverCreateOptions::*;
   match options {
     Managed(options) => managed::create_managed_npm_resolver(options).await,
-    Byonm(options) => Ok(byonm::create_byonm_npm_resolver(options)),
+    Byonm(options) => Ok(Arc::new(ByonmNpmResolver::new(options))),
   }
 }
 
 pub enum InnerCliNpmResolverRef<'a> {
   Managed(&'a ManagedCliNpmResolver),
   #[allow(dead_code)]
-  Byonm(&'a ByonmCliNpmResolver),
+  Byonm(&'a CliByonmNpmResolver),
 }
 
 pub trait CliNpmResolver: NpmResolver {
@@ -78,14 +80,14 @@ pub trait CliNpmResolver: NpmResolver {
     }
   }
 
-  fn as_byonm(&self) -> Option<&ByonmCliNpmResolver> {
+  fn as_byonm(&self) -> Option<&CliByonmNpmResolver> {
     match self.as_inner() {
       InnerCliNpmResolverRef::Managed(_) => None,
       InnerCliNpmResolverRef::Byonm(inner) => Some(inner),
     }
   }
 
-  fn root_node_modules_path(&self) -> Option<&PathBuf>;
+  fn root_node_modules_path(&self) -> Option<&Path>;
 
   fn resolve_pkg_folder_from_deno_module_req(
     &self,