feat(unstable): rework Deno.Command (#16812)

Refactors the `Deno.Command` class to not handle any state, but only being an intermediary to calling its methods, and as such any methods and properties besides `output`, `outputSync` & `spawn` have been removed. Interracting with a `spawn`ed subprocess now works by using the methods and properties on the returned class of the `spawn` method.

cli/tests/unit/command_test.ts

@@ -31,13 +31,13 @@ tryExit();
 
     Deno.writeFileSync(`${cwd}/${programFile}`, enc.encode(program));
 
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       cwd,
       args: ["run", "--allow-read", programFile],
       stdout: "inherit",
       stderr: "inherit",
     });
-    child.spawn();
+    const child = command.spawn();
 
     // Write the expected exit code *after* starting deno.
     // This is how we verify that `Child` is actually asynchronous.
@@ -55,7 +55,7 @@ tryExit();
 Deno.test(
   { permissions: { run: true, read: true } },
   async function commandStdinPiped() {
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: [
         "eval",
         "if (new TextDecoder().decode(await Deno.readAll(Deno.stdin)) !== 'hello') throw new Error('Expected \\'hello\\'')",
@@ -64,7 +64,7 @@ Deno.test(
       stdout: "null",
       stderr: "null",
     });
-    child.spawn();
+    const child = command.spawn();
 
     assertThrows(() => child.stdout, TypeError, "stdout is not piped");
     assertThrows(() => child.stderr, TypeError, "stderr is not piped");
@@ -85,14 +85,14 @@ Deno.test(
 Deno.test(
   { permissions: { run: true, read: true } },
   async function commandStdoutPiped() {
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: [
         "eval",
         "await Deno.stdout.write(new TextEncoder().encode('hello'))",
       ],
       stderr: "null",
     });
-    child.spawn();
+    const child = command.spawn();
 
     assertThrows(() => child.stdin, TypeError, "stdin is not piped");
     assertThrows(() => child.stderr, TypeError, "stderr is not piped");
@@ -118,14 +118,14 @@ Deno.test(
 Deno.test(
   { permissions: { run: true, read: true } },
   async function commandStderrPiped() {
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: [
         "eval",
         "await Deno.stderr.write(new TextEncoder().encode('hello'))",
       ],
       stdout: "null",
     });
-    child.spawn();
+    const child = command.spawn();
 
     assertThrows(() => child.stdin, TypeError, "stdin is not piped");
     assertThrows(() => child.stdout, TypeError, "stdout is not piped");
@@ -158,13 +158,13 @@ Deno.test(
       write: true,
     });
 
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: [
         "eval",
         "Deno.stderr.write(new TextEncoder().encode('error\\n')); Deno.stdout.write(new TextEncoder().encode('output\\n'));",
       ],
     });
-    child.spawn();
+    const child = command.spawn();
     await child.stdout.pipeTo(file.writable, {
       preventClose: true,
     });
@@ -189,7 +189,7 @@ Deno.test(
     await Deno.writeFile(fileName, encoder.encode("hello"));
     const file = await Deno.open(fileName);
 
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: [
         "eval",
         "if (new TextDecoder().decode(await Deno.readAll(Deno.stdin)) !== 'hello') throw new Error('Expected \\'hello\\'')",
@@ -198,7 +198,7 @@ Deno.test(
       stdout: "null",
       stderr: "null",
     });
-    child.spawn();
+    const child = command.spawn();
     await file.readable.pipeTo(child.stdin, {
       preventClose: true,
     });
@@ -212,12 +212,12 @@ Deno.test(
 Deno.test(
   { permissions: { run: true, read: true } },
   async function commandKillSuccess() {
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: ["eval", "setTimeout(() => {}, 10000)"],
       stdout: "null",
       stderr: "null",
     });
-    child.spawn();
+    const child = command.spawn();
 
     child.kill("SIGKILL");
     const status = await child.status;
@@ -236,12 +236,12 @@ Deno.test(
 Deno.test(
   { permissions: { run: true, read: true } },
   async function commandKillFailed() {
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: ["eval", "setTimeout(() => {}, 5000)"],
       stdout: "null",
       stderr: "null",
     });
-    child.spawn();
+    const child = command.spawn();
 
     assertThrows(() => {
       // @ts-expect-error testing runtime error of bad signal
@@ -255,12 +255,12 @@ Deno.test(
 Deno.test(
   { permissions: { run: true, read: true } },
   async function commandKillOptional() {
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: ["eval", "setTimeout(() => {}, 10000)"],
       stdout: "null",
       stderr: "null",
     });
-    child.spawn();
+    const child = command.spawn();
 
     child.kill();
     const status = await child.status;
@@ -280,7 +280,7 @@ Deno.test(
   { permissions: { run: true, read: true } },
   async function commandAbort() {
     const ac = new AbortController();
-    const child = new Deno.Command(Deno.execPath(), {
+    const command = new Deno.Command(Deno.execPath(), {
       args: [
         "eval",
         "setTimeout(console.log, 1e8)",
@@ -289,7 +289,7 @@ Deno.test(
       stdout: "null",
       stderr: "null",
     });
-    child.spawn();
+    const child = command.spawn();
     queueMicrotask(() => ac.abort());
     const status = await child.status;
     assertEquals(status.success, false);
@@ -735,11 +735,12 @@ Deno.test(
 
     const programFile = "unref.ts";
     const program = `
-const child = await new Deno.Command(Deno.execPath(), {
+const command = await new Deno.Command(Deno.execPath(), {
   cwd: Deno.args[0],
   stdout: "piped",
   args: ["run", "-A", "--unstable", Deno.args[1]],
-});child.spawn();
+});
+const child = command.spawn();
 const readable = child.stdout.pipeThrough(new TextDecoderStream());
 const reader = readable.getReader();
 // set up an interval that will end after reading a few messages from stdout,

cli/tsc/diagnostics.rs

@@ -31,6 +31,7 @@ const UNSTABLE_DENO_PROPS: &[&str] = &[
   "umask",
   "spawnChild",
   "Child",
+  "ChildProcess",
   "spawn",
   "spawnSync",
   "SpawnOptions",

cli/tsc/dts/lib.deno.unstable.d.ts

@@ -1639,14 +1639,14 @@ declare namespace Deno {
    *   ],
    *   stdin: "piped",
    * });
-   * command.spawn();
+   * const child = command.spawn();
    *
    * // open a file and pipe the subprocess output to it.
-   * command.stdout.pipeTo(Deno.openSync("output").writable);
+   * child.stdout.pipeTo(Deno.openSync("output").writable);
    *
    * // manually close stdin
-   * command.stdin.close();
-   * const status = await command.status;
+   * child.stdin.close();
+   * const status = await child.status;
    * ```
    *
    * ```ts
@@ -1678,13 +1678,6 @@ declare namespace Deno {
    * @category Sub Process
    */
   export class Command {
-    get stdin(): WritableStream<Uint8Array>;
-    get stdout(): ReadableStream<Uint8Array>;
-    get stderr(): ReadableStream<Uint8Array>;
-    readonly pid: number;
-    /** Get the status of the child process. */
-    readonly status: Promise<CommandStatus>;
-
     constructor(command: string | URL, options?: CommandOptions);
     /**
      * Executes the {@linkcode Deno.Command}, waiting for it to finish and
@@ -1711,8 +1704,27 @@ declare namespace Deno {
     /**
      * Spawns a streamable subprocess, allowing to use the other methods.
      */
-    spawn(): void;
+    spawn(): ChildProcess;
+  }
 
+  /** **UNSTABLE**: New API, yet to be vetted.
+   *
+   * The interface for handling a child process returned from
+   * {@linkcode Deno.Command.spawn}.
+   *
+   * @category Sub Process
+   */
+  export class ChildProcess {
+    get stdin(): WritableStream<Uint8Array>;
+    get stdout(): ReadableStream<Uint8Array>;
+    get stderr(): ReadableStream<Uint8Array>;
+    readonly pid: number;
+    /** Get the status of the child. */
+    readonly status: Promise<CommandStatus>;
+
+    /** Waits for the child to exit completely, returning all its output and
+     * status. */
+    output(): Promise<CommandOutput>;
     /** Kills the process with given {@linkcode Deno.Signal}. Defaults to
      * `"SIGTERM"`. */
     kill(signo?: Signal): void;

runtime/js/40_spawn.js

@@ -277,136 +277,44 @@
     };
   }
 
-  class Command {
-    #command;
-    #options;
+  function createCommand(spawn, spawnSync, spawnChild) {
+    return class Command {
+      #command;
+      #options;
 
-    #child;
+      constructor(command, options) {
+        this.#command = command;
+        this.#options = options;
+      }
 
-    #consumed;
-
-    constructor(command, options) {
-      this.#command = command;
-      this.#options = options;
-    }
-
-    output() {
-      if (this.#child) {
-        return this.#child.output();
-      } else {
-        if (this.#consumed) {
-          throw new TypeError(
-            "Command instance is being or has already been consumed.",
-          );
-        }
+      output() {
         if (this.#options?.stdin === "piped") {
           throw new TypeError(
             "Piped stdin is not supported for this function, use 'Deno.Command.spawn()' instead",
           );
         }
-
-        this.#consumed = true;
-        return Deno.spawn(this.#command, this.#options);
-      }
-    }
-
-    outputSync() {
-      if (this.#consumed) {
-        throw new TypeError(
-          "Command instance is being or has already been consumed.",
-        );
-      }
-      if (this.#child) {
-        throw new TypeError("Was spawned");
-      }
-      if (this.#options?.stdin === "piped") {
-        throw new TypeError(
-          "Piped stdin is not supported for this function, use 'Deno.Command.spawn()' instead",
-        );
+        return spawn(this.#command, this.#options);
       }
 
-      this.#consumed = true;
-      return Deno.spawnSync(this.#command, this.#options);
-    }
-
-    spawn() {
-      if (this.#consumed) {
-        throw new TypeError(
-          "Command instance is being or has already been consumed.",
-        );
+      outputSync() {
+        if (this.#options?.stdin === "piped") {
+          throw new TypeError(
+            "Piped stdin is not supported for this function, use 'Deno.Command.spawn()' instead",
+          );
+        }
+        return spawnSync(this.#command, this.#options);
       }
 
-      this.#consumed = true;
-      this.#child = Deno.spawnChild(this.#command, this.#options);
-    }
-
-    get stdin() {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
+      spawn() {
+        return spawnChild(this.#command, this.#options);
       }
-
-      return this.#child.stdin;
-    }
-
-    get stdout() {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
-      }
-
-      return this.#child.stdout;
-    }
-
-    get stderr() {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
-      }
-
-      return this.#child.stderr;
-    }
-
-    get status() {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
-      }
-
-      return this.#child.status;
-    }
-
-    get pid() {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
-      }
-
-      return this.#child.pid;
-    }
-
-    kill(signo = "SIGTERM") {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
-      }
-      this.#child.kill(signo);
-    }
-
-    ref() {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
-      }
-
-      this.#child.ref();
-    }
-
-    unref() {
-      if (!this.#child) {
-        throw new TypeError("Wasn't spawned");
-      }
-
-      this.#child.unref();
-    }
+    };
   }
 
   window.__bootstrap.spawn = {
     Child,
-    Command,
+    ChildProcess: Child,
+    createCommand,
     createSpawn,
     createSpawnChild,
     createSpawnSync,

runtime/js/90_deno_ns.js

@@ -144,6 +144,7 @@
     funlock: __bootstrap.fs.funlock,
     funlockSync: __bootstrap.fs.funlockSync,
     Child: __bootstrap.spawn.Child,
+    ChildProcess: __bootstrap.spawn.ChildProcess,
     spawnChild: __bootstrap.spawn.spawnChild,
     spawn: __bootstrap.spawn.spawn,
     spawnSync: __bootstrap.spawn.spawnSync,

runtime/js/99_main.js

@@ -482,6 +482,14 @@ delete Intl.v8BreakIterator;
       },
     });
 
+    ObjectAssign(internals.nodeUnstable, {
+      Command: __bootstrap.spawn.createCommand(
+        internals.nodeUnstable.spawn,
+        internals.nodeUnstable.spawnSync,
+        internals.nodeUnstable.spawnChild,
+      ),
+    });
+
     const finalDenoNs = {
       core,
       internal: internalSymbol,
@@ -513,6 +521,14 @@ delete Intl.v8BreakIterator;
           ops.op_net_listen_unixpacket,
         ),
       });
+
+      ObjectAssign(finalDenoNs, {
+        Command: __bootstrap.spawn.createCommand(
+          finalDenoNs.spawn,
+          finalDenoNs.spawnSync,
+          finalDenoNs.spawnChild,
+        ),
+      });
     }
 
     // Setup `Deno` global - we're actually overriding already existing global
@@ -617,6 +633,14 @@ delete Intl.v8BreakIterator;
       },
     });
 
+    ObjectAssign(internals.nodeUnstable, {
+      Command: __bootstrap.spawn.createCommand(
+        internals.nodeUnstable.spawn,
+        internals.nodeUnstable.spawnSync,
+        internals.nodeUnstable.spawnChild,
+      ),
+    });
+
     const finalDenoNs = {
       core,
       internal: internalSymbol,
@@ -640,6 +664,13 @@ delete Intl.v8BreakIterator;
           ops.op_net_listen_unixpacket,
         ),
       });
+      ObjectAssign(finalDenoNs, {
+        Command: __bootstrap.spawn.createCommand(
+          finalDenoNs.spawn,
+          finalDenoNs.spawnSync,
+          finalDenoNs.spawnChild,
+        ),
+      });
     }
     ObjectDefineProperties(finalDenoNs, {
       pid: util.readOnly(runtimeOptions.pid),