Conversation
`lazy: true` starts the stdout/stderr pipe readers paused. Reading begins on the first read from the stream, not on property access. Say so, and say that a child that fills the pipe buffer blocks until then. JSON5.stringify writes single-quoted strings. The JSDoc prose and its example said double quotes.
|
Warning Review limit reached
On-demand reviews are free for the next 10 days. After that, they cost $0.25 per reviewed file. Or wait 58 seconds for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs — both JSDoc corrections check out against the implementation. Leaving final sign-off to the packages/bun-types/ CODEOWNER.
What was reviewed:
JSON5.stringifysingle-quote claim —append_quoted_stringinsrc/runtime/api/JSON5Object.rs:365writes'unconditionally; the example output now matches runtime behavior.lazysemantics —SubprocessPipeReader.rs(posix line ~192, windows ~158) defers poll registration "until JS first pulls" and notes the kernel pipe buffer provides backpressure; the new prose mirrors those source comments.- No
*/introduced inside the JSDoc blocks; no declaration signatures touched.
Extended reasoning...
Overview
This PR edits JSDoc comments only, in a single file: packages/bun-types/bun.d.ts. Two spots change: the JSON5.stringify description and example are corrected from double quotes to single quotes, and the SpawnOptions.lazy description is clarified to say reading starts on the first stream read (not on property access), with a note about pipe-buffer backpressure. No type signatures, overloads, or exported symbols change.
Security risks
None. This is documentation prose inside .d.ts comments; it does not affect compiled output, runtime behavior, or the type-check surface.
Level of scrutiny
Per the repo's docs/types review guidance, .d.ts JSDoc must mirror the runtime exactly and every claim should be verified against the implementation. I checked both:
src/runtime/api/JSON5Object.rs:364-365—append_quoted_stringopens with a literal', confirming single quotes are always used and the new example{a:1,b:'two'}is what Bun prints.src/runtime/api/bun/subprocess/SubprocessPipeReader.rs— on both the Windows (~158) and POSIX (~192) branches,lazyleaves the reader paused so poll/uv_read_startis "deferred until JS first pulls", and the in-source comment explicitly states "the kernel pipe buffer then provides backpressure and the child blocks." The new JSDoc restates exactly this, and the added backpressure sentence is load-bearing (it explains the observable consequence, not just the option name).
I also confirmed the edited JSDoc blocks contain no stray */ that would break the .d.ts parse.
Other factors
.github/CODEOWNERS assigns /packages/bun-types/ to a specific owner, so per the approval guidelines I'm deferring rather than approving outright even though the change is small and verifiably correct. The bug hunt ran to a dry streak with zero findings.
…nst the runtime jsdoc-examples.test.ts runs the @example of JSON5.stringify from bun.d.ts and compares stdout with the comment lines under each console.log call. A second test covers what the lazy JSDoc states: access to the stdout property does not start the pipe reader, the first read does.
Problem
SpawnOptions.lazy(packages/bun-types/bun.d.ts:7793) says "Reading begins only when you access thestdoutorstderrproperties." Since child_process: apply kernel backpressure to stdout/stderr pipes #34971 a lazy pipe reader starts paused, and the first read from JS starts it.p.stdoutalone starts nothing.JSON5.stringify(bun.d.ts:2089, example at:2107) says "strings use double quotes" and shows{a:1,b:"two"}. Bun prints{a:1,b:'two'}.append_quoted_string(src/runtime/api/JSON5Object.rs:365) always writes single quotes.Fix
lazy: reading begins when you first read from the stream, for example.text()or.getReader(). Access to the property alone does not start it. A new sentence says that, until then, a child that fills the pipe buffer blocks.JSON5.stringify: the prose says "single quotes" and the example output is{a:1,b:'two'}.test/integration/bun-types/jsdoc-examples.test.ts. It runs theJSON5.stringifyexample and compares stdout with the example's comments (fails with the old JSDoc). A second test covers thelazytext. Both pass on Linux x64 (debug build) and Windows x64.Background
node:child_processpasseslazy: truetoBun.spawn. child_process: apply kernel backpressure to stdout/stderr pipes #34971 madelazydefer poll registration (SubprocessPipeReader.rs:192,:158on Windows). The kernel pipe buffer then applies backpressure and the child blocks, as in Node.ReadableStreamover the pipe starts its native source when a consumer attaches.getReader(),.text(),for await,tee()andpipeTo()start the read.p.stdoutandnew Response(p.stdout)do not.json5package also prints{a:1,b:'two'}. It uses double quotes only for a string with more'than". Bun always writes single quotes and escapes'.Notes
The new test file sits next to
bun-types.test.tsand not inside it, because that file packs and installsbun-typesinbeforeAlland only type-checks. Thelazytest passes with and without this diff, because the runtime does not change. It fails when the spawn useslazy: false: the child exits within 16 ms.lazyprobe. The child writes 4 MB to stdout withfs.writeSync.maxBuffer: 1000shows when the reader runs, because the kill fires only after the reader counts bytes. The output is the same on Bun 1.4.3 on Linux x64 (4ff9193) and Windows x64 (canary e5f9986):On Linux the same probe also covered
.bytes(),for await,tee(),pipeTo()andnew Response(p.stdout).text()(each starts the read),p.stdout.locked(does not), andstderr(same asstdout).JSON5.stringifyon 1.4.3:Before #34971 the poll was registered at spawn with or without
lazy, andlazyonly skipped the first synchronous read. #34971 gavelazyits current meaning and did not touchbun.d.ts.docs/runtime/child-process.mdxdoes not mentionlazy, so it needs no edit.docs/runtime/json5.mdxalready shows single quotes.Two open PRs touch the same JSDoc blocks. #39925 (the
replacerfeature) carries the same two JSON5 lines with the same text. #42256 (enforcemaxBufferwithlazy) adds a paragraph further down in thelazyblock. Neither conflicts with this diff.test/integration/bun-types/bun-types.test.tsgives identical output with and without this diff. TheTypeScript typescheck fails on every PR that touchespackages/bun-types/**since 2026-09-09, because the npmlatesttag of@types/nodemoved to 22.20.2. #42230 has the cause and the fix.prettier --checkpasses.[human-review] gate passed · iteration 1 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file