bun-types: declare Subprocess.writable and lint interface Subprocess against the classes.ts table - #39279
bun-types: declare Subprocess.writable and lint interface Subprocess against the classes.ts table#39279robobun wants to merge 4 commits into
Conversation
Subprocess.prototype has had a writable getter since the readable one was added: both are registered in BunObject.classes.ts, and writable returns the same cached value as stdin. bun.d.ts declared readable but not writable, so proc.writable was a type error while working at runtime. Declare it with stdin's type, list it in the docs reference block, and assert in the bun-types fixture that writable and readable have the same types as stdin and stdout for every stdio configuration. The fixture is also type-checked on its own by a tsc run that is not skipped on debug builds.
|
Warning Review limit reached
Next review available in: 3 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 (5)
Comment |
|
Status: restructured after self-review, ready for review.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — declares the missing Subprocess.writable type alias to match the runtime getter that has existed since 2022.
What was reviewed:
- Confirmed
src/runtime/api/BunObject.classes.ts:71registerswritableas{ getter: "getStdin", cache: "stdin" }, soSpawnOptions.WritableToIO<In>(identical tostdin's type) is the correct declaration — same treatmentreadablealready gets forstdout. - Checked the new fixture assertions don't add diagnostics to the
lib.dom.d.tsexpected-error list (they use the sameReadableStream<Uint8Array<ArrayBuffer>>/FileSinkshapes already asserted earlier in the file without error). - The new
Bun.spawn > fixture/spawn.ts type-checkstest usestsconfig.fileswith an absolute path intoBASE_FIXTURE_DIR(rather thanincludelike the mmap case) so./utilitiesresolves relative to the fixture file — verified this is intentional and correct.
Extended reasoning...
Overview
This PR adds readonly writable: SpawnOptions.WritableToIO<In> to interface Subprocess in packages/bun-types/bun.d.ts, mirroring the existing readonly readable: SpawnOptions.ReadableToIO<Out> declaration for stdout. It also adds the property to the docs Reference block, adds fixture type assertions in test/integration/bun-types/fixture/spawn.ts, and adds a debug-build-friendly tsc test case that type-checks that fixture in isolation.
The runtime already exposes this property: src/runtime/api/BunObject.classes.ts:71 registers writable: { getter: "getStdin", cache: "stdin" }, so proc.writable === proc.stdin for every stdin configuration. The declaration uses the exact same WritableToIO<In> mapping as stdin at line 7547, so the two properties have identical types by construction.
Security risks
None. This is a pure type-declaration and docs change with no runtime code modification.
Level of scrutiny
Low. Type declarations for an existing, long-shipped runtime property, following the exact pattern of the sibling readable declaration two lines below it. The PR description demonstrates the tests fail without the .d.ts hunk (9 × TS2339) and pass with it, on both release (all 16 in-process cases + tsgo) and debug builds.
Other factors
- The fixture additions cover the generic case (
writable≡stdintype for any<In, Out, Err>), concrete resolutions ("pipe"→FileSink, fd →number, default/"inherit"→undefined,stdiotuple), the utility type aliases, andreadonlyenforcement via@ts-expect-error. - The new test case in
bun-types.test.tsfollows the existingBun.mmappattern but pointstsconfig.filesat the absoluteBASE_FIXTURE_DIR/spawn.tspath (rather than copying an inline file intocheckDir), which correctly lets the fixture'simport "./utilities"resolve. Pipes are drained concurrently and exit code is asserted last per repo conventions. - The JSDoc explicitly notes this is a
FileSink, not aWritableStream, so users don't misread it as makingpipeThrough(proc)work — a thoughtful clarification. - No prior human review comments to address; only a CodeRabbit rate-limit notice in the timeline.
…ead of adding a tsc case The spawned tsc case duplicated the one test/integration/bun-types is converging on (one whole-fixture run), and only proved that the one name was declared. A Subprocess instance of the RedisClient declaration lint checks every member the table installs, with connected listed as pending on the PR that declares it and @@asyncDispose accounted for by the interface's extends clause. bun.d.ts joins the source-lints triggers because the lint reads it. The fixture keeps the assertions about the aliases themselves (same type as stdin/stdout for any Subprocess, FileSink under stdin: "pipe", read-only, the utility types) and drops the ones that restated the stdio option mapping; they also move below the block another change to that mapping rewrites. The docs reference block spells both aliases as indexed access types so neither copy can drift from stdin/stdout.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The lint's failure output names both files, so the table does not need to.
|
Updated 1:37 AM PT - Aug 16th, 2026
✅ @robobun, your commit 98827edf21a68f0373218081d30fe6e959fb1b4d passed in 🧪 To try this PR locally: bunx bun-pr 39279That installs a local version of the PR into your bun-39279 --bun |
robobun
changed the title
There was a problem hiding this comment.
LGTM — types-only addition mirroring the just-merged RedisClient lint pattern.
What was reviewed:
writableinBunObject.classes.ts:71is{ getter: "getStdin", cache: "stdin" }, soSpawnOptions.WritableToIO<In>(identical tostdin's declared type) is the correct declaration.- The new lint is a close copy of
redis-client-types.test.ts(parent commit f0f6b2c), adapted for an interface body: the header regex matches the actual multi-lineinterface Subprocess<...> extends AsyncDisposable {shape, and thenoConstructor/empty-klassguard matches the current table. - Fixture assertions use the existing
expectType<T>().is<U>()zero-arg overload fromutilities.ts; the comment-cop feedback on the earliersrc/change was addressed in c34a74b.
Extended reasoning...
Overview
Adds readonly writable: SpawnOptions.WritableToIO<In> to interface Subprocess in packages/bun-types/bun.d.ts, plus a source-lint test comparing that interface's member set against the Subprocess.proto table in BunObject.classes.ts, a fixture block in test/integration/bun-types/fixture/spawn.ts asserting the alias types, a docs Reference-block line, and a workflow path trigger. No native/runtime code is touched.
Security risks
None. This is a .d.ts declaration, a docs line, a type-fixture, a source-tree-reading lint test, and a CI workflow path filter. Nothing executes at runtime and nothing handles untrusted input.
Level of scrutiny
Low-to-medium. The core change is an 8-line type declaration that exactly mirrors the existing stdin declaration (both use SpawnOptions.WritableToIO<In>), matching runtime behavior confirmed at BunObject.classes.ts:71 where writable shares stdin's getter and cache slot. The lint test is a near-verbatim adaptation of redis-client-types.test.ts from #39271 (this PR's direct parent commit), differing only where an interface body differs from a class body: no static/get/set in the member regex, an extends-clause map for @@asyncDispose, and a guard rejecting constructor/statics that an interface cannot mirror. I checked the header regex against the actual d.ts (multi-line type-param list, extends AsyncDisposable) and it matches; noConstructor: true and klass: {} on the Subprocess entry satisfy the guard.
Other factors
The gate evidence shows the lint fails on main naming exactly ["writable"] and passes on the branch (4/4), and the bun-types integration test goes from 6 TS2339 errors to 15/15 pass. The one prior reviewer comment (comment-cop on a src/ pointer comment) was resolved in c34a74b — the PR no longer touches src/. The pendingDeclarations entry for connected (#38677) is an intentional coordination point with a concurrent PR, matching the same mechanism in the RedisClient lint. The fixture's expectType<T>().is<U>() zero-arg form is the documented overload in utilities.ts and is already used elsewhere in the fixtures.
2 participants
Problem
Bun.spawn(["cat"], { stdin: "pipe" }).writablefails to type-check:error TS2339: Property 'writable' does not exist on type 'Subprocess<"pipe", "pipe", "inherit">'.src/runtime/api/BunObject.classes.ts:71registerswritableon the Subprocess prototype as{ getter: "getStdin", cache: "stdin" }, so it runs thestdingetter and shares its cache slot:proc.writable === proc.stdinfor every stdin configuration (transcript below). It was added in the same commit asreadable(4700762, 2022).interface Subprocessinpackages/bun-types/bun.d.tshas declaredreadable(thestdoutalias) and neverwritable, and nothing compares the interface with the table: of the 20 members the table installs,writableandconnectedare undeclared today (connectedis being declared by bun-types: declare Subprocess.connected #38677).Fix
readonly writable: SpawnOptions.WritableToIO<In>oninterface Subprocess, next toreadable. This is the one declaration that agrees with the runtime: the getter isstdin's getter, so it getsstdin's exact type andreadonly(there is no setter), the same treatmentreadablealready gets forstdout. The JSDoc says it is aFileSinkunderstdin: "pipe", not aWritableStream, so thepipeThroughnote onreadableis not read as applying to the pair (stream.pipeThrough(proc)throwsTypeError: The transform's 'writable' property must be a WritableStreamtoday; unchanged here).test/internal/source-lints/subprocess-types.test.ts, the Subprocess counterpart of the RedisClient lint from Lint the RedisClient class in redis.d.ts against the valkey.classes.ts tables #39271: it loadsBunObject.classes.ts, collects theprotonames the codegen installs, parses the member names in theinterface Subprocessbody, and requires the two sets to match in both directions. Differences from the RedisClient version, because the declaration is an interface:@@asyncDisposecounts as declared while the header saysextends AsyncDisposable, and the lint refuses to run if the table ever gains a constructor or statics, which an interface body cannot mirror.connectedsits inpendingDeclarationspointing at bun-types: declare Subprocess.connected #38677; whichever of the two PRs lands second gets a one-line failure on rebase telling it to delete that entry (verified on a trial merge, output below)..github/workflows/source-lints.ymlgainspackages/bun-types/bun.d.tsas a trigger because the lint reads it.test/integration/bun-types/fixture/spawn.tsasserts the property's type: for a genericSubprocess<In, Out, Err>,writablehas exactlystdin's type andreadableexactlystdout's; understdin: "pipe"it is aFileSinkand assigning to it is an error; and thePipedSubprocess/WritableSubprocess/NullSubprocess/ReadableSubprocessutility types resolve the aliases. The fixture is checked by the existing release-build cases (with and withoutlib.dom.d.ts, and tsgo); this PR adds no tsc case of its own, since test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270 is replacing those per-API cases with one whole-fixture run.writable, and spells both aliases asSubprocess["stdin"]/Subprocess["stdout"]so neither copy can drift from thestdin/stdoutlines above it (bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283 is changing thestdinline).bun test test/internal/source-lints/subprocess-types.test.ts: with main'sbun.d.tsthe "declares every member" test fails naming exactly["writable"]; with this PR's, 4 pass (same underbun bd test). Whole directory: 174 pass. The eight drift shapes in the details block each fail the intended test.USE_SYSTEM_BUN=1 bun test test/integration/bun-types/bun-types.test.ts: with main'sbun.d.tsthe fixture additions produceTS2339onwritableat 6 sites in every type-check case; with this PR's, 15/15 pass.git merge-tree) with bun-types: declare Subprocess.connected #38677, bun-types: type Subprocess.send(message, handle, options, callback) and the ipc callback's handle #38662, test(bun-types): replace the tsgo and Bun.mmap spawns with one whole-fixture tsc run, enforced by a lint #39270 and bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283 are all conflict-free. On the merged trees the lint still parses bun-types: type Subprocess.send(message, handle, options, callback) and the ipc callback's handle #38662's multi-linesendoverloads (4 pass), the bun-types test passes with bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283's new stdio mapping (16/16), and bun-types: declare Subprocess.connected #38677 trips only the pending-entry test as designed.WritableToIO/ReadableToIOthemselves (whatstdin, and nowwritable, resolve to for buffered inputs) are being corrected in bun-types: type Subprocess stdio properties by what the runtime exposes for each stdio option #39283;writablefollows whateverstdin's type is, so it needs no change there.Background
*.classes.tsfiles are the input of bun's class codegen: eachprotoentry becomes a property on the native class's prototype.cache: truestores a getter's result on the instance so later reads return the same object;cache: "stdin"makes thewritablegetter read and write the slotstdinuses, which is what makes the two properties return the identical value. Entries markedinternal/privateSymbol/publicSymbolare not installed under a plain name, and a key spelled@@xis installed underSymbol.x.packages/bun-typesis the publishedbun-types/@types/bunsurface. It is hand-written, not generated from the class definitions, so a prototype property can exist at runtime without a declaration.interface Subprocessthere is the typeBun.spawnreturns, andSpawnOptions.WritableToIO<In>is the mapping it already uses to turn thestdinoption's type into thestdinproperty's type ("pipe"gives aFileSink, the writer for the process's stdin pipe).test/internal/source-lints/holds tests that only read the source tree; they run in thesource-lintsGitHub workflow against a released bun, triggered by the paths listed in the workflow, and are excluded from the Buildkite lanes.test/integration/bun-types/packsbun-typesand type-checksfixture/*.tsagainst it;expectType(x).is<T>()there is an exact type-equality assertion.Lint output with main's bun.d.ts, and the simulated drift shapes
Trial merge with #38677 (both declaring their member):
Fixture failure with main's bun.d.ts (release build)
Runtime behavior the declaration describes (release build, linux x64)
Earlier revision
The first push (18bc3c9) covered the declaration with a
bun-types.test.tscase that spawned tsc overfixture/spawn.tsalone, plus fixture blocks restating the stdio option mapping (stdin: 0is anumber, and so on). Self-review pointed out that the tsc case duplicated the whole-fixture run #39270 introduces (and would fail the spawn-site lint it adds), that the mapping blocks conflicted with #39283, and that the check only proved one name was declared while a second member was undeclared in the same table. c42e5dc replaced the case with the lint, trimmed the fixture to the alias assertions and moved it out of #39283's region, and switched the docs lines to the indexed form; c34a74b dropped a pointer comment on the proto table, leaving no src/ change.[review] gate passed · iteration 0 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file