fix(core/shell): speedup Command.execute & fix extra new lines (#9796)

* fix(core/shell): speedup `Command.execute` & fix extra new lines (#9706)

* fix(core/shell): speedup `Command.execute` & fix extra new lines

The speed gains comes from running the Command in Rust fully and returning the result in one go instead of using events.

The extra new lines was a regression from https://github.com/tauri-apps/tauri/pull/6519

ref: https://github.com/tauri-apps/tauri/issues/7684#issuecomment-2100897383

* fix unix build

* clippy

* cleanup

---------

Co-authored-by: Lucas Nogueira <lucas@tauri.app>

* lock file

* minor

---------

Co-authored-by: Lucas Nogueira <lucas@tauri.app>
Co-authored-by: Lucas Nogueira <lucas@tauri.studio>

.changes/shell-execute-extra-newline.md

@@ -0,0 +1,6 @@
+---
+"@tauri-apps/api": "patch:bug"
+---
+
+Fix The JS `Command.execute` API from `shell` module including extra new lines.
+

.changes/shell-execute-performance.md

@@ -0,0 +1,7 @@
+---
+"tauri": "minor:enhance"
+"@tauri-apps/api": "minor:enhance"
+---
+
+Enhance the speed of The JS `Command.execute` API from `shell` module.
+

core/tauri/src/endpoints/shell.rs

@@ -8,7 +8,7 @@ use super::InvokeContext;
 use crate::{api::ipc::CallbackFn, Runtime};
 #[cfg(shell_scope)]
 use crate::{Manager, Scopes};
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use tauri_macros::{command_enum, module_command_handler, CommandModule};
 
 #[cfg(shell_scope)]
@@ -63,6 +63,15 @@ pub struct CommandOptions {
 #[derive(Deserialize, CommandModule)]
 #[serde(tag = "cmd", rename_all = "camelCase")]
 pub enum Cmd {
+  /// The execute and return script API.
+  #[cmd(shell_script, "shell > execute or shell > sidecar")]
+  #[serde(rename_all = "camelCase")]
+  ExecuteAndReturn {
+    program: String,
+    args: ExecuteArgs,
+    #[serde(default)]
+    options: CommandOptions,
+  },
   /// The execute script API.
   #[cmd(shell_script, "shell > execute or shell > sidecar")]
   #[serde(rename_all = "camelCase")]
@@ -81,9 +90,58 @@ pub enum Cmd {
   Open { path: String, with: Option<String> },
 }
 
+#[derive(Serialize)]
+#[cfg(any(shell_execute, shell_sidecar))]
+struct ChildProcessReturn {
+  code: Option<i32>,
+  signal: Option<i32>,
+  stdout: String,
+  stderr: String,
+}
+
 impl Cmd {
   #[module_command_handler(shell_script)]
-  #[allow(unused_variables)]
+  fn execute_and_return<R: Runtime>(
+    context: InvokeContext<R>,
+    program: String,
+    args: ExecuteArgs,
+    options: CommandOptions,
+  ) -> super::Result<ChildProcessReturn> {
+    let encoding = options
+      .encoding
+      .as_ref()
+      .and_then(|encoding| crate::api::process::Encoding::for_label(encoding.as_bytes()));
+    let command = prepare_cmd(&context, &program, args, options)?;
+
+    let mut command: std::process::Command = command.into();
+    let output = command.output()?;
+
+    let (stdout, stderr) = match encoding {
+      Some(encoding) => (
+        encoding.decode_with_bom_removal(&output.stdout).0.into(),
+        encoding.decode_with_bom_removal(&output.stderr).0.into(),
+      ),
+      None => (
+        String::from_utf8(output.stdout)?,
+        String::from_utf8(output.stderr)?,
+      ),
+    };
+
+    #[cfg(unix)]
+    use std::os::unix::process::ExitStatusExt;
+
+    Ok(ChildProcessReturn {
+      code: output.status.code(),
+      #[cfg(windows)]
+      signal: None,
+      #[cfg(unix)]
+      signal: output.status.signal(),
+      stdout,
+      stderr,
+    })
+  }
+
+  #[module_command_handler(shell_script)]
   fn execute<R: Runtime>(
     context: InvokeContext<R>,
     program: String,
@@ -91,91 +149,29 @@ impl Cmd {
     on_event_fn: CallbackFn,
     options: CommandOptions,
   ) -> super::Result<ChildId> {
-    let mut command = if options.sidecar {
-      #[cfg(not(shell_sidecar))]
-      return Err(crate::Error::ApiNotAllowlisted("shell > sidecar".to_string()).into_anyhow());
-      #[cfg(shell_sidecar)]
-      {
-        let program = PathBuf::from(program);
-        let program_as_string = program.display().to_string();
-        let program_no_ext_as_string = program.with_extension("").display().to_string();
-        let configured_sidecar = context
-          .config
-          .tauri
-          .bundle
-          .external_bin
-          .as_ref()
-          .map(|bins| {
-            bins
-              .iter()
-              .find(|b| b == &&program_as_string || b == &&program_no_ext_as_string)
-          })
-          .unwrap_or_default();
-        if let Some(sidecar) = configured_sidecar {
-          context
-            .window
-            .state::<Scopes>()
-            .shell
-            .prepare_sidecar(&program.to_string_lossy(), sidecar, args)
-            .map_err(crate::error::into_anyhow)?
-        } else {
-          return Err(crate::Error::SidecarNotAllowed(program).into_anyhow());
-        }
-      }
-    } else {
-      #[cfg(not(shell_execute))]
-      return Err(crate::Error::ApiNotAllowlisted("shell > execute".to_string()).into_anyhow());
-      #[cfg(shell_execute)]
-      match context
-        .window
-        .state::<Scopes>()
-        .shell
-        .prepare(&program, args)
-      {
-        Ok(cmd) => cmd,
-        Err(e) => {
-          #[cfg(debug_assertions)]
-          eprintln!("{e}");
-          return Err(crate::Error::ProgramNotAllowed(PathBuf::from(program)).into_anyhow());
-        }
-      }
-    };
-    #[cfg(any(shell_execute, shell_sidecar))]
-    {
-      if let Some(cwd) = options.cwd {
-        command = command.current_dir(cwd);
-      }
-      if let Some(env) = options.env {
-        command = command.envs(env);
-      } else {
-        command = command.env_clear();
-      }
-      if let Some(encoding) = options.encoding {
-        if let Some(encoding) = crate::api::process::Encoding::for_label(encoding.as_bytes()) {
-          command = command.encoding(encoding);
-        } else {
-          return Err(anyhow::anyhow!(format!("unknown encoding {encoding}")));
-        }
-      }
-      let (mut rx, child) = command.spawn()?;
+    use std::future::Future;
+    use std::pin::Pin;
 
-      let pid = child.pid();
-      command_child_store().lock().unwrap().insert(pid, child);
+    let command = prepare_cmd(&context, &program, args, options)?;
 
-      crate::async_runtime::spawn(async move {
-        while let Some(event) = rx.recv().await {
-          if matches!(event, crate::api::process::CommandEvent::Terminated(_)) {
-            command_child_store().lock().unwrap().remove(&pid);
-          }
-          let js = crate::api::ipc::format_callback(on_event_fn, &event)
-            .expect("unable to serialize CommandEvent");
+    let (mut rx, child) = command.spawn()?;
 
-          let _ = context.window.eval(js.as_str());
+    let pid = child.pid();
+    command_child_store().lock().unwrap().insert(pid, child);
+
+    crate::async_runtime::spawn(async move {
+      while let Some(event) = rx.recv().await {
+        if matches!(event, crate::api::process::CommandEvent::Terminated(_)) {
+          command_child_store().lock().unwrap().remove(&pid);
         }
-      });
+        let js = crate::api::ipc::format_callback(on_event_fn, &event)
+          .expect("unable to serialize CommandEvent");
 
-      Ok(pid)
-    }
+        let _ = context.window.eval(js.as_str());
+      }
+    });
+
+    Ok(pid)
   }
 
   #[module_command_handler(shell_script)]
@@ -226,6 +222,81 @@ impl Cmd {
   }
 }
 
+fn prepare_cmd<R: Runtime>(
+  context: &InvokeContext<R>,
+  program: &String,
+  args: ExecuteArgs,
+  options: CommandOptions,
+) -> super::Result<crate::api::process::Command> {
+  let mut command = if options.sidecar {
+    #[cfg(not(shell_sidecar))]
+    return Err(crate::Error::ApiNotAllowlisted("shell > sidecar".to_string()).into_anyhow());
+    #[cfg(shell_sidecar)]
+    {
+      let program = PathBuf::from(program);
+      let program_as_string = program.display().to_string();
+      let program_no_ext_as_string = program.with_extension("").display().to_string();
+      let configured_sidecar = context
+        .config
+        .tauri
+        .bundle
+        .external_bin
+        .as_ref()
+        .map(|bins| {
+          bins
+            .iter()
+            .find(|b| b == &&program_as_string || b == &&program_no_ext_as_string)
+        })
+        .unwrap_or_default();
+      if let Some(sidecar) = configured_sidecar {
+        context
+          .window
+          .state::<Scopes>()
+          .shell
+          .prepare_sidecar(&program.to_string_lossy(), sidecar, args)
+          .map_err(crate::error::into_anyhow)
+      } else {
+        Err(crate::Error::SidecarNotAllowed(program).into_anyhow())
+      }
+    }
+  } else {
+    #[cfg(not(shell_execute))]
+    return Err(crate::Error::ApiNotAllowlisted("shell > execute".to_string()).into_anyhow());
+    #[cfg(shell_execute)]
+    match context
+      .window
+      .state::<Scopes>()
+      .shell
+      .prepare(program, args)
+    {
+      Ok(cmd) => Ok(cmd),
+      Err(e) => {
+        #[cfg(debug_assertions)]
+        eprintln!("{e}");
+        Err(crate::Error::ProgramNotAllowed(PathBuf::from(program)).into_anyhow())
+      }
+    }
+  }?;
+
+  if let Some(cwd) = options.cwd {
+    command = command.current_dir(cwd);
+  }
+  if let Some(env) = options.env {
+    command = command.envs(env);
+  } else {
+    command = command.env_clear();
+  }
+  if let Some(encoding) = &options.encoding {
+    if let Some(encoding) = crate::api::process::Encoding::for_label(encoding.as_bytes()) {
+      command = command.encoding(encoding);
+    } else {
+      return Err(anyhow::anyhow!(format!("unknown encoding {encoding}")));
+    }
+  }
+
+  Ok(command)
+}
+
 #[cfg(test)]
 mod tests {
   use super::{Buffer, ChildId, CommandOptions, ExecuteArgs};

tooling/api/src/shell.ts

@@ -115,38 +115,6 @@ interface ChildProcess {
   stderr: string
 }
 
-/**
- * Spawns a process.
- *
- * @ignore
- * @param program The name of the scoped command.
- * @param onEvent Event handler.
- * @param args Program arguments.
- * @param options Configuration for the process spawn.
- * @returns A promise resolving to the process id.
- */
-async function execute(
-  onEvent: (event: CommandEvent) => void,
-  program: string,
-  args: string | string[] = [],
-  options?: InternalSpawnOptions
-): Promise<number> {
-  if (typeof args === 'object') {
-    Object.freeze(args)
-  }
-
-  return invokeTauriCommand<number>({
-    __tauriModule: 'Shell',
-    message: {
-      cmd: 'execute',
-      program,
-      args,
-      options,
-      onEventFn: transformCallback(onEvent)
-    }
-  })
-}
-
 /**
  * @since 1.0.0
  */
@@ -449,27 +417,41 @@ class Command extends EventEmitter<'close' | 'error'> {
    * @returns A promise resolving to the child process handle.
    */
   async spawn(): Promise<Child> {
-    return execute(
-      (event) => {
-        switch (event.event) {
-          case 'Error':
-            this.emit('error', event.payload)
-            break
-          case 'Terminated':
-            this.emit('close', event.payload)
-            break
-          case 'Stdout':
-            this.stdout.emit('data', event.payload)
-            break
-          case 'Stderr':
-            this.stderr.emit('data', event.payload)
-            break
-        }
-      },
-      this.program,
-      this.args,
-      this.options
-    ).then((pid) => new Child(pid))
+    const program = this.program
+    const args = this.args
+    const options = this.options
+
+    if (typeof args === 'object') {
+      Object.freeze(args)
+    }
+
+    const onEvent = (event: CommandEvent) => {
+      switch (event.event) {
+        case 'Error':
+          this.emit('error', event.payload)
+          break
+        case 'Terminated':
+          this.emit('close', event.payload)
+          break
+        case 'Stdout':
+          this.stdout.emit('data', event.payload)
+          break
+        case 'Stderr':
+          this.stderr.emit('data', event.payload)
+          break
+      }
+    }
+
+    return invokeTauriCommand<number>({
+      __tauriModule: 'Shell',
+      message: {
+        cmd: 'execute',
+        program,
+        args,
+        options,
+        onEventFn: transformCallback(onEvent)
+      }
+    }).then((pid) => new Child(pid))
   }
 
   /**
@@ -487,25 +469,22 @@ class Command extends EventEmitter<'close' | 'error'> {
    * @returns A promise resolving to the child process output.
    */
   async execute(): Promise<ChildProcess> {
-    return new Promise((resolve, reject) => {
-      this.on('error', reject)
-      const stdout: string[] = []
-      const stderr: string[] = []
-      this.stdout.on('data', (line: string) => {
-        stdout.push(line)
-      })
-      this.stderr.on('data', (line: string) => {
-        stderr.push(line)
-      })
-      this.on('close', (payload: TerminatedPayload) => {
-        resolve({
-          code: payload.code,
-          signal: payload.signal,
-          stdout: stdout.join('\n'),
-          stderr: stderr.join('\n')
-        })
-      })
-      this.spawn().catch(reject)
+    const program = this.program
+    const args = this.args
+    const options = this.options
+
+    if (typeof args === 'object') {
+      Object.freeze(args)
+    }
+
+    return invokeTauriCommand<ChildProcess>({
+      __tauriModule: 'Shell',
+      message: {
+        cmd: 'executeAndReturn',
+        program,
+        args,
+        options
+      }
     })
   }
 }