fix(runtime,codegen): expose the Web Streams globals; resolve instanceof against a built-in held in a variable#6335
Merged
proggeramlug merged 3 commits intoJul 13, 2026
Conversation
…eof against a built-in held in a variable Two gaps that both surface the moment a bundler minifies. 1. `ReadableStream` / `WritableStream` / `TransformStream` were missing from `globalThis`. Perry implements all three — codegen lowers `new ReadableStream(…)` and `x instanceof ReadableStream`, and each has a class id — but the NAMES were never registered, so a bare `ReadableStream` identifier resolved to nothing: `typeof ReadableStream === "undefined"` and `"ReadableStream" in globalThis` was false, where Node exposes all three as functions. Libraries feature-detect exactly that (`typeof ReadableStream !== "undefined" ? … : …`) when deciding how to consume a `fetch()` body, so they silently took the wrong branch. 2. `js_instanceof_dynamic` returned `false` for every built-in reached through a *variable* rather than its bare name. `x instanceof Response` worked; the equivalent after `const R = Response` did not — and minifiers alias constructors as a matter of course, so in a bundle this was broadly broken. The dynamic path only understood user class objects; a built-in constructor is a `ClosureHeader` thunk, which it did not map back to a class id. It now recovers the id from the thunk's recorded name (ReadableStream / WritableStream / TransformStream / Response / Request / Headers / Blob) and defers to `js_instanceof`. Validation: node-vs-perry differential — `typeof` and `in globalThis` for the three stream constructors, `instanceof` against both the bare name and an alias, and `Response`/`Headers`/`Blob` through aliases — byte-identical to Node, where 9 of the 10 assertions previously disagreed. perry-runtime 1264 passed / 0 failed; perry-codegen 159 passed / 0 failed.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWeb Streams constructors are added to Perry’s globalThis builtin name and constructor tables. Dynamic ChangesWeb Streams builtin support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🤖 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 `@crates/perry-runtime/src/object/instanceof.rs`:
- Line 110: Update the pointer extraction in the instanceof logic to use the
canonical js_nanbox_get_pointer helper instead of JSValue::as_pointer when
decoding the NaN-boxed type_ref value, preserving the existing ClosureHeader
pointer handling.
- Around line 119-120: Update the string conversion flow around
str_bytes_from_jsvalue to check ptr.is_null() before calling
slice::from_raw_parts, returning None for a null pointer (including the
zero-length case); preserve the existing UTF-8 validation for non-null pointers.
- Around line 117-129: Update the constructor-to-class-ID logic around the
closure name lookup to resolve the constructor by identity against the current
globalThis builtins, rather than trusting the closure’s `.name` value. Only map
`ReadableStream`, `WritableStream`, `TransformStream`, `Response`, `Request`,
`Headers`, and `Blob` when the constructor is the corresponding native builtin;
otherwise return None, preserving the existing class-ID mappings.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f58df21-8a09-42c1-97ea-cd7ca040f57e
📒 Files selected for processing (3)
crates/perry-codegen/src/expr/helpers.rscrates/perry-runtime/src/object/global_this_tables.rscrates/perry-runtime/src/object/instanceof.rs
added 2 commits
July 13, 2026 08:47
…t the globalThis builtin CodeRabbit findings on the builtin-ctor brand map: use js_nanbox_get_pointer for the pointer payload; refuse a null string pointer before from_raw_parts; and require pointer identity with the constructor installed on globalThis so a user function merely NAMED Response/ReadableStream/... cannot satisfy the native brand check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two gaps that both surface the moment a bundler minifies.
1. The Web Streams globals were missing
ReadableStream/WritableStream/TransformStreamwere absent fromglobalThis. Perry implements all three — codegen lowersnew ReadableStream(…)andx instanceof ReadableStream, and each has a class id — but the NAMES were never registered, so a bareReadableStreamidentifier resolved to nothing:Libraries feature-detect exactly that (
typeof ReadableStream !== "undefined" ? … : …) when deciding how to consume afetch()body, so they silently took the wrong branch.2.
instanceoffailed for a built-in held in a variablejs_instanceof_dynamicreturnedfalsefor every built-in reached through a variable rather than its bare name:Minifiers alias constructors as a matter of course, so in a bundle this was broadly broken. The dynamic path only understood user class objects; a built-in constructor is a
ClosureHeaderthunk, which it did not map back to a class id. It now recovers the id from the thunk's recorded name (ReadableStream / WritableStream / TransformStream / Response / Request / Headers / Blob) and defers tojs_instanceof.Validation
node-vs-perry differential —
typeofandin globalThisfor the three stream constructors,instanceofagainst both the bare name and an alias, andResponse/Headers/Blobthrough aliases — byte-identical to Node, where 9 of the 10 assertions previously disagreed.perry-runtime1264 passed / 0 failed;perry-codegen159 passed / 0 failed.Summary by CodeRabbit
New Features
ReadableStream,WritableStream, andTransformStream.globalThisfor feature detection.Bug Fixes
instanceofbehavior so checks work consistently when built-in constructors (including Web Streams,Request,Response,Headers, andBlob) are assigned to variables before use.