bun-types: type Subprocess.stderr as undefined when the stderr option is undefined - #39297
bun-types: type Subprocess.stderr as undefined when the stderr option is undefined#39297robobun wants to merge 2 commits into
Conversation
… is undefined
Spawn.ReadableToIO mapped an undefined option to ReadableStream for both
stdout and stderr, but an undefined option means the slot's default, and
Bun.spawn defaults stderr to "inherit" (stdout to "pipe"). So
Bun.spawn(cmd, { stderr: undefined }).stderr type-checked as a
ReadableStream while being undefined at runtime.
ReadableToIO now takes the slot's default as a second type parameter and
resolves an undefined option through it. The parameter defaults to
"pipe", so the one-parameter form keeps its meaning; Subprocess passes
"pipe" for stdout/readable and "inherit" for stderr. ReadableToSyncIO is
unchanged because spawnSync defaults both slots to "pipe".
The fixture asserts the undefined option for stdout and stderr through
both spawn overloads, the stdio tuple, the onExit/ipc callback argument,
explicit type arguments, NullSubprocess, and spawnSync, and a tsc run
over fixture/spawn.ts is added so debug builds check it too.
|
Reproduced at the type level against the current Fix is in this PR; see the description for the reasoning and the related open PRs (#39283, #39279, #39270) that touch the same lines. |
|
Warning Review limit reached
Next review available in: 5 minutes Limit details: You’ve used all 5 included reviews currently available under your plan. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
There was a problem hiding this comment.
LGTM — types-only fix aligning Subprocess.stderr's type with runtime behavior when the option is undefined.
What was reviewed:
ReadableToIO<X, Default>change: distributivity over unions, theExclude<Readable, undefined>constraint onDefault(prevents recursion), and backward compat of the single-arg form — all hold.- Runtime defaults confirmed against
js_bun_spawn_bindings.rs:361-366([Ignore, Pipe, Inherit]; sync overrides both toPipe). - Holder types
Subprocess["stderr"]/WritableSubprocess["stderr"]stayReadableStream | number | undefined;NullSubprocess["stderr"]narrowing toundefinedis correct. - New
bun-types.test.tscase follows the existingBun.mmapblock pattern; fixture assertions cover both overloads,stdiotuple,onExit/ipc,spawnSync, and explicit type args.
Extended reasoning...
Overview
This PR fixes a type-vs-runtime mismatch in packages/bun-types/bun.d.ts: when stderr: undefined is passed to Bun.spawn, the runtime falls back to the slot default ("inherit"), so proc.stderr is undefined — but the types previously said ReadableStream. The fix adds a second Default type parameter to ReadableToIO so the undefined arm resolves to the slot's actual default; Subprocess.stderr passes "inherit", stdout/readable pass "pipe". JSDoc on the stdio/stdout/stderr options is updated to stop claiming undefined means pipe. Fixture assertions and a debug-build tsc case are added.
Security risks
None. This is a .d.ts-only change plus type-check test fixtures; no compiled runtime code is touched.
Level of scrutiny
Low-to-medium. Type declarations for a public API — the risk is breaking existing user code that type-checked before. I checked the specific concerns: the single-arg ReadableToIO<X> keeps its old meaning (Default defaults to "pipe"); the unconstrained Subprocess["stderr"] and WritableSubprocess["stderr"] unions are unchanged (pinned in the fixture); NullSubprocess["stderr"] narrows from ReadableStream | undefined to undefined, which is a strict correction (none of "ignore" | "inherit" | null | undefined produces a stream at runtime for stderr). The Default extends Exclude<Readable, undefined> constraint guarantees the recursive arm terminates.
Other factors
The runtime claim was verified directly against src/runtime/api/bun/js_bun_spawn_bindings.rs:361-366: async spawn starts from [Ignore, Pipe, Inherit] and spawnSync sets both output slots to Pipe, matching the PR's mapping (and why ReadableToSyncIO is left alone). The new test in bun-types.test.ts is byte-for-byte the same shape as the existing Bun.mmap debug-build check just above it. The fixture coverage is thorough — both spawn overloads, the stdio tuple with literal and union undefined, onExit/ipc, explicit Subprocess<...> args, spawnSync, and the alias itself — and the PR description documents that the fixture fails (10 diagnostics) against the unfixed declarations. No outstanding reviewer comments.
|
Updated 2:21 AM PT - Aug 16th, 2026
✅ @robobun, your commit e0e7b620eaf69ba37b7f6b582a51b1f8ecb2d399 passed in 🧪 To try this PR locally: bunx bun-pr 39297That installs a local version of the PR into your bun-39297 --bun |
There was a problem hiding this comment.
LGTM — types-only fix aligning Subprocess.stderr with runtime behavior when the option is undefined.
What was reviewed:
ReadableToIO's newDefaultparameter distributes correctly over unions;NullSubprocess["stderr"],WritableSubprocess["stderr"](theanypath) and unconstrainedSubprocess["stderr"]all resolve as the fixture asserts.- Backward compat: one-arg
ReadableToIO<X>keeps its old meaning viaDefault = "pipe";ReadableToSyncIOandWritableToIOare correctly left alone. - New fixture lines are appended, so the pinned
spawn.ts:62/:107diagnostics in the lib.dom case don't shift; the newBun.spawndescribe block mirrors the existingBun.mmapblock byte-for-byte in shape.
Extended reasoning...
Overview
This PR fixes a type-vs-runtime mismatch in packages/bun-types/bun.d.ts: Bun.spawn(cmd, { stderr: undefined }) typed proc.stderr as ReadableStream when the runtime returns undefined (because an explicit undefined option keeps the slot's default, and stderr's default for spawn is "inherit"). The fix adds a second Default parameter to Spawn.ReadableToIO so undefined recurses into the slot's actual default, and Subprocess.stderr passes "inherit" while stdout/readable pass "pipe". JSDoc on the stdio/stdout/stderr options is corrected to match. The test file gains a debug-build-friendly tsc case and the fixture gains ~75 lines of type assertions covering both spawn overloads, the stdio tuple, onExit/ipc callbacks, spawnSync, the holder unions, and the alias itself.
Security risks
None. This is a .d.ts type-declaration and JSDoc change plus type-check fixtures; nothing here executes at runtime or affects any security-sensitive path.
Level of scrutiny
Low-to-medium. Types-only changes cannot break runtime behavior; the risk is limited to (a) breaking existing user type-checks and (b) the new fixture assertions being wrong. For (a), Default defaults to "pipe", so the one-parameter form ReadableToIO<X> is unchanged, and I traced the distributive-conditional evaluation for NullSubprocess["stderr"] (all four union members → undefined), WritableSubprocess["stderr"] (any → both branches → ReadableStream | number | undefined), and unconstrained Subprocess["stderr"] (full Readable union → same) — each matches what the fixture pins. For (b), the assertions match the runtime probe in the PR description and the runtime source references ([Ignore, Pipe, Inherit] defaults for spawn, both "pipe" for spawnSync).
Other factors
- The new fixture lines are appended after the last existing block, so the line-pinned lib.dom diagnostics at
spawn.ts:62:38andspawn.ts:107:38inbun-types.test.tsremain valid; the new assertions use the sameexpectType(...).is<ReadableStream<...>>()shape that already appears earlier in the fixture without producing lib.dom diagnostics. - The new
describe("Bun.spawn")block is structurally identical to the existingdescribe("Bun.mmap")block right above it (same tsconfig setup, same tsc spawn, same stdout/stderr/exitCode assertion order), and the PR notes it is byte-identical to the same block in two related open PRs so they merge cleanly. tsd.expectType<T>()(zero-arg) and.is<X>are both supported byfixture/utilities.ts, andBun.Spawnis the real namespace name (SpawnOptionsis anexport importalias), soBun.Spawn.ReadableToIOin the fixture resolves.- No prior human reviews or unaddressed comments on the timeline; the bug-hunting pass found nothing.
Problem
Bun.spawn(cmd, { stderr: undefined })typesproc.stderrasReadableStream<Uint8Array<ArrayBuffer>>; at runtime (bun 1.4.0) it isundefined, soproc.stderr.text()type-checks and throws. The same happens withstdio: [.., .., undefined], with theSubprocesspassed toonExit/ipc, withSubprocess<In, Out, "pipe" | undefined>["stderr"](a plainReadableStream, also whatstderr: cond ? "pipe" : undefinedinfers underexactOptionalPropertyTypes), and withNullSubprocess["stderr"](ReadableStream | undefined).Spawn.ReadableToIO(packages/bun-types/bun.d.ts:7439) maps the option type"pipe" | undefinedtoReadableStream, andSubprocessuses it for bothstdoutandstderr(bun.d.ts:7548-7549). Anundefinedoption means "use this slot's default", and the two slots have different defaults: the runtime starts from[Ignore, Pipe, Inherit]forspawn(src/runtime/api/bun/js_bun_spawn_bindings.rs:361-366), anundefinedoption leaves that default in place (spawn/stdio.rs:422-424), and an inherited stream reads back asundefined(subprocess/Readable.rs:266). The alias dates from when the declarations modelled the stderr default as"pipe"(bun-types: infer strictSubprocessfromBun.spawn()options #1501); bun-types: infer strictSubprocessfromBun.spawn()options, part 2 #2573 and later Improve types and autocomplete for Bun.spawn (fixes #17274) #19162 changed the omitted-option default to"inherit"without touching this arm.stdio,stdoutandstderroptions made the same claim ("pipe",undefined: the process has a ReadableStream).Fix
ReadableToIO<X, Default = "pipe">: anundefinedXresolves toReadableToIO<Default>; the other arms are unchanged.Subprocess.stdoutand.readablepass"pipe",Subprocess.stderrpasses"inherit". The option JSDoc listsundefinedas "the default below" instead of grouping it with"pipe".Subprocessonly ever comes fromBun.spawn(js_bun_spawn_bindings.rs:1670is the single construction site;spawnSyncreturns a plain object typedSyncSubprocess, the shell uses an internal type), so its stderr slot always defaults to"inherit".ReadableToSyncIOis untouched becausespawnSyncdefaults both slots to"pipe";WritableToIOis untouched because stdin's default ("ignore") already maps toundefined.Defaultdefaults to"pipe", soBun.Spawn.ReadableToIO<X>in user code keeps its meaning, and the holder unions do not move (Subprocess["stdout"]/["stderr"]andWritableSubprocess["stderr"]stayReadableStream | number | undefined; pinned in the fixture). Type-checkingtest/,src/jsandsrc/against the changed declarations produces exactly the same diagnostics as before.stderr: cond ? "pipe" : undefinedas a named option still infersErr = "pipe"under default compiler options, because TypeScript dropsundefinedwhen inferring through an optional property; no declaration shape changes that. It is fixed underexactOptionalPropertyTypes, and the same union through thestdiotuple or explicit type arguments is fixed under any options (both asserted).test/integration/bun-types/fixture/spawn.tsassertsstderr: undefined/stdout: undefinedthrough bothspawnoverloads, thestdiotuple (literalundefinedand"pipe" | undefined), theonExit/ipcargument, explicitSubprocess<...>arguments,NullSubprocess["stderr"], both forms of the alias, andspawnSyncwithundefinedoptions (stillBuffer). Against the unfixedbun.d.tsthese lines produce 10 diagnostics (below; spawn.ts:241-302); with the fixbun test test/integration/bun-types/bun-types.test.tspasses all 16 cases (tsc with and without lib.dom, and tsgo). That file is what.github/workflows/bun-types.ymlruns, so CI checks the new lines through the existing whole-fixture cases.bun-types.test.tsalso gains a case running tsc overfixture/spawn.tsalone, because under a debug build (bun bd test) the whole-fixture cases are skipped and nothing else would exercise the fixture; it fails without thebun.d.tschange and passes with it. It is byte-identical to the block bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283 carries (one hunk for git), and it is interim: test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270 replaces this per-file pattern with one whole-fixture check that runs on debug builds too, plus a lint holdingbun-types.test.tsto a single compiler spawn. If test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270 lands first, this block is dropped from here on rebase and the fixture lines are covered by its case; if this lands first, test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270's lint flags the block on its rebase and it goes out with theBun.mmapblock test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270 already removes. Either way nothing from it survives.git merge-treeof the two branches conflicts only in the alias declaration and theSubprocess.stdout/stderrmembers (where bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283 adds JSDoc), and the changes compose. Whichever lands second resolves it by adding theDefaultparameter and theX extends undefined ? ReadableToIO<Default>arm in front of bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283's arms, passing"pipe"/"inherit"on the threeSubprocessmembers, and merging the two alias doc comments; the option JSDoc and the fixture additions (appended at the end ofspawn.ts) merge on their own. The branch merges cleanly with bun-types: declare Subprocess.writable and lint interface Subprocess against the classes.ts table #39279 and test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270.NullSubprocess/NullSyncSubprocesslistundefinedin parameters whose default is"pipe"(soNullSubprocess["stdout"]staysReadableStream | undefined), and theterminaloption makes all three stdio propertiesnullat runtime, which none of the mapping types model.Background
Bun.spawninfers one type parameter per stdio slot (In,Out,Err) from thestdin/stdout/stderroptions or thestdiotuple. A parameter whose option is omitted takes the declared default ("ignore","pipe","inherit"); an option that is present butundefinedmakes the parameterundefineditself.Subprocess<In, Out, Err>then derives each property's type with a conditional type alias, which is therefore the place where anundefinedparameter has to be turned back into that slot's default."pipe" | undefinedbecomesReadableStream | undefinedfor stderr, and why the unconstrained holder typeSubprocess(every option at once) still lists every possible value.Diagnostics the new fixture lines produce against the unfixed bun.d.ts
Runtime probe (bun 1.4.0, linux x64)