refactor(cli): make `CliNpmResolver` a trait (#20732)

This makes `CliNpmResolver` a trait. The terminology used is:

- **managed** - Deno manages the node_modules folder and does an
auto-install (ex. `ManagedCliNpmResolver`)
- **byonm** - "Bring your own node_modules" (ex. `ByonmCliNpmResolver`,
which is in this PR, but unimplemented at the moment)

Part of #18967

4 files changed, 117 insertions, 32 deletions

diff --git a/cli/tools/check.rs b/cli/tools/check.rs
index a61e3cfe1..0a25518e4 100644
--- a/cli/tools/check.rs
+++ b/cli/tools/check.rs
@@ -44,7 +44,7 @@ pub struct TypeChecker {
   caches: Arc<Caches>,
   cli_options: Arc<CliOptions>,
   node_resolver: Arc<NodeResolver>,
-  npm_resolver: Arc<CliNpmResolver>,
+  npm_resolver: Arc<dyn CliNpmResolver>,
 }
 
 impl TypeChecker {
@@ -52,7 +52,7 @@ impl TypeChecker {
     caches: Arc<Caches>,
     cli_options: Arc<CliOptions>,
     node_resolver: Arc<NodeResolver>,
-    npm_resolver: Arc<CliNpmResolver>,
+    npm_resolver: Arc<dyn CliNpmResolver>,
   ) -> Self {
     Self {
       caches,
@@ -74,11 +74,10 @@ impl TypeChecker {
     // node built-in specifiers use the @types/node package to determine
     // types, so inject that now (the caller should do this after the lockfile
     // has been written)
-    if graph.has_node_specifier {
-      self
-        .npm_resolver
-        .inject_synthetic_types_node_package()
-        .await?;
+    if let Some(npm_resolver) = self.npm_resolver.as_managed() {
+      if graph.has_node_specifier {
+        npm_resolver.inject_synthetic_types_node_package().await?;
+      }
     }
 
     log::debug!("Type checking.");
diff --git a/cli/tools/info.rs b/cli/tools/info.rs
index 941ba1cbd..e1972f08f 100644
--- a/cli/tools/info.rs
+++ b/cli/tools/info.rs
@@ -31,6 +31,7 @@ use crate::display;
 use crate::factory::CliFactory;
 use crate::graph_util::graph_lock_or_exit;
 use crate::npm::CliNpmResolver;
+use crate::npm::ManagedCliNpmResolver;
 use crate::util::checksum;
 
 pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> {
@@ -71,11 +72,11 @@ pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> {
 
     if info_flags.json {
       let mut json_graph = json!(graph);
-      add_npm_packages_to_json(&mut json_graph, npm_resolver);
+      add_npm_packages_to_json(&mut json_graph, npm_resolver.as_ref());
       display::write_json_to_stdout(&json_graph)?;
     } else {
       let mut output = String::new();
-      GraphDisplayContext::write(&graph, npm_resolver, &mut output)?;
+      GraphDisplayContext::write(&graph, npm_resolver.as_ref(), &mut output)?;
       display::write_to_stdout_ignore_sigpipe(output.as_bytes())?;
     }
   } else {
@@ -165,8 +166,12 @@ fn print_cache_info(
 
 fn add_npm_packages_to_json(
   json: &mut serde_json::Value,
-  npm_resolver: &CliNpmResolver,
+  npm_resolver: &dyn CliNpmResolver,
 ) {
+  let Some(npm_resolver) = npm_resolver.as_managed() else {
+    return; // does not include byonm to deno info's output
+  };
+
   // ideally deno_graph could handle this, but for now we just modify the json here
   let snapshot = npm_resolver.snapshot();
   let json = json.as_object_mut().unwrap();
@@ -339,7 +344,7 @@ struct NpmInfo {
 impl NpmInfo {
   pub fn build<'a>(
     graph: &'a ModuleGraph,
-    npm_resolver: &'a CliNpmResolver,
+    npm_resolver: &'a ManagedCliNpmResolver,
     npm_snapshot: &'a NpmResolutionSnapshot,
   ) -> Self {
     let mut info = NpmInfo::default();
@@ -365,7 +370,7 @@ impl NpmInfo {
   fn fill_package_info<'a>(
     &mut self,
     package: &NpmResolutionPackage,
-    npm_resolver: &'a CliNpmResolver,
+    npm_resolver: &'a ManagedCliNpmResolver,
     npm_snapshot: &'a NpmResolutionSnapshot,
   ) {
     self.packages.insert(package.id.clone(), package.clone());
@@ -399,11 +404,16 @@ struct GraphDisplayContext<'a> {
 impl<'a> GraphDisplayContext<'a> {
   pub fn write<TWrite: Write>(
     graph: &'a ModuleGraph,
-    npm_resolver: &'a CliNpmResolver,
+    npm_resolver: &'a dyn CliNpmResolver,
     writer: &mut TWrite,
   ) -> fmt::Result {
-    let npm_snapshot = npm_resolver.snapshot();
-    let npm_info = NpmInfo::build(graph, npm_resolver, &npm_snapshot);
+    let npm_info = match npm_resolver.as_managed() {
+      Some(npm_resolver) => {
+        let npm_snapshot = npm_resolver.snapshot();
+        NpmInfo::build(graph, npm_resolver, &npm_snapshot)
+      }
+      None => NpmInfo::default(),
+    };
     Self {
       graph,
       npm_info,
diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs
index a1b602b4b..f833fbf5d 100644
--- a/cli/tools/repl/session.rs
+++ b/cli/tools/repl/session.rs
@@ -123,7 +123,7 @@ pub struct TsEvaluateResponse {
 }
 
 pub struct ReplSession {
-  npm_resolver: Arc<CliNpmResolver>,
+  npm_resolver: Arc<dyn CliNpmResolver>,
   resolver: Arc<CliGraphResolver>,
   pub worker: MainWorker,
   session: LocalInspectorSession,
@@ -136,7 +136,7 @@ pub struct ReplSession {
 impl ReplSession {
   pub async fn initialize(
     cli_options: &CliOptions,
-    npm_resolver: Arc<CliNpmResolver>,
+    npm_resolver: Arc<dyn CliNpmResolver>,
     resolver: Arc<CliGraphResolver>,
     mut worker: MainWorker,
   ) -> Result<Self, AnyError> {
@@ -508,6 +508,10 @@ impl ReplSession {
     &mut self,
     program: &swc_ast::Program,
   ) -> Result<(), AnyError> {
+    let Some(npm_resolver) = self.npm_resolver.as_managed() else {
+      return Ok(()); // don't auto-install for byonm
+    };
+
     let mut collector = ImportCollector::new();
     program.visit_with(&mut collector);
 
@@ -531,14 +535,11 @@ impl ReplSession {
     let has_node_specifier =
       resolved_imports.iter().any(|url| url.scheme() == "node");
     if !npm_imports.is_empty() || has_node_specifier {
-      self.npm_resolver.add_package_reqs(&npm_imports).await?;
+      npm_resolver.add_package_reqs(&npm_imports).await?;
 
       // prevent messages in the repl about @types/node not being cached
       if has_node_specifier {
-        self
-          .npm_resolver
-          .inject_synthetic_types_node_package()
-          .await?;
+        npm_resolver.inject_synthetic_types_node_package().await?;
       }
     }
     Ok(())
diff --git a/cli/tools/task.rs b/cli/tools/task.rs
index 6a6c23e39..d1513072a 100644
--- a/cli/tools/task.rs
+++ b/cli/tools/task.rs
@@ -5,7 +5,7 @@ use crate::args::Flags;
 use crate::args::TaskFlags;
 use crate::colors;
 use crate::factory::CliFactory;
-use crate::npm::CliNpmResolver;
+use crate::npm::ManagedCliNpmResolver;
 use crate::util::fs::canonicalize_path;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
@@ -19,6 +19,7 @@ use deno_task_shell::ShellCommand;
 use deno_task_shell::ShellCommandContext;
 use indexmap::IndexMap;
 use std::collections::HashMap;
+use std::path::Path;
 use std::path::PathBuf;
 use std::rc::Rc;
 use tokio::task::LocalSet;
@@ -67,8 +68,6 @@ pub async fn execute_script(
     Ok(exit_code)
   } else if package_json_scripts.contains_key(task_name) {
     let package_json_deps_provider = factory.package_json_deps_provider();
-    let package_json_deps_installer =
-      factory.package_json_deps_installer().await?;
     let npm_resolver = factory.npm_resolver().await?;
     let node_resolver = factory.node_resolver().await?;
 
@@ -85,10 +84,15 @@ pub async fn execute_script(
       }
     }
 
-    package_json_deps_installer
-      .ensure_top_level_install()
-      .await?;
-    npm_resolver.resolve_pending().await?;
+    // install the npm packages if we're using a managed resolver
+    if let Some(npm_resolver) = npm_resolver.as_managed() {
+      let package_json_deps_installer =
+        factory.package_json_deps_installer().await?;
+      package_json_deps_installer
+        .ensure_top_level_install()
+        .await?;
+      npm_resolver.resolve_pending().await?;
+    }
 
     log::info!(
       "{} Currently only basic package.json `scripts` are supported. Programs like `rimraf` or `cross-env` will not work correctly. This will be fixed in an upcoming release.",
@@ -120,8 +124,16 @@ pub async fn execute_script(
         output_task(&task_name, &script);
         let seq_list = deno_task_shell::parser::parse(&script)
           .with_context(|| format!("Error parsing script '{task_name}'."))?;
-        let npx_commands = resolve_npm_commands(npm_resolver, node_resolver)?;
-        let env_vars = collect_env_vars();
+        let npx_commands = match npm_resolver.as_managed() {
+          Some(npm_resolver) => {
+            resolve_npm_commands(npm_resolver, node_resolver)?
+          }
+          None => Default::default(),
+        };
+        let env_vars = match npm_resolver.node_modules_path() {
+          Some(dir_path) => collect_env_vars_with_node_modules_dir(&dir_path),
+          None => collect_env_vars(),
+        };
         let local = LocalSet::new();
         let future =
           deno_task_shell::execute(seq_list, env_vars, &cwd, npx_commands);
@@ -162,6 +174,36 @@ fn output_task(task_name: &str, script: &str) {
   );
 }
 
+fn collect_env_vars_with_node_modules_dir(
+  node_modules_dir_path: &Path,
+) -> HashMap<String, String> {
+  let mut env_vars = collect_env_vars();
+  prepend_to_path(
+    &mut env_vars,
+    node_modules_dir_path
+      .join(".bin")
+      .to_string_lossy()
+      .to_string(),
+  );
+  env_vars
+}
+
+fn prepend_to_path(env_vars: &mut HashMap<String, String>, value: String) {
+  match env_vars.get_mut("PATH") {
+    Some(path) => {
+      if path.is_empty() {
+        *path = value;
+      } else {
+        *path =
+          format!("{}{}{}", value, if cfg!(windows) { ";" } else { ":" }, path);
+      }
+    }
+    None => {
+      env_vars.insert("PATH".to_string(), value);
+    }
+  }
+}
+
 fn collect_env_vars() -> HashMap<String, String> {
   // get the starting env vars (the PWD env var will be set by deno_task_shell)
   let mut env_vars = std::env::vars().collect::<HashMap<String, String>>();
@@ -262,7 +304,7 @@ impl ShellCommand for NpmPackageBinCommand {
 }
 
 fn resolve_npm_commands(
-  npm_resolver: &CliNpmResolver,
+  npm_resolver: &ManagedCliNpmResolver,
   node_resolver: &NodeResolver,
 ) -> Result<HashMap<String, Rc<dyn ShellCommand>>, AnyError> {
   let mut result = HashMap::new();
@@ -286,3 +328,36 @@ fn resolve_npm_commands(
   }
   Ok(result)
 }
+
+#[cfg(test)]
+mod test {
+  use super::*;
+
+  #[test]
+  fn test_prepend_to_path() {
+    let mut env_vars = HashMap::new();
+
+    prepend_to_path(&mut env_vars, "/example".to_string());
+    assert_eq!(
+      env_vars,
+      HashMap::from([("PATH".to_string(), "/example".to_string())])
+    );
+
+    prepend_to_path(&mut env_vars, "/example2".to_string());
+    let separator = if cfg!(windows) { ";" } else { ":" };
+    assert_eq!(
+      env_vars,
+      HashMap::from([(
+        "PATH".to_string(),
+        format!("/example2{}/example", separator)
+      )])
+    );
+
+    env_vars.get_mut("PATH").unwrap().clear();
+    prepend_to_path(&mut env_vars, "/example".to_string());
+    assert_eq!(
+      env_vars,
+      HashMap::from([("PATH".to_string(), "/example".to_string())])
+    );
+  }
+}