fix(byonm): resolve npm deps of jsr deps (#25399)

This allows using npm deps of jsr deps without having to add them to the
root package.json.

Works by taking the package requirement and scanning the
`node_modules/.deno` directory for the best matching package, so it
relies on deno's node_modules structure.

Additionally to make the transition from package.json to deno.json
easier, Deno now:

1. Installs npm deps in a deno.json at the same time as installing npm
deps from a package.json.
2. Uses the alias in the import map for `node_modules/<alias>` for
better package.json compatiblity.

4 files changed, 160 insertions, 67 deletions

diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs
index e62650d8a..3249b2ed1 100644
--- a/cli/npm/byonm.rs
+++ b/cli/npm/byonm.rs
@@ -17,6 +17,7 @@ use deno_runtime::deno_node::NodeRequireResolver;
 use deno_runtime::deno_node::NpmProcessStateProvider;
 use deno_runtime::deno_node::PackageJson;
 use deno_semver::package::PackageReq;
+use deno_semver::Version;
 use node_resolver::errors::PackageFolderResolveError;
 use node_resolver::errors::PackageFolderResolveIoError;
 use node_resolver::errors::PackageJsonLoadError;
@@ -29,6 +30,7 @@ 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::managed::normalize_pkg_name_for_node_modules_deno_folder;
 use super::CliNpmResolver;
 use super::InnerCliNpmResolverRef;
 
@@ -60,9 +62,7 @@ impl ByonmCliNpmResolver {
   ) -> Result<Option<Arc<PackageJson>>, PackageJsonLoadError> {
     load_pkg_json(&DenoPkgJsonFsAdapter(self.fs.as_ref()), path)
   }
-}
 
-impl ByonmCliNpmResolver {
   /// Finds the ancestor package.json that contains the specified dependency.
   pub fn find_ancestor_package_json_with_dep(
     &self,
@@ -98,7 +98,7 @@ impl ByonmCliNpmResolver {
     &self,
     req: &PackageReq,
     referrer: &ModuleSpecifier,
-  ) -> Result<(Arc<PackageJson>, String), AnyError> {
+  ) -> Result<Option<(Arc<PackageJson>, String)>, AnyError> {
     fn resolve_alias_from_pkg_json(
       req: &PackageReq,
       pkg_json: &PackageJson,
@@ -134,7 +134,7 @@ impl ByonmCliNpmResolver {
           if let Some(alias) =
             resolve_alias_from_pkg_json(req, pkg_json.as_ref())
           {
-            return Ok((pkg_json, alias));
+            return Ok(Some((pkg_json, alias)));
           }
         }
         current_path = dir_path;
@@ -148,19 +148,65 @@ impl ByonmCliNpmResolver {
       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((pkg_json, alias));
+          return Ok(Some((pkg_json, alias)));
         }
       }
     }
 
-    bail!(
-      concat!(
-        "Could not find a matching package for 'npm:{}' in a package.json file. ",
-        "You must specify this as a package.json dependency when the ",
-        "node_modules folder is not managed by Deno.",
-      ),
-      req,
+    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,
+      )
+    })
   }
 }
 
