fix(bun): polyfill ReadableStream to strip unsupported type direct#4265
fix(bun): polyfill ReadableStream to strip unsupported type direct#4265harshagarwalnyu wants to merge 3 commits into
Conversation
React 19's server.edge.js creates ReadableStream({ type: "direct", ... })
using a Cloudflare Workers-specific extension. Bun follows the web spec
strictly and throws ERR_INVALID_ARG_VALUE for unknown type values, breaking
prerender builds with the bun preset.
Add a global ReadableStream wrapper in the bun runtime entry that strips
the type property when its value is "direct" before delegating to the
native constructor. Prototype chain is preserved so instanceof checks pass.
Fixes nitrojs#4259
|
@harshagarwalnyu is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Bun preset overrides ChangesReadableStream Compatibility Override
🎯 3 (Moderate) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/presets/bun/runtime/bun.ts`:
- Around line 9-32: The wrapper breaks instanceof because instances are created
with _OriginalReadableStream.prototype but globalThis.ReadableStream.prototype
is a different object; to fix it, make the wrapper reuse the original prototype
object instead of inheriting from it: after defining the wrapper function
(globalThis.ReadableStream) assign globalThis.ReadableStream.prototype =
_OriginalReadableStream.prototype (and if needed restore constructor on that
prototype to point to globalThis.ReadableStream), and keep
Object.setPrototypeOf(globalThis.ReadableStream, _OriginalReadableStream) only
if you need constructor inheritance—this ensures instanceof checks against
globalThis.ReadableStream succeed because both constructors share the exact same
prototype object used when instances are created by _OriginalReadableStream.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e9759a2-e3a0-4f90-83f5-a2d06eedaee4
📒 Files selected for processing (1)
src/presets/bun/runtime/bun.ts
There was a problem hiding this comment.
Pull request overview
Adds a Bun-preset runtime compatibility shim so React 19’s ReadableStream({ type: "direct" }) (Cloudflare Workers extension) won’t crash prerendering/builds under Bun by stripping the unsupported type: "direct" before delegating to the native ReadableStream constructor.
Changes:
- Overrides
globalThis.ReadableStreamin the Bun runtime entry to removeunderlyingSource.type === "direct". - Attempts to preserve constructor/prototype chaining by setting prototype links back to the original
ReadableStream.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/presets/bun.test.ts (1)
28-33: ⚡ Quick winExpand test coverage to verify key polyfill behaviors.
The test only checks a boolean flag (
data.isStream), but doesn't verify the core behaviors mentioned in the PR objectives and commit message:
- That the polyfill actually handles
type: "direct"correctly (strips it before delegation)- That the
ReadableStreamprototype chain is preserved- That the returned value is an actual
ReadableStreaminstanceConsider adding assertions to verify these aspects:
📋 Suggested additional assertions
it("bun: ReadableStream polyfill works", async () => { const { data } = await callHandler({ url: "/bun-direct-stream" }); // Existing assertion expect(data.isStream).toBe(true); // Verify it's actually a ReadableStream instance with correct prototype // (requires the endpoint to return the actual stream or metadata about it) expect(data.hasCorrectPrototype).toBe(true); // Verify the stream was created with type: "direct" that got stripped expect(data.wasDirectType).toBe(true); });Note: This assumes the fixture endpoint returns sufficient metadata. You may need to adjust the fixture route to expose these details for testing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/presets/bun.test.ts` around lines 28 - 33, The test "it(\"bun: ReadableStream polyfill works\")" only checks data.isStream; update the test to also assert that the returned value is an actual ReadableStream instance (use instanceof ReadableStream or equivalent check), that the stream's prototype chain is preserved (assert data.hasCorrectPrototype or inspect Object.getPrototypeOf on the returned object), and that the polyfill stripped type: "direct" before delegation (assert data.wasDirectType or add an endpoint metadata field indicating the original type was "direct"); if the fixture at "/bun-direct-stream" doesn't already return these metadata fields, modify that route to expose hasCorrectPrototype and wasDirectType so the test can assert them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/presets/bun.test.ts`:
- Around line 28-33: The test "it(\"bun: ReadableStream polyfill works\")" only
checks data.isStream; update the test to also assert that the returned value is
an actual ReadableStream instance (use instanceof ReadableStream or equivalent
check), that the stream's prototype chain is preserved (assert
data.hasCorrectPrototype or inspect Object.getPrototypeOf on the returned
object), and that the polyfill stripped type: "direct" before delegation (assert
data.wasDirectType or add an endpoint metadata field indicating the original
type was "direct"); if the fixture at "/bun-direct-stream" doesn't already
return these metadata fields, modify that route to expose hasCorrectPrototype
and wasDirectType so the test can assert them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 405bed12-79a3-41fb-90d1-4a0f661e87d1
📒 Files selected for processing (3)
src/presets/bun/runtime/bun.tstest/fixture/server/routes/bun-direct-stream.tstest/presets/bun.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/fixture/server/routes/bun-direct-stream.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/presets/bun/runtime/bun.ts
| // Bun's constructor so prerendering works without switching to the node preset. | ||
| // Using class extends preserves the prototype chain so instanceof checks work correctly. | ||
| const _OriginalReadableStream = globalThis.ReadableStream; | ||
| class _PatchedReadableStream extends _OriginalReadableStream { |
There was a problem hiding this comment.
Can you please check if there is already an upstream tracker issue we can link? (or we can make a minimal report for Bun)
Polyfill global ReadableStream in bun preset to strip type=direct (Cloudflare Workers extension) before delegating to native constructor. Prototype chain preserved. Fixes #4259