bun-types: type Subprocess stdio as null and terminal as Terminal when spawned with the terminal option - #39350
bun-types: type Subprocess stdio as null and terminal as Terminal when spawned with the terminal option#39350robobun wants to merge 1 commit into
Conversation
… when Bun.spawn gets the terminal option When Bun.spawn is given the terminal option, the stdin, stdout and stderr getters return null and the terminal getter returns the Terminal, but the declarations derived the stdio properties from the stdio options alone and typed terminal as Terminal | undefined regardless. Subprocess gains a fourth type parameter holding the type of its terminal property (Terminal or undefined); the stdio properties and readable are null when it is Terminal. Bun.spawn picks it with overloads: a present terminal option gives Terminal, an absent one undefined, and an option that may be undefined gives Terminal | undefined. The parameter defaults to undefined when the stdio type arguments are given, so existing annotations keep their types, and to Terminal | undefined when they are not, so a plain Subprocess still accepts every process Bun.spawn returns. BaseOptions and SpawnOptions carry the same parameter so the onExit and ipc callbacks see the same process type, and a SpawnOptions annotation states whether it may carry a terminal. The spawn fixture asserts these shapes and bun-types.test.ts gains a case that type-checks that fixture on debug builds as well.
|
Status: types-only fix for the Reproduced by type-checking the new lines at the end of test/integration/bun-types/fixture/spawn.ts against main's bun.d.ts (52 diagnostics, among them With this branch |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan includes up to 5 reviews per rolling hour; 1 remains after this review. WalkthroughChangesThe declarations add terminal-state generics to spawn options, callbacks, and subprocesses. Terminal spawn typing
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The PR improves TypeScript accuracy for terminal-backed subprocesses and adds focused type tests; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it adds a fourth type parameter to the public Subprocess / SpawnOptions / BaseOptions interfaces and reshapes Bun.spawn's overload set — with acknowledged blast radius (new type errors in test/cli/watch/watch.test.ts, changed diagnostic shapes, Parameters<typeof Bun.spawn>[0] flipping forms) and several sibling PRs touching the same lines — a human look at the API design would still be worthwhile.
What was reviewed:
- Overload ordering and the
Term = undefined/Terminal/Terminal | undefinedsplit against the fixture cases; the terminal-less overloads correctly stay first so existing calls withoutterminalkeep their exact types. - The conditional default for
TermonSubprocess—Subprocess<any, any, any>and bareSubprocessgetTerminal | undefined, while any narrowed stdio arg givesundefined, which is what the fixture pins. - The
Term | (Term extends Terminal ? TerminalOptions : never)spelling for covariance, and thatSpawnSyncOptionsstill extendsBaseOptions<In, Out, Err>(implicitTerm = undefined), sospawnSynccontinues to rejectterminal. - The new
bun-types.test.tsblock matches the existingBun.mmappattern in the same file.
Extended reasoning...
Overview
This is a types-only change to packages/bun-types/bun.d.ts plus fixture and test additions. It threads a fourth type parameter Term extends Terminal | undefined through BaseOptions, SpawnOptions and Subprocess, so that when Bun.spawn is called with the terminal option the returned process types stdin/stdout/stderr/readable as null and terminal as Terminal (matching runtime behavior in src/runtime/api/bun/subprocess.rs). Bun.spawn grows from 2 to 6 overloads to pick Term from the call site. ~180 lines of fixture assertions in spawn.ts pin the inference for both call forms, the conditional-terminal case, onExit/ipc callbacks, annotated SpawnOptions values, and the holder-type assignability in both directions. A new bun-types.test.ts case runs tsc over fixture/spawn.ts alone so debug builds check it too.
Security risks
None. This is .d.ts only — no runtime code, no native code, no I/O.
Level of scrutiny
Medium-high, as an API-design change rather than a correctness risk. The runtime behavior being modeled is verified and the fixture coverage is very thorough. But this widens the public type surface of three heavily-used interfaces, uses a non-obvious conditional default ([Writable, Readable, Readable] extends [In, Out, Err] ? Terminal | undefined : undefined) whose intent is that specifying any stdio arg opts you out of the terminal union, and relies on a variance trick that is documented in a comment but is the kind of thing a maintainer should agree to. The PR itself reports it introduces three new type errors in test/cli/watch/watch.test.ts and changes the shape of overload error messages for array-form mistakes (now 3 candidates listed). Per the repo's review guidance, adding or changing user-facing API surface warrants a maintainer look.
Other factors
- The deprecated
OptionsObjectalias is unchanged (still 3 params), so it no longer forwards aTerm— that seems intentional given it's deprecated, but worth a maintainer glance. - Four related open PRs (#39283, #39297, #39279, #39303) edit the same
Subprocessproperty lines; the description calls out the mechanical conflicts and that #39270 will supersede the duplicated test block. Landing order is a maintainer call. - No prior human review on the thread; only the robobun status comment. Given the design-level decisions (overloads vs. inferred parameter, the conditional default's exact trigger, whether the new watch.test.ts errors should be fixed in this PR), I'm deferring rather than approving.
|
On the const opts: Bun.SpawnOptions.OptionsObject<"pipe", "pipe", "pipe"> = {};
Bun.spawn(["cat"], opts); // Subprocess<"pipe", "pipe", "pipe", undefined>The design questions (overloads instead of an inferred parameter, what triggers the |
|
Updated 3:58 AM PT - Aug 16th, 2026
✅ @robobun, your commit 95aaf26dc5fe0ca576d1fd71ecf734f2e9d8f124 passed in 🧪 To try this PR locally: bunx bun-pr 39350That installs a local version of the PR into your bun-39350 --bun |
Problem
Bun.spawn(["bash"], { terminal: { data() {} } })typesproc.stdoutasReadableStream,proc.stderrasundefinedand, withstdin: "pipe",proc.stdinasFileSink, soproc.stdin.write(..)type-checks and throws; at runtime (bun 1.4.0) all three arenull.proc.terminalis typedTerminal | undefinedalthough it is always set, so the documentedproc.terminal.write(..)(docs/runtime/child-process.mdx, and the option's own JSDoc example) does not compile, and bun's own tests writeproc.terminal!.get_stdin/get_stdout/get_stderrin src/runtime/api/bun/subprocess.rs:577-606 returnJSValue::NULLwhenever a terminal is attached,get_terminal(:617) returns it, and js_bun_spawn_bindings.rs:888-899 overwritesstdio[0..3]with the PTY, so the stdio options have no effect.readableandwritableare registered with the same getters (src/runtime/api/BunObject.classes.ts:71-76).Subprocess<In, Out, Err>(packages/bun-types/bun.d.ts:7542 on main) derivesstdin/stdout/stderr/readablefrom the three stdio options only,terminalis a fixedTerminal | undefined(:7567), and theterminaloption (:7436) is not reflected inspawn's signatures at all.Fix
Subprocessgets a fourth type parameter,Term extends Terminal | undefined, which is the type ofproc.terminal;stdin,stdout,stderrandreadablearenullwhen it isTerminaland keep their current types when it isundefined.Term = Terminal | undefinedgives the union of both (stdout: ReadableStream | null,terminal: Terminal | undefined).Bun.spawnpicksTermwith overloads, three per call form: options with aterminalproperty giveTerminal; options without one (or withterminal: undefined) giveundefined, as the existing two signatures now state explicitly; options whoseterminalmay beundefined(terminal: interactive ? {..} : undefined, or a value whose type declares it optional) fall through to a last pair that givesTerminal | undefined. Overloads rather than a type parameter inferred from the option because the usualterminal: { data(term, data) {..} }literal is context-sensitive: TypeScript fixes such a parameter to its default before it can infer from the literal (verified with a prototype: the literal was rejected andterm/databecame implicitany), and inference through an optional property also dropsundefined, so the third case could not be told apart.undefinedwhen the stdio type arguments are given (Subprocess<"pipe", "pipe", "pipe">,PipedSubprocess,Subprocess<any, "pipe", any>, ...), so every existing annotation keeps exactly its current property types;Terminal | undefinedwhen all three are left at their defaults (Subprocess,Subprocess<any, any, any>,ReturnType<typeof Bun.spawn>), so the plain holder still accepts every processBun.spawnreturns, which is what docs/runtime/child-process.mdx's reference block has described since the terminal option landed (stdin: FileSink | number | undefined | null,terminal: Terminal | undefined). The docs need no change.BaseOptionsandSpawnOptionscarry the same parameter (defaultundefined) so theSubprocesspassed toonExit/ipcmatches the returned one, andSpawnOptions<In, Out, Err>written as an annotation says whether it may carry aterminal(4th argumentTerminalorTerminal | undefined; without it the property is rejected, asstdin: "pipe"is rejected bySpawnOptions<"ignore", ..>today). The option's type is spelledTerm | (Term extends Terminal ? TerminalOptions : never)rather than as one conditional so that TypeScript measures the interface as covariant inTerm; with a bare conditional it measures bivariant and aSpawnOptions<.., Terminal | undefined>value matched the terminal-less overload (found while prototyping; the fixture pins the assignability both ways).SpawnSyncOptionsis untouched, sospawnSyncstill rejectsterminaland its callbacks seeterminal: undefined.ReturnType<typeof Bun.spawn>is still exactlySubprocessand a mistyped one-argument call is reported against the object form. Side effect:Parameters<typeof Bun.spawn>[0]was the array form'sstring[]and is now the object form's options type; the one in-tree use (test/js/web/fetch/fetch-http2-client.test.ts:162) wanted the latter.test/(7203 pre-existing diagnostics),src/jsandsrcbefore and after:srcis identical;src/jsand most oftest/only change the wording or anchor line of diagnostics that were already there (possibly 'undefined'becomespossibly 'null' or 'undefined'on plainSubprocessholders; printed types gain the fourth argument; overload errors now name the offending property, e.g. the pre-existing error in src/js/node/child_process.ts:1410 now points at the two real mismatches instead of "'cmd' does not exist in type 'string[]'"). The one new kind of error is test/cli/watch/watch.test.ts:233/274/307, which passes a plainSubprocessvariable to aSubprocess<"ignore", "pipe", any>parameter; that was accepted only because the conditional stdio types measure as bivariant, and is now rejected because the holder may hold a terminal process. Array-form mistakes now list the three two-argument overloads instead of one message (each line still carries the real error and the spelling suggestion); object-form mistakes get the one relevant message, where main listed both forms.data(term, data)callbacks, an existingTerminal, both call forms,stdio: [...]being overridden, the conditional case,onExit/ipc/spawnSynccallbacks, explicitterminal: undefined, annotatedSpawnOptionsobjects and their assignability,@ts-expect-erroronproc.stdin.write/proc.stdout.getReader(), and the holder types (Subprocess,ReturnType<typeof Bun.spawn>,Subprocess<any, any, any, Terminal>, and thatPipedSubprocess/ReadableSubprocess/WritableSubprocess/NullSubprocesskeep their types and reject terminal processes). Against main's bun.d.ts it produces 52 diagnostics (listed below);USE_SYSTEM_BUN=1 bun test test/integration/bun-types/bun-types.test.tspasses all 16 cases with the fix (tsc 6 with and without lib.dom, and the fixture's own TypeScript 7), the pinned spawn.ts:62 / :107 diagnostics unchanged.bun bd test test/integration/bun-types/bun-types.test.tspasses with the fix and fails on that case with packages/ stashed. The block is byte-identical to the one in bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283, bun-types: type Subprocess.stderr as undefined when the stderr option is undefined #39297 and bun-types: drop undefined from the Null* subprocess aliases where the default is "pipe" #39303, so whichever lands first the others merge cleanly; all of them become redundant once test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270 lands.ReadableToIOdefault for stderr) edit the sameSubprocessproperty lines and pin the plain holder unions, which this PR extends with| null; bun-types: declare Subprocess.writable and lint interface Subprocess against the classes.ts table #39279 addswritable, which needs the sameTerm extends Terminal ? null : ...wrapper asstdin; bun-types: drop undefined from the Null* subprocess aliases where the default is "pipe" #39303 (Null*aliases) is unaffected. Whichever of these lands second has a small, mechanical conflict in those hunks. The stale "Only available on POSIX systems" line in the same JSDoc block predates this change and is tracked separately.Background
Bun.spawninfers one type parameter per stdio slot from the literal options it is called with (stdin: "pipe"givesIn = "pipe"), andSubprocess<In, Out, Err>turns each into a property type with a conditional type alias (WritableToIO<"pipe">isFileSink,ReadableToIO<"inherit">isundefined). With no type arguments the parameters are the full option unions and each property becomes the union of everything it can be; that instantiation is what code stores processes of unknown configuration in.terminaloption (feat: add Bun.Terminal API for pseudo-terminal (PTY) support #25415) attaches a PTY; the child's three descriptors are the PTY's slave side, and the parent talks to it throughBun.Terminal(write()and thedatacallback), which is why the process exposes no streams for it.terminaland one allowing onlyterminal?: undefinedsplit the cases, and a signature that allows both has to come after them. When more than three signatures could apply and none does, TypeScript reports only the last one's error;ReturnTypeandParametersof an overloaded function also use the last signature.Generic<A>is assignable toGeneric<B>by first measuring how the type parameter is used (its variance) and comparingAwithBaccordingly, falling back to comparing the members only in some cases. A parameter used only inside conditional types and method parameters measures as bivariant (either direction passes); a parameter that also appears directly as a property type measures as covariant.Runtime probe (bun 1.4.0, linux x64)
Diagnostic shape for a mistyped option, main vs this PR (tsc 6.0.2)
main:
this PR:
Diagnostics the new fixture lines produce against main's bun.d.ts (52, selection)
Totals by code: TS2707 x17, TS2344 x9, TS2554 x8, TS2578 x7, TS2345 x4, TS7006 x3, TS2314 x2, TS18048 x2.