feat(task): run npm run commands with Deno more often (#23794)

Closes #23036

Author: dsherret

cli/tools/task.rs

@@ -28,6 +28,10 @@ use std::path::PathBuf;
 use std::rc::Rc;
 use tokio::task::LocalSet;
 
+// WARNING: Do not depend on this env var in user code. It's not stable API.
+const USE_PKG_JSON_HIDDEN_ENV_VAR_NAME: &str =
+  "DENO_INTERNAL_TASK_USE_PKG_JSON";
+
 pub async fn execute_script(
   flags: Flags,
   task_flags: TaskFlags,
@@ -55,13 +59,20 @@ pub async fn execute_script(
   let npm_resolver = factory.npm_resolver().await?;
   let node_resolver = factory.node_resolver().await?;
   let env_vars = real_env_vars();
+  let force_use_pkg_json = std::env::var_os(USE_PKG_JSON_HIDDEN_ENV_VAR_NAME)
+    .map(|v| {
+      // always remove so sub processes don't inherit this env var
+      std::env::remove_var(USE_PKG_JSON_HIDDEN_ENV_VAR_NAME);
+      v == "1"
+    })
+    .unwrap_or(false);
 
   if let Some(
     deno_config::Task::Definition(script)
     | deno_config::Task::Commented {
       definition: script, ..
     },
-  ) = tasks_config.get(task_name)
+  ) = tasks_config.get(task_name).filter(|_| !force_use_pkg_json)
   {
     let config_file_url = cli_options.maybe_config_file_specifier().unwrap();
     let config_file_path = if config_file_url.scheme() == "file" {
@@ -77,16 +88,18 @@ pub async fn execute_script(
 
     let custom_commands =
       resolve_custom_commands(npm_resolver.as_ref(), node_resolver)?;
-    run_task(
+    run_task(RunTaskOptions {
       task_name,
       script,
-      &cwd,
-      cli_options.initial_cwd(),
+      cwd: &cwd,
+      init_cwd: cli_options.initial_cwd(),
       env_vars,
-      cli_options.argv(),
+      argv: cli_options.argv(),
       custom_commands,
-      npm_resolver.root_node_modules_path().map(|p| p.as_path()),
-    )
+      root_node_modules_dir: npm_resolver
+        .root_node_modules_path()
+        .map(|p| p.as_path()),
+    })
     .await
   } else if package_json_scripts.contains_key(task_name) {
     let package_json_deps_provider = factory.package_json_deps_provider();
@@ -134,18 +147,20 @@ pub async fn execute_script(
     ];
     let custom_commands =
       resolve_custom_commands(npm_resolver.as_ref(), node_resolver)?;
-    for task_name in task_names {
-      if let Some(script) = package_json_scripts.get(&task_name) {
-        let exit_code = run_task(
-          &task_name,
+    for task_name in &task_names {
+      if let Some(script) = package_json_scripts.get(task_name) {
+        let exit_code = run_task(RunTaskOptions {
+          task_name,
           script,
-          &cwd,
-          cli_options.initial_cwd(),
-          env_vars.clone(),
-          cli_options.argv(),
-          custom_commands.clone(),
-          npm_resolver.root_node_modules_path().map(|p| p.as_path()),
-        )
+          cwd: &cwd,
+          init_cwd: cli_options.initial_cwd(),
+          env_vars: env_vars.clone(),
+          argv: cli_options.argv(),
+          custom_commands: custom_commands.clone(),
+          root_node_modules_dir: npm_resolver
+            .root_node_modules_path()
+            .map(|p| p.as_path()),
+        })
         .await?;
         if exit_code > 0 {
           return Ok(exit_code);
@@ -167,25 +182,31 @@ pub async fn execute_script(
   }
 }
 
-#[allow(clippy::too_many_arguments)]
-async fn run_task(
-  task_name: &str,
-  script: &str,
-  cwd: &Path,
-  init_cwd: &Path,
+struct RunTaskOptions<'a> {
+  task_name: &'a str,
+  script: &'a str,
+  cwd: &'a Path,
+  init_cwd: &'a Path,
   env_vars: HashMap<String, String>,
-  argv: &[String],
+  argv: &'a [String],
   custom_commands: HashMap<String, Rc<dyn ShellCommand>>,
-  root_node_modules_dir: Option<&Path>,
-) -> Result<i32, AnyError> {
-  let script = get_script_with_args(script, argv);
-  output_task(task_name, &script);
+  root_node_modules_dir: Option<&'a Path>,
+}
+
+async fn run_task(opts: RunTaskOptions<'_>) -> Result<i32, AnyError> {
+  let script = get_script_with_args(opts.script, opts.argv);
+  output_task(opts.task_name, &script);
   let seq_list = deno_task_shell::parser::parse(&script)
-    .with_context(|| format!("Error parsing script '{}'.", task_name))?;
-  let env_vars = prepare_env_vars(env_vars, init_cwd, root_node_modules_dir);
+    .with_context(|| format!("Error parsing script '{}'.", opts.task_name))?;
+  let env_vars =
+    prepare_env_vars(opts.env_vars, opts.init_cwd, opts.root_node_modules_dir);
   let local = LocalSet::new();
-  let future =
-    deno_task_shell::execute(seq_list, env_vars, cwd, custom_commands);
+  let future = deno_task_shell::execute(
+    seq_list,
+    env_vars,
+    opts.cwd,
+    opts.custom_commands,
+  );
   Ok(local.run_until(future).await)
 }
 
@@ -315,6 +336,48 @@ fn print_available_tasks(
   Ok(())
 }
 
+struct NpmCommand;
+
+impl ShellCommand for NpmCommand {
+  fn execute(
+    &self,
+    mut context: ShellCommandContext,
+  ) -> LocalBoxFuture<'static, ExecuteResult> {
+    if context.args.first().map(|s| s.as_str()) == Some("run")
+      && !context.args.iter().any(|s| s == "--")
+    {
+      if let Some(task_name) = context.args.get(1) {
+        // run with deno task instead
+        let mut args = vec!["task".to_string(), task_name.to_string()];
+        args.extend(context.args.iter().skip(2).cloned());
+        let mut state = context.state;
+        state.apply_env_var(USE_PKG_JSON_HIDDEN_ENV_VAR_NAME, "1");
+        return ExecutableCommand::new(
+          "deno".to_string(),
+          std::env::current_exe().unwrap(),
+        )
+        .execute(ShellCommandContext {
+          args,
+          state,
+          ..context
+        });
+      }
+    }
+
+    // fallback to running the real npm command
+    let npm_path = match context.resolve_command_path("npm") {
+      Ok(path) => path,
+      Err(err) => {
+        let _ = context.stderr.write_line(&format!("{}", err));
+        return Box::pin(futures::future::ready(
+          ExecuteResult::from_exit_code(err.exit_code()),
+        ));
+      }
+    };
+    ExecutableCommand::new("npm".to_string(), npm_path).execute(context)
+  }
+}
+
 struct NpxCommand;
 
 impl ShellCommand for NpxCommand {
@@ -413,15 +476,17 @@ fn resolve_custom_commands(
   npm_resolver: &dyn CliNpmResolver,
   node_resolver: &NodeResolver,
 ) -> Result<HashMap<String, Rc<dyn ShellCommand>>, AnyError> {
-  match npm_resolver.as_inner() {
+  let mut commands = 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))
+      resolve_npm_commands_from_bin_dir(node_modules_dir)
     }
     InnerCliNpmResolverRef::Managed(npm_resolver) => {
-      resolve_managed_npm_commands(npm_resolver, node_resolver)
+      resolve_managed_npm_commands(npm_resolver, node_resolver)?
     }
-  }
+  };
+  commands.insert("npm".to_string(), Rc::new(NpmCommand));
+  Ok(commands)
 }
 
 fn resolve_npm_commands_from_bin_dir(

tests/specs/task/npm_run/__test__.jsonc

@@ -0,0 +1,17 @@
+{
+  "tests": {
+    "uses_deno_no_flags": {
+      "args": "task test",
+      "output": "task_test.out"
+    },
+    "uses_npm_flags": {
+      "args": "task test_using_npm",
+      "output": "task_test_using_npm.out",
+      "exitCode": 1
+    },
+    "npm_run": {
+      "args": "task npm_run",
+      "output": "task_npm_run.out"
+    }
+  }
+}

tests/specs/task/npm_run/deno.jsonc

@@ -0,0 +1,11 @@
+{
+  "tasks": {
+    "echo": "echo 'Hello, World!'",
+    // should use the task from package.json and not the one above
+    "test": "npm run echo hi there",
+    // currently this will execute using the actual `npm run` because we
+    // haven't implemented the flags for `npm run` yet
+    "test_using_npm": "npm run non_existent -- --ignore-scripts",
+    "npm_run": "npm run"
+  }
+}

tests/specs/task/npm_run/package.json

@@ -0,0 +1,5 @@
+{
+  "scripts": {
+    "echo": "echo"
+  }
+}

tests/specs/task/npm_run/task_npm_run.out

@@ -0,0 +1,2 @@
+Task npm_run npm run
+Scripts available [WILDCARD]

tests/specs/task/npm_run/task_test.out

@@ -0,0 +1,3 @@
+Task test npm run echo hi there
+Task echo echo "hi" "there"
+hi there

tests/specs/task/npm_run/task_test_using_npm.out

@@ -0,0 +1,3 @@
+Task test_using_npm npm run non_existent -- --ignore-scripts
+npm [WILDLINE] Missing script: "non_existent"
+[WILDCARD]