@@ -288,29 +334,62 @@ impl CliNpmResolver for ByonmCliNpmResolver {
     req: &PackageReq,
     referrer: &ModuleSpecifier,
   ) -> Result<PathBuf, AnyError> {
-    // resolve the pkg json and alias
-    let (pkg_json, alias) =
-      self.resolve_pkg_json_and_alias_for_req(req, referrer)?;
-    // now try node resolution
-    for ancestor in pkg_json.path.parent().unwrap().ancestors() {
-      let node_modules_folder = ancestor.join("node_modules");
-      let sub_dir = join_package_name(&node_modules_folder, &alias);
-      if self.fs.is_dir_sync(&sub_dir) {
-        return Ok(canonicalize_path_maybe_not_exists_with_fs(
-          &sub_dir,
-          self.fs.as_ref(),
-        )?);
+    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)
     }
 
-    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,
-    );
+    // 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,
+        );
+      }
+    }
   }
 
   fn check_state_hash(&self) -> Option<u64> {
diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
index a0148c648..0be26b785 100644
--- a/cli/npm/managed/mod.rs
+++ b/cli/npm/managed/mod.rs
@@ -32,9 +32,9 @@ use resolution::AddPkgReqsResult;
 
 use crate::args::CliLockfile;
 use crate::args::LifecycleScriptsConfig;
+use crate::args::NpmInstallDepsProvider;
 use crate::args::NpmProcessState;
 use crate::args::NpmProcessStateKind;
-use crate::args::PackageJsonInstallDepsProvider;
 use crate::cache::FastInsecureHasher;
 use crate::http_util::HttpClientProvider;
 use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs;
@@ -45,6 +45,7 @@ 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;
@@ -71,7 +72,7 @@ pub struct CliNpmResolverManagedCreateOptions {
   pub text_only_progress_bar: crate::util::progress_bar::ProgressBar,
   pub maybe_node_modules_path: Option<PathBuf>,
   pub npm_system_info: NpmSystemInfo,
-  pub package_json_deps_provider: Arc<PackageJsonInstallDepsProvider>,
+  pub npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
   pub npmrc: Arc<ResolvedNpmRc>,
   pub lifecycle_scripts: LifecycleScriptsConfig,
 }
@@ -97,7 +98,7 @@ pub async fn create_managed_npm_resolver_for_lsp(
       npm_api,
       npm_cache,
       options.npmrc,
-      options.package_json_deps_provider,
+      options.npm_install_deps_provider,
       options.text_only_progress_bar,
       options.maybe_node_modules_path,
       options.npm_system_info,
@@ -122,7 +123,7 @@ pub async fn create_managed_npm_resolver(
     npm_api,
     npm_cache,
     options.npmrc,
-    options.package_json_deps_provider,
+    options.npm_install_deps_provider,
     options.text_only_progress_bar,
     options.maybe_node_modules_path,
     options.npm_system_info,
@@ -139,7 +140,7 @@ fn create_inner(
   npm_api: Arc<CliNpmRegistryApi>,
   npm_cache: Arc<NpmCache>,
   npm_rc: Arc<ResolvedNpmRc>,
-  package_json_deps_provider: Arc<PackageJsonInstallDepsProvider>,
+  npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
   text_only_progress_bar: crate::util::progress_bar::ProgressBar,
   node_modules_dir_path: Option<PathBuf>,
   npm_system_info: NpmSystemInfo,
@@ -161,7 +162,7 @@ fn create_inner(
   let fs_resolver = create_npm_fs_resolver(
     fs.clone(),
     npm_cache.clone(),
-    &package_json_deps_provider,
+    &npm_install_deps_provider,
     &text_only_progress_bar,
     resolution.clone(),
     tarball_cache.clone(),
@@ -175,7 +176,7 @@ fn create_inner(
     maybe_lockfile,
     npm_api,
     npm_cache,
-    package_json_deps_provider,
+    npm_install_deps_provider,
     resolution,
     tarball_cache,
     text_only_progress_bar,
@@ -261,7 +262,7 @@ pub struct ManagedCliNpmResolver {
   maybe_lockfile: Option<Arc<CliLockfile>>,
   npm_api: Arc<CliNpmRegistryApi>,
   npm_cache: Arc<NpmCache>,
-  package_json_deps_provider: Arc<PackageJsonInstallDepsProvider>,
+  npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
   resolution: Arc<NpmResolution>,
   tarball_cache: Arc<TarballCache>,
   text_only_progress_bar: ProgressBar,
@@ -286,7 +287,7 @@ impl ManagedCliNpmResolver {
     maybe_lockfile: Option<Arc<CliLockfile>>,
     npm_api: Arc<CliNpmRegistryApi>,
     npm_cache: Arc<NpmCache>,
-    package_json_deps_provider: Arc<PackageJsonInstallDepsProvider>,
+    npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
     resolution: Arc<NpmResolution>,
     tarball_cache: Arc<TarballCache>,
     text_only_progress_bar: ProgressBar,
@@ -299,7 +300,7 @@ impl ManagedCliNpmResolver {
       maybe_lockfile,
       npm_api,
       npm_cache,
-      package_json_deps_provider,
+      npm_install_deps_provider,
       text_only_progress_bar,
       resolution,
       tarball_cache,
@@ -476,7 +477,7 @@ impl ManagedCliNpmResolver {
     if !self.top_level_install_flag.raise() {
       return Ok(false); // already did this
     }
-    let pkg_json_remote_pkgs = self.package_json_deps_provider.remote_pkgs();
+    let pkg_json_remote_pkgs = self.npm_install_deps_provider.remote_pkgs();
     if pkg_json_remote_pkgs.is_empty() {
       return Ok(false);
     }
@@ -605,7 +606,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
       create_npm_fs_resolver(
         self.fs.clone(),
         self.npm_cache.clone(),
-        &self.package_json_deps_provider,
+        &self.npm_install_deps_provider,
         &self.text_only_progress_bar,
         npm_resolution.clone(),
         self.tarball_cache.clone(),
@@ -616,7 +617,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
       self.maybe_lockfile.clone(),
       self.npm_api.clone(),
       self.npm_cache.clone(),
-      self.package_json_deps_provider.clone(),
+      self.npm_install_deps_provider.clone(),
       npm_resolution,
       self.tarball_cache.clone(),
       self.text_only_progress_bar.clone(),
diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index 6ac83ee94..eb6f9de9b 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -41,7 +41,7 @@ use node_resolver::errors::ReferrerNotFoundError;
 use serde::Deserialize;
 use serde::Serialize;
 
-use crate::args::PackageJsonInstallDepsProvider;
+use crate::args::NpmInstallDepsProvider;
 use crate::cache::CACHE_PERM;
 use crate::npm::cache_dir::mixed_case_package_name_decode;
 use crate::npm::cache_dir::mixed_case_package_name_encode;
@@ -65,7 +65,7 @@ use super::common::RegistryReadPermissionChecker;
 pub struct LocalNpmPackageResolver {
   cache: Arc<NpmCache>,
   fs: Arc<dyn deno_fs::FileSystem>,
-  pkg_json_deps_provider: Arc<PackageJsonInstallDepsProvider>,
+  npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
   progress_bar: ProgressBar,
   resolution: Arc<NpmResolution>,
   tarball_cache: Arc<TarballCache>,
@@ -81,7 +81,7 @@ impl LocalNpmPackageResolver {
   pub fn new(
     cache: Arc<NpmCache>,
     fs: Arc<dyn deno_fs::FileSystem>,
-    pkg_json_deps_provider: Arc<PackageJsonInstallDepsProvider>,
+    npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
     progress_bar: ProgressBar,
     resolution: Arc<NpmResolution>,
     tarball_cache: Arc<TarballCache>,
@@ -92,7 +92,7 @@ impl LocalNpmPackageResolver {
     Self {
       cache,
       fs: fs.clone(),
-      pkg_json_deps_provider,
+      npm_install_deps_provider,
       progress_bar,
       resolution,
       tarball_cache,
@@ -248,7 +248,7 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
     sync_resolution_with_fs(
       &self.resolution.snapshot(),
       &self.cache,
-      &self.pkg_json_deps_provider,
+      &self.npm_install_deps_provider,
       &self.progress_bar,
       &self.tarball_cache,
       &self.root_node_modules_path,
@@ -412,14 +412,16 @@ fn has_lifecycle_scripts(
 async fn sync_resolution_with_fs(
   snapshot: &NpmResolutionSnapshot,
   cache: &Arc<NpmCache>,
-  pkg_json_deps_provider: &PackageJsonInstallDepsProvider,
+  npm_install_deps_provider: &NpmInstallDepsProvider,
   progress_bar: &ProgressBar,
   tarball_cache: &Arc<TarballCache>,
   root_node_modules_dir_path: &Path,
   system_info: &NpmSystemInfo,
   lifecycle_scripts: &LifecycleScriptsConfig,
 ) -> Result<(), AnyError> {
-  if snapshot.is_empty() && pkg_json_deps_provider.workspace_pkgs().is_empty() {
+  if snapshot.is_empty()
+    && npm_install_deps_provider.workspace_pkgs().is_empty()
+  {
     return Ok(()); // don't create the directory
   }
 
@@ -620,7 +622,7 @@ async fn sync_resolution_with_fs(
 
   // 4. Create symlinks for package json dependencies
   {
-    for remote in pkg_json_deps_provider.remote_pkgs() {
+    for remote in npm_install_deps_provider.remote_pkgs() {
       let remote_pkg = if let Ok(remote_pkg) =
         snapshot.resolve_pkg_from_pkg_req(&remote.req)
       {
@@ -684,7 +686,7 @@ async fn sync_resolution_with_fs(
   }
 
   // 5. Create symlinks for the remaining top level packages in the node_modules folder.
-  // (These may be present if they are not in the package.json dependencies, such as )
+  // (These may be present if they are not in the package.json dependencies)
   // Symlink node_modules/.deno/<package_id>/node_modules/<package_name> to
   // node_modules/<package_name>
   let mut ids = snapshot
@@ -757,10 +759,10 @@ async fn sync_resolution_with_fs(
 
   // 8. Create symlinks for the workspace packages
   {
-    // todo(#24419): this is not exactly correct because it should
+    // todo(dsherret): this is not exactly correct because it should
     // install correctly for a workspace (potentially in sub directories),
     // but this is good enough for a first pass
-    for workspace in pkg_json_deps_provider.workspace_pkgs() {
+    for workspace in npm_install_deps_provider.workspace_pkgs() {
       symlink_package_dir(
         &workspace.target_dir,
         &root_node_modules_dir_path.join(&workspace.alias),
@@ -985,21 +987,31 @@ 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 {
   let copy_str = if folder_id.copy_index == 0 {
-    "".to_string()
+    Cow::Borrowed("")
   } else {
-    format!("_{}", folder_id.copy_index)
+    Cow::Owned(format!("_{}", folder_id.copy_index))
   };
   let nv = &folder_id.nv;
-  let name = if nv.name.to_lowercase() == nv.name {
-    Cow::Borrowed(&nv.name)
-  } else {
-    Cow::Owned(format!("_{}", mixed_case_package_name_encode(&nv.name)))
-  };
-  format!("{}@{}{}", name, nv.version, copy_str).replace('/', "+")
+  let name = normalize_pkg_name_for_node_modules_deno_folder(&nv.name);
+  format!("{}@{}{}", name, nv.version, copy_str)
 }
 
 fn get_package_folder_id_from_folder_name(
diff --git a/cli/npm/managed/resolvers/mod.rs b/cli/npm/managed/resolvers/mod.rs
index 2dfc323e9..f5d9e4b05 100644
--- a/cli/npm/managed/resolvers/mod.rs
+++ b/cli/npm/managed/resolvers/mod.rs
@@ -11,10 +11,11 @@ use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs::FileSystem;
 
 use crate::args::LifecycleScriptsConfig;
-use crate::args::PackageJsonInstallDepsProvider;
+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;
@@ -27,7 +28,7 @@ use super::resolution::NpmResolution;
 pub fn create_npm_fs_resolver(
   fs: Arc<dyn FileSystem>,
   npm_cache: Arc<NpmCache>,
-  pkg_json_deps_provider: &Arc<PackageJsonInstallDepsProvider>,
+  npm_install_deps_provider: &Arc<NpmInstallDepsProvider>,
   progress_bar: &ProgressBar,
   resolution: Arc<NpmResolution>,
   tarball_cache: Arc<TarballCache>,
@@ -39,7 +40,7 @@ pub fn create_npm_fs_resolver(
     Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new(
       npm_cache,
       fs,
-      pkg_json_deps_provider.clone(),
+      npm_install_deps_provider.clone(),
       progress_bar.clone(),
       resolution,
       tarball_cache,