bun-types: declare Subprocess.connected - #38677
Conversation
The Subprocess prototype has had a connected getter since IPC landed (BunObject.classes.ts, getConnected), and node:child_process builds ChildProcess#connected on top of it, but interface Subprocess in bun.d.ts never declared it, so proc.connected is a TS2339 error for users. Declare it as readonly connected: boolean, document it in the child process docs, and assert on it in the bun-types spawn fixture. The fixture is now also type-checked on its own with tsc in bun-types.test.ts, since the in-process type-check cases are skipped on debug builds.
|
Warning Review limit reached
Next review available in: 36 minutes 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 (4)
Comment |
|
Status: ready for review. Reproduced by adding The runtime already has the getter ( |
There was a problem hiding this comment.
LGTM — declares the existing runtime Subprocess.connected getter in bun-types, with matching docs and a type-check test.
What was reviewed:
- Confirmed the getter exists on the prototype (
BunObject.classes.ts:103, no setter) andonDisconnectreferenced in the JSDoc is a real spawn option. - New fixture blocks land after line 126, so the pinned
spawn.ts:62/spawn.ts:107diagnostics in the lib.dom case are unaffected. - New
Bun.spawn > fixture/spawn.ts type-checkscase mirrors the existingBun.mmaptsc-spawn pattern in the same file.
Extended reasoning...
Overview
Adds readonly connected: boolean to interface Subprocess in packages/bun-types/bun.d.ts, documents it in docs/runtime/child-process.mdx (IPC prose + Reference block), adds fixture assertions in test/integration/bun-types/fixture/spawn.ts, and adds a spawned-tsc test case in bun-types.test.ts that runs on debug builds too. No runtime code is touched.
Security risks
None. This is a .d.ts declaration plus documentation; nothing executes differently at runtime.
Level of scrutiny
Low. Type declarations for an already-shipping getter are mechanical. I verified the runtime side: src/runtime/api/BunObject.classes.ts:103 declares connected: { getter: "getConnected" } with no setter, so readonly boolean is the correct shape. The JSDoc's onDisconnect reference resolves to a real option at bun.d.ts:7140.
Other factors
- The fixture additions are inserted after the existing
proc.unref()block (line 126+), so the hard-codedspawn.ts:62:38andspawn.ts:107:38diagnostic line numbers in thechecks with lib.dom.d.tscase remain valid. - The new test case is a near-verbatim copy of the adjacent
Bun.mmapcase (sametscbinary path, sametypeRootswiring, same stdout/stderr/exitCode assertion order), so it inherits a known-working pattern. - The PR description shows the test failing without the
.d.tschange (4× TS2339) and passing with it on both debug and release builds, satisfying the fails-for-the-right-reason requirement.
|
Heads-up: #39279 adds |
Problem
Bun.spawn(cmd, { ipc() {} }).connectedfails to type-check:error TS2339: Property 'connected' does not exist on type 'Subprocess<"ignore", "pipe", "inherit">'.src/runtime/api/BunObject.classes.ts:103puts aconnectedgetter on the Subprocess prototype, implemented byget_connectedinsrc/runtime/api/bun/subprocess.rs:860, andnode:child_processimplementsChildProcess#connectedby reading it (src/js/node/child_process.ts:1343).interface Subprocessinpackages/bun-types/bun.d.tsnever declared it, so TypeScript users have to cast to reach it.Fix
readonly connected: booleanoninterface Subprocess, with JSDoc describing when it is true and false.truewhile the IPC channel is open,falseafterdisconnect(), after the child disconnected or exited, and alwaysfalsewithout theipcoption), and it has no setter. Every statement in the JSDoc and the docs paragraph was checked by running it, see the transcript below; thesend()claim comes fromdo_sendinsrc/runtime/ipc_host.rs:117, which gates on the sameis_connected()predicate as the getter and throwsERR_IPC_CHANNEL_CLOSEDwhen it is false.docs/runtime/child-process.mdx(IPC section and the Reference block).test/integration/bun-types/fixture/spawn.tsassertsproc.connectedis abooleanon a process spawned with and withoutipc, on theipccallback's subprocess argument, and that assigning to it is an error.test/integration/bun-types/bun-types.test.tsgets aBun.spawn > fixture/spawn.ts type-checkscase that runstscover that fixture alone; it fails without thebun.d.tschange (4 x TS2339, on both release and debug builds) and passes with it.bun bd test test/integration/bun-types/bun-types.test.ts(4 pass, 12 skipped as debug-only skips), and the same file under a release build, where the in-process cases also run (16 pass, including thelib.dom.d.tsvariant and tsgo).Subprocess.prototypealso has an undeclaredwritablegetter (alias ofstdin, returning aFileSink). It has been left out of bun-types since the package's first commit, presumably because it is not aWritableStream, so this PR leaves it alone.send()'s signature is being extended separately in bun-types: type Subprocess.send(message, handle, options, callback) and the ipc callback's handle #38662; this PR does not touch it.Background
Bun.spawnwith theipcoption opens a message channel (a socket pair) between the parent and a childbun/nodeprocess.subprocess.send()writes to it, theipccallback receives from it, and either side can close it withdisconnect(); it also closes when the child exits.connectedis the parent's view of whether that channel is still open.packages/bun-typesis the publishedbun-typespackage;interface Subprocessinbun.d.tsis the type of the objectBun.spawnreturns, so a runtime property missing from it is invisible to TypeScript users.test/integration/bun-types/packsbun-typesand type-checks thefixture/directory against it. Those in-process checks are skipped on debug builds (they drive the TypeScript language service and are very slow there), which is why this PR adds a case that spawnstscon the one fixture file instead, following the existingBun.mmapcase in the same file.Runtime behavior the declaration and docs describe (release build)
A child that calls
process.disconnect()itself also flips the parent'sconnectedtofalse(observed afteronDisconnectfired).Failure without the bun.d.ts change