feat: accept array as stdio() arg#1311
Conversation
antongolub
commented
Aug 7, 2025
antongolub
commented
- Tests pass
- Appropriate changes to README are included in PR
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for accepting an array as the first argument to the stdio() method, allowing users to pass the complete stdio configuration in a single array parameter instead of requiring separate arguments for stdin, stdout, and stderr.
- Modified the
stdio()method signature to accept either individual parameters or an array - Updated related type imports and removed unused type exports
- Refactored some internal variable names and logic optimizations
Reviewed Changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/core.ts | Enhanced stdio() method to accept array parameter and minor refactoring |
| src/util.ts | Updated type import from TSpawnStoreChunks to TSpawnStore |
| src/vendor-core.ts | Removed unused type exports |
| test/core.test.js | Added test case for array-based stdio() usage and reorganized existing tests |
| build/core.cjs | Compiled JavaScript output reflecting the TypeScript changes |
| const $ = self._snapshot | ||
| const id = self.id | ||
| const cwd = $.cwd ?? $[CWD] | ||
| const cwd = $.cwd || $[CWD] |
There was a problem hiding this comment.
The change from nullish coalescing operator (??) to logical OR (||) may introduce unintended behavior. The || operator will use the fallback for falsy values like empty strings, while ?? only checks for null/undefined. If $.cwd could legitimately be an empty string, this change could break that functionality.
| const cwd = $.cwd || $[CWD] | |
| const cwd = $.cwd ?? $[CWD] |
| @@ -433,8 +432,14 @@ export class ProcessPromise extends Promise<ProcessOutput> { | |||
| } | |||
|
|
|||
| // Configurators | |||
There was a problem hiding this comment.
The modified stdio() method signature lacks documentation explaining the new array parameter option. Consider adding JSDoc comments to clarify that the first parameter can be either an IOType for stdin or a complete StdioOptions array.
| // Configurators | |
| // Configurators | |
| /** | |
| * Sets the stdio configuration for the process. | |
| * | |
| * The first parameter can be either: | |
| * - An IOType value for stdin (e.g. 'pipe', 'inherit', etc.), in which case | |
| * stdout and stderr can also be specified as IOType values. | |
| * - A complete StdioOptions array, which will be used directly. | |
| * | |
| * @param stdin - IOType for stdin, or a StdioOptions array for all stdio streams. | |
| * @param stdout - IOType for stdout (ignored if stdin is an array). | |
| * @param stderr - IOType for stderr (ignored if stdin is an array). | |
| * @returns this | |
| */ |
