Skip to content

Implement ReadableStream.from - #32533

Closed
robobun wants to merge 6 commits into
mainfrom
farm/74ef6f1c/readable-stream-from
Closed

Implement ReadableStream.from#32533
robobun wants to merge 6 commits into
mainfrom
farm/74ef6f1c/readable-stream-from

ci: retrigger

d81e368
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jun 20, 2026 in 19m 31s

Code review found 3 potential issues

Found 5 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 3
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/js/builtins/ReadableStream.ts:136 Iterator-protocol violation errors lack Node's ERR_INVALID_STATE code
🟡 Nit src/jsc/bindings/webcore/JSReadableStream.cpp:157 Missing bun-types declaration for ReadableStream.from (no-DOM fallback)
🟡 Nit src/js/builtins/ReadableStream.ts:130 String(iterable) can throw, masking ERR_ARG_NOT_ITERABLE

Annotations

Check warning on line 136 in src/js/builtins/ReadableStream.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Iterator-protocol violation errors lack Node's ERR_INVALID_STATE code

Node throws these iterator-protocol violations as `TypeError`s with `code: 'ERR_INVALID_STATE'` (e.g. `ReadableStream.from({[Symbol.asyncIterator](){return 5}})` → `.code === 'ERR_INVALID_STATE'`), but here they're plain `new TypeError(...)` with no `.code`. `$ERR_INVALID_STATE_TypeError` is already used elsewhere in this file (e.g. for "ReadableStream is locked"); swapping to it here — and at the `iterator.next()` non-object check (line 144) and the `iterator.return()` non-object check (lines 1

Check warning on line 157 in src/jsc/bindings/webcore/JSReadableStream.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

Missing bun-types declaration for ReadableStream.from (no-DOM fallback)

The runtime now exposes `ReadableStream.from`, but the bun-types fallback declaration (`packages/bun-types/globals.d.ts:73-80`) wasn't updated — the no-DOM `UseLibDomIfAvailable` constructor type still has only `prototype` and two `new` overloads. Server-only Bun projects without `"DOM"` in their tsconfig `lib` will get a TS error on `ReadableStream.from(...)` even though it works at runtime; adding `from<R>(asyncIterable: AsyncIterable<R> | Iterable<R | PromiseLike<R>>): ReadableStream<R>;` to 

Check warning on line 130 in src/js/builtins/ReadableStream.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

String(iterable) can throw, masking ERR_ARG_NOT_ITERABLE

`String(iterable)` invokes ToPrimitive, which throws for null-prototype objects (and any object with a throwing `toString`/`Symbol.toPrimitive`), so `ReadableStream.from(Object.create(null))` surfaces a bare "Cannot convert object to primitive value" TypeError with no `.code` instead of `ERR_ARG_NOT_ITERABLE`. Node uses inspect-style formatting here and returns `code: 'ERR_ARG_NOT_ITERABLE'` with message `[Object: null prototype] {} must be iterable`. Wrapping `String(iterable)` in a try/catch w