feat(task): support running npm binary commands in deno.json (#23478)

npm binary commands like `vite` from a `node_modules/.bin` folder will
now execute when defined in a deno.json

Closes https://github.com/denoland/deno/issues/23477

cli/tools/task.rs

@@ -48,6 +48,8 @@ pub async fn execute_script(
       return Ok(1);
     }
   };
+  let npm_resolver = factory.npm_resolver().await?;
+  let node_resolver = factory.node_resolver().await?;
 
   if let Some(
     deno_config::Task::Definition(script)
@@ -66,20 +68,20 @@ pub async fn execute_script(
       Some(path) => canonicalize_path(&PathBuf::from(path))?,
       None => config_file_path.parent().unwrap().to_owned(),
     };
-    let script = get_script_with_args(script, cli_options);
-    output_task(task_name, &script);
-    let seq_list = deno_task_shell::parser::parse(&script)
-      .with_context(|| format!("Error parsing script '{task_name}'."))?;
-    let env_vars = collect_env_vars();
-    let local = LocalSet::new();
-    let future =
-      deno_task_shell::execute(seq_list, env_vars, &cwd, Default::default());
-    let exit_code = local.run_until(future).await;
-    Ok(exit_code)
+
+    let npm_commands =
+      resolve_npm_commands(npm_resolver.as_ref(), node_resolver)?;
+    run_task(
+      task_name,
+      script,
+      &cwd,
+      cli_options,
+      npm_commands,
+      npm_resolver.as_ref(),
+    )
+    .await
   } else if package_json_scripts.contains_key(task_name) {
     let package_json_deps_provider = factory.package_json_deps_provider();
-    let npm_resolver = factory.npm_resolver().await?;
-    let node_resolver = factory.node_resolver().await?;
 
     if let Some(package_deps) = package_json_deps_provider.deps() {
       for (key, value) in package_deps {
@@ -122,30 +124,19 @@ pub async fn execute_script(
       task_name.clone(),
       format!("post{}", task_name),
     ];
+    let npm_commands =
+      resolve_npm_commands(npm_resolver.as_ref(), node_resolver)?;
     for task_name in task_names {
       if let Some(script) = package_json_scripts.get(&task_name) {
-        let script = get_script_with_args(script, cli_options);
-        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 = match npm_resolver.as_inner() {
-          InnerCliNpmResolverRef::Managed(npm_resolver) => {
-            resolve_npm_commands(npm_resolver, node_resolver)?
-          }
-          InnerCliNpmResolverRef::Byonm(npm_resolver) => {
-            let node_modules_dir =
-              npm_resolver.root_node_modules_path().unwrap();
-            resolve_npm_commands_from_bin_dir(node_modules_dir)?
-          }
-        };
-        let env_vars = match npm_resolver.root_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);
-        let exit_code = local.run_until(future).await;
+        let exit_code = run_task(
+          &task_name,
+          script,
+          &cwd,
+          cli_options,
+          npm_commands.clone(),
+          npm_resolver.as_ref(),
+        )
+        .await?;
         if exit_code > 0 {
           return Ok(exit_code);
         }
@@ -160,6 +151,27 @@ pub async fn execute_script(
   }
 }
 
+async fn run_task(
+  task_name: &str,
+  script: &str,
+  cwd: &Path,
+  cli_options: &CliOptions,
+  npm_commands: HashMap<String, Rc<dyn ShellCommand>>,
+  npm_resolver: &dyn CliNpmResolver,
+) -> Result<i32, AnyError> {
+  let script = get_script_with_args(script, cli_options);
+  output_task(task_name, &script);
+  let seq_list = deno_task_shell::parser::parse(&script)
+    .with_context(|| format!("Error parsing script '{}'.", task_name))?;
+  let env_vars = match npm_resolver.root_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, npm_commands);
+  Ok(local.run_until(future).await)
+}
+
 fn get_script_with_args(script: &str, options: &CliOptions) -> String {
   let additional_args = options
     .argv()
@@ -358,9 +370,24 @@ impl ShellCommand for NodeModulesFileRunCommand {
   }
 }
 
+fn resolve_npm_commands(
+  npm_resolver: &dyn CliNpmResolver,
+  node_resolver: &NodeResolver,
+) -> Result<HashMap<String, Rc<dyn ShellCommand>>, AnyError> {
+  match npm_resolver.as_inner() {
+    InnerCliNpmResolverRef::Byonm(npm_resolver) => {
+      let node_modules_dir = npm_resolver.root_node_modules_path().unwrap();
+      Ok(resolve_npm_commands_from_bin_dir(node_modules_dir))
+    }
+    InnerCliNpmResolverRef::Managed(npm_resolver) => {
+      resolve_managed_npm_commands(npm_resolver, node_resolver)
+    }
+  }
+}
+
 fn resolve_npm_commands_from_bin_dir(
   node_modules_dir: &Path,
-) -> Result<HashMap<String, Rc<dyn ShellCommand>>, AnyError> {
+) -> HashMap<String, Rc<dyn ShellCommand>> {
   let mut result = HashMap::<String, Rc<dyn ShellCommand>>::new();
   let bin_dir = node_modules_dir.join(".bin");
   log::debug!("Resolving commands in '{}'.", bin_dir.display());
@@ -379,7 +406,7 @@ fn resolve_npm_commands_from_bin_dir(
       log::debug!("Failed read_dir for '{}': {:#}", bin_dir.display(), err);
     }
   }
-  Ok(result)
+  result
 }
 
 fn resolve_bin_dir_entry_command(
@@ -436,7 +463,7 @@ fn resolve_execution_path_from_npx_shim(
   }
 }
 
-fn resolve_npm_commands(
+fn resolve_managed_npm_commands(
   npm_resolver: &ManagedCliNpmResolver,
   node_resolver: &NodeResolver,
 ) -> Result<HashMap<String, Rc<dyn ShellCommand>>, AnyError> {

tests/integration/task_tests.rs

@@ -3,11 +3,9 @@
 // Most of the tests for this are in deno_task_shell.
 // These tests are intended to only test integration.
 
-use deno_core::serde_json::json;
 use test_util::env_vars_for_npm_tests;
 use test_util::itest;
 use test_util::TestContext;
-use test_util::TestContextBuilder;
 
 itest!(task_no_args {
   args: "task -q --config task/deno_json/deno.json",
@@ -302,57 +300,3 @@ itest!(task_deno_no_pre_post {
   exit_code: 0,
   envs: vec![("NO_COLOR".to_string(), "1".to_string())],
 });
-
-#[test]
-fn task_byonm() {
-  let context = TestContextBuilder::for_npm().use_temp_cwd().build();
-  let temp_dir = context.temp_dir().path();
-  temp_dir.join("package.json").write_json(&json!({
-    "name": "example",
-    "scripts": {
-      "say": "cowsay 'do make say'",
-      "think": "cowthink think"
-    },
-    "dependencies": {
-      "cowsay": "*"
-    }
-  }));
-  temp_dir.join("deno.json").write_json(&json!({
-    "unstable": ["byonm"],
-  }));
-  context.run_npm("install");
-
-  context
-    .new_command()
-    .args_vec(["task", "say"])
-    .run()
-    .assert_matches_text(
-      r#"Task say cowsay 'do make say'
- _____________
-< do make say >
- -------------
-        \   ^__^
-         \  (oo)\_______
-            (__)\       )\/\
-                ||----w |
-                ||     ||
-"#,
-    );
-
-  context
-    .new_command()
-    .args_vec(["task", "think"])
-    .run()
-    .assert_matches_text(
-      r#"Task think cowthink think
- _______
-( think )
- -------
-        o   ^__^
-         o  (oo)\_______
-            (__)\       )\/\
-                ||----w |
-                ||     ||
-"#,
-    );
-}

tests/specs/task/byonm/__test__.jsonc

@@ -0,0 +1,20 @@
+{
+  "tempDir": true,
+  "steps": [{
+    "commandName": "npm",
+    "args": "install",
+    "output": "[WILDCARD]"
+  }, {
+    "args": "task say",
+    "output": "package_json_say.out"
+  }, {
+    "args": "task think",
+    "output": "package_json_think.out"
+  }, {
+    "args": "task deno-say",
+    "output": "deno_json_say.out"
+  }, {
+    "args": "task deno-think",
+    "output": "deno_json_think.out"
+  }]
+}

tests/specs/task/byonm/deno.json

@@ -0,0 +1,7 @@
+{
+  "tasks": {
+    "deno-say": "cowsay 'do make say'",
+    "deno-think": "cowthink think"
+  },
+  "unstable": ["byonm"]
+}

tests/specs/task/byonm/deno_json_say.out

@@ -0,0 +1,9 @@
+Task deno-say cowsay 'do make say'
+ _____________
+< do make say >
+ -------------
+        \   ^__^
+         \  (oo)\_______
+            (__)\       )\/\
+                ||----w |
+                ||     ||

tests/specs/task/byonm/deno_json_think.out

@@ -0,0 +1,9 @@
+Task deno-think cowthink think
+ _______
+( think )
+ -------
+        o   ^__^
+         o  (oo)\_______
+            (__)\       )\/\
+                ||----w |
+                ||     ||

tests/specs/task/byonm/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "example",
+  "scripts": {
+    "say": "cowsay 'do make say'",
+    "think": "cowthink think"
+  },
+  "dependencies": {
+    "cowsay": "*"
+  }
+}

tests/specs/task/byonm/package_json_say.out

@@ -0,0 +1,9 @@
+Task say cowsay 'do make say'
+ _____________
+< do make say >
+ -------------
+        \   ^__^
+         \  (oo)\_______
+            (__)\       )\/\
+                ||----w |
+                ||     ||

tests/specs/task/byonm/package_json_think.out

@@ -0,0 +1,9 @@
+Task think cowthink think
+ _______
+( think )
+ -------
+        o   ^__^
+         o  (oo)\_______
+            (__)\       )\/\
+                ||----w |
+                ||     ||