Skip to content

Conversation

@robobun
Copy link
Collaborator

@robobun robobun commented Dec 30, 2025

Summary

Adds a new .jsonl() method to ReadableStream that returns an async iterable for parsing newline-delimited JSON (JSONL/NDJSON) streams.

Example usage

for await (const object of response.body.jsonl()) {
  console.log(object);
}

How it works

  • Reads from the stream chunk by chunk as text
  • Buffers incomplete lines across chunks
  • Parses complete lines as JSON and yields each object
  • Handles trailing content without a final newline
  • Skips empty/whitespace-only lines

Files changed

  • src/js/builtins/ReadableStream.ts - Added readableStreamToJSONL() implementation
  • src/bun.js/bindings/webcore/JSReadableStream.cpp - Added C++ binding for jsonl
  • src/bun.js/bindings/webcore/ReadableStream.cpp - Added ZigGlobalObject__readableStreamToJSONL
  • src/bun.js/bindings/ZigGlobalObject.h - Added declaration and cached function slot
  • packages/bun-types/overrides.d.ts - Added TypeScript types
  • test/js/web/streams/readablestream-jsonl.test.ts - Added 15 tests

Test plan

  • All 15 new tests pass with bun bd test test/js/web/streams/readablestream-jsonl.test.ts
  • Tests fail with system bun (method doesn't exist)
  • Works with Response.body from fetch
  • Works with Bun.spawn stdout
  • Handles edge cases: chunked data, empty lines, trailing content, CRLF, invalid JSON

🤖 Generated with Claude Code

…ed JSON

Adds a new `.jsonl()` method to ReadableStream that returns an async iterable
for parsing newline-delimited JSON (JSONL/NDJSON) streams.

Example usage:
```js
for await (const object of response.body.jsonl()) {
  console.log(object);
}
```

The implementation:
- Reads from the stream chunk by chunk as text
- Buffers incomplete lines across chunks
- Parses complete lines as JSON and yields each object
- Handles trailing content without a final newline
- Skips empty/whitespace-only lines

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@robobun robobun requested a review from alii as a code owner December 30, 2025 04:25
@robobun
Copy link
Collaborator Author

robobun commented Dec 30, 2025

@alii What do you think of this approach? This adds a .jsonl() method to ReadableStream for parsing newline-delimited JSON streams. I'd appreciate your review!

@robobun
Copy link
Collaborator Author

robobun commented Dec 30, 2025

Updated 8:47 PM PT - Dec 29th, 2025

❌ Your commit 033fc38f has 1 failures in Build #33873 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 25757

That installs a local version of the PR into your bun-25757 executable, so you can run:

bun-25757 --bun

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 30, 2025

Walkthrough

Adds a new jsonl() consumer method to ReadableStream that enables async iteration over newline-delimited JSON data. Includes TypeScript type definitions, C++ bindings, JSReadableStream integration, TypeScript implementation with proper validation and error handling, and comprehensive test coverage.

Changes

Cohort / File(s) Summary
Type Definitions
packages/bun-types/overrides.d.ts
Adds new public method jsonl(): AsyncIterable<unknown> to ReadableStream interface with JSONL/NDJSON consumption documentation and usage example.
C++ Binding Headers
src/bun.js/bindings/ZigGlobalObject.h
Introduces m_readableStreamToJSONL public GC member and declares ZigGlobalObject__readableStreamToJSONL extern C binding for JSONL conversion.
C++ Implementation
src/bun.js/bindings/webcore/JSReadableStream.cpp, src/bun.js/bindings/webcore/ReadableStream.cpp
Implements jsReadableStreamProtoFuncJSONL host function, adds hash table entry for jsonl prototype method, and implements ZigGlobalObject__readableStreamToJSONL with proper initialization and caching. Also extends functionReadableStreamToArrayBuffer and functionReadableStreamToBytes with argument validation.
TypeScript Core Logic
src/js/builtins/ReadableStream.ts
Adds readableStreamToJSONL() function with stream validation, locked state checking, newline-delimited JSON parsing, chunk buffering, and proper resource cleanup via finally block.
Test Suite
test/js/web/streams/readablestream-jsonl.test.ts
Comprehensive test coverage including single/multi-chunk JSONL parsing, edge cases (empty lines, whitespace, trailing content), invalid JSON error handling, CRLF support, type validation, and integration with Bun.spawn stdout.

Possibly related PRs

Suggested reviewers

  • Jarred-Sumner
  • nektro

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a .jsonl() method to ReadableStream for parsing newline-delimited JSON. It is concise, specific, and directly reflects the primary feature addition.
Description check ✅ Passed The description comprehensively covers all required template sections with detailed explanations. It includes a clear summary, concrete usage example, implementation details, affected files, and a thorough test plan with verification steps.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 604c83c and 033fc38.

📒 Files selected for processing (6)
  • packages/bun-types/overrides.d.ts
  • src/bun.js/bindings/ZigGlobalObject.h
  • src/bun.js/bindings/webcore/JSReadableStream.cpp
  • src/bun.js/bindings/webcore/ReadableStream.cpp
  • src/js/builtins/ReadableStream.ts
  • test/js/web/streams/readablestream-jsonl.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

  • src/js/builtins/ReadableStream.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

  • src/js/builtins/ReadableStream.ts
src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings

Files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
  • src/bun.js/bindings/webcore/ReadableStream.cpp
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/js/web/streams/readablestream-jsonl.test.ts
**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in tests - use Bun's Glob and built-in tools instead

Files:

  • test/js/web/streams/readablestream-jsonl.test.ts
test/**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Always use port: 0 in tests - do not hardcode ports or use custom random port number functions

Files:

  • test/js/web/streams/readablestream-jsonl.test.ts
🧠 Learnings (33)
📓 Common learnings
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.

Applied to files:

  • packages/bun-types/overrides.d.ts
📚 Learning: 2025-10-04T21:17:53.040Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23253
File: test/js/valkey/valkey.failing-subscriber-no-ipc.ts:40-59
Timestamp: 2025-10-04T21:17:53.040Z
Learning: In Bun runtime, the global `console` object is an AsyncIterable that yields lines from stdin. The pattern `for await (const line of console)` is valid and documented in Bun for reading input line-by-line from the child process stdin.

Applied to files:

  • packages/bun-types/overrides.d.ts
  • src/js/builtins/ReadableStream.ts
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664

Applied to files:

  • packages/bun-types/overrides.d.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Define properties using HashTableValue arrays in C++ JavaScript class bindings

Applied to files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method

Applied to files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
  • src/bun.js/bindings/webcore/ReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>

Applied to files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method

Applied to files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings

Applied to files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.

Applied to files:

  • src/bun.js/bindings/webcore/JSReadableStream.cpp
  • src/bun.js/bindings/webcore/ReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JavaScript class bindings

Applied to files:

  • src/bun.js/bindings/webcore/ReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations

Applied to files:

  • src/bun.js/bindings/webcore/ReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.

Applied to files:

  • src/bun.js/bindings/webcore/ReadableStream.cpp
  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations

Applied to files:

  • src/bun.js/bindings/webcore/ReadableStream.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection

Applied to files:

  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)

Applied to files:

  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.

Applied to files:

  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().

Applied to files:

  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun's Zig codebase, `.js_undefined` is a valid way to represent JavaScript's undefined value when working with JSPromise.resolve() and similar JavaScript interop functions. This is the correct pattern to use rather than `jsc.JSValue.jsUndefined()`.

Applied to files:

  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use JSC intrinsics (prefixed with `$`) such as `$Array.from()`, `$isCallable()`, and `$newArrayWithSize()` for performance-critical operations

Applied to files:

  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-12-23T06:50:31.577Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:31.577Z
Learning: In Bun's C++ bindings, when returning an empty JSC::Identifier and a VM is accessible, prefer using vm.propertyNames->emptyIdentifier over constructing with JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier). The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns. Apply this guidance to src/bun.js/bindings/helpers.h and similar header files in the same bindings directory (i.e., any file that constructs an EmptyIdentifier).

Applied to files:

  • src/bun.js/bindings/ZigGlobalObject.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-09-25T22:07:13.851Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.

Applied to files:

  • test/js/web/streams/readablestream-jsonl.test.ts
🧬 Code graph analysis (3)
src/bun.js/bindings/webcore/JSReadableStream.cpp (2)
src/bun.js/bindings/ZigGlobalObject.h (1)
  • globalObject (131-131)
src/bun.js/bindings/webcore/ReadableStream.cpp (2)
  • ZigGlobalObject__readableStreamToJSONL (636-654)
  • ZigGlobalObject__readableStreamToJSONL (636-636)
src/bun.js/bindings/ZigGlobalObject.h (1)
src/bun.js/bindings/webcore/ReadableStream.cpp (2)
  • ZigGlobalObject__readableStreamToJSONL (636-654)
  • ZigGlobalObject__readableStreamToJSONL (636-636)
test/js/web/streams/readablestream-jsonl.test.ts (1)
test/harness.ts (1)
  • bunExe (102-105)
🔇 Additional comments (11)
packages/bun-types/overrides.d.ts (1)

34-47: LGTM!

The TypeScript type definition is well-documented with a clear example. Using AsyncIterable<unknown> is appropriate since JSONL lines can contain any valid JSON type.

src/bun.js/bindings/webcore/JSReadableStream.cpp (2)

139-151: LGTM!

The host function follows the established pattern for other ReadableStream consumer methods (text, json, blob, bytes). The this validation and error handling are correct.


185-185: LGTM!

The HashTableValue entry correctly registers the jsonl method on the ReadableStream prototype following the pattern of other native functions. As per coding guidelines, properties are defined using HashTableValue arrays for JavaScript class bindings.

src/js/builtins/ReadableStream.ts (1)

333-379: LGTM!

The implementation correctly handles:

  • Input validation with appropriate error types
  • Chunked data across JSON boundaries via buffering
  • Empty and whitespace-only lines (skipped)
  • Trailing content without a final newline
  • Proper UTF-8 decoding with { stream: true } for multi-byte character boundaries
  • Resource cleanup in the finally block

The synchronous throw for locked streams (vs Promise.$reject in other methods) is appropriate since jsonl() returns an AsyncIterable, not a Promise.

src/bun.js/bindings/ZigGlobalObject.h (2)

467-467: LGTM!

The GC member m_readableStreamToJSONL is correctly added to the macro, following the pattern of existing readableStream bindings. This ensures proper garbage collection handling for the cached function. As per coding guidelines, structures are cached in ZigGlobalObject for JavaScript class bindings.


832-832: LGTM!

The extern "C" declaration follows the established pattern for other readableStream bindings.

test/js/web/streams/readablestream-jsonl.test.ts (4)

1-19: LGTM!

Comprehensive test setup with proper imports from bun:test and harness. The basic JSONL parsing test correctly validates the core functionality.


128-145: LGTM!

Excellent use of using for automatic server cleanup and port: 0 for random port allocation, following the coding guidelines. The Response.body integration test validates end-to-end NDJSON streaming.


213-230: LGTM!

Good Bun.spawn integration test using await using for process cleanup, bunExe() and bunEnv from harness, and the -e flag for inline scripts. Correctly expects stdout results before checking exit code for better error diagnostics. As per coding guidelines.


106-126: LGTM!

The invalid JSON error handling test correctly verifies that:

  1. Valid JSON objects parsed before the error are yielded
  2. A SyntaxError is thrown when invalid JSON is encountered
src/bun.js/bindings/webcore/ReadableStream.cpp (1)

636-654: LGTM!

The implementation follows the established pattern for other readableStream bindings (JSON, Blob, Text, etc.). The function is correctly cached in globalObject->m_readableStreamToJSONL for reuse, and the code generator reference readableStreamReadableStreamToJSONLCodeGenerator(vm) properly links to the TypeScript builtin.


Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants