Skip to content

fix(runtime): reify all Reflect namespace members as real values - #7005

Merged
proggeramlug merged 1 commit into
PerryTS:mainfrom
jdalton:fix/reflect-member-values
Jul 30, 2026
Merged

fix(runtime): reify all Reflect namespace members as real values#7005
proggeramlug merged 1 commit into
PerryTS:mainfrom
jdalton:fix/reflect-member-values

Conversation

@jdalton

@jdalton jdalton commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Any code that stores a Reflect method in a variable and calls it later gets undefined back instead of a result — for Reflect.ownKeys that means destructuring or iterating the "key list" throws at module init.

Background on the mechanism: bundler-emitted "primordials" captures are a common hardening pattern — const ReflectOwnKeys = Reflect.ownKeys; at load time, calls through the stored binding forever after. The runtime installs the Reflect namespace's members for reflection parity (#4139), but most members were installed as noop stubs that exist as properties without being callable as values: calling one silently returns undefined. Reflect.apply and Reflect.construct had already been reified as real values for exactly this reason (#5989 — a captured-binding call from a Date extension); the other eleven members still had the stub behavior.

Minimal repro
const ReflectOwnKeys = Reflect.ownKeys;   // primordials-style capture
function recursiveFreeze(obj) {
  for (const key of ReflectOwnKeys(obj)) {  // stub returns undefined
                                            // -> "undefined is not iterable"
  }
}

Observed in the wild through a hardened-primordials library whose recursiveFreeze walked Reflect.ownKeys(obj) at class-static-init time.

Root cause

crates/perry-runtime/src/object/global_this/install_static.rs:860install_reflect_namespace_members installed get, set, has, deleteProperty, defineProperty, getOwnPropertyDescriptor, getPrototypeOf, setPrototypeOf, isExtensible, ownKeys, and preventExtensions with global_this_builtin_noop_thunk, so a call through a stored binding returned undefined with no error.

The fix adds one thunk per remaining member in bigint_promise.rs (same shape as the existing reflect_apply_thunk/reflect_construct_thunk), each routing to its already-implemented crate::proxy::js_reflect_* runtime function, and wires them into the member table. No new runtime behavior — the implementations existed; only the namespace's value-position dispatch was missing.

Test plan
$ cargo build -p perry-runtime
Finished `dev` profile
$ cargo test -p perry-runtime --lib reflect
running 5 tests
test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 1507 filtered out

Found while compiling a manifest generation CLI — a real-world TypeScript CLI with a deep pnpm dependency graph. Independent of the other fixes from that effort; lands standalone.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed Reflect methods so they now perform their intended runtime operations when stored or reused as standalone values.
    • Improved compatibility with hardened runtime behavior, preventing failures during class initialization and object reflection.
    • Updated reflection operations including property access, prototype management, extensibility checks, and key enumeration.

@jdalton
jdalton force-pushed the fix/reflect-member-values branch from 7c986e5 to d34d850 Compare July 29, 2026 13:21
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jdalton, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bba11405-d315-475a-a722-621110f7d9af

📥 Commits

Reviewing files that changed from the base of the PR and between 799eef1 and 36c8e86.

📒 Files selected for processing (4)
  • changelog.d/7005-reflect-members-real-values.md
  • crates/perry-runtime/src/object/global_this.rs
  • crates/perry-runtime/src/object/global_this/bigint_promise.rs
  • crates/perry-runtime/src/object/global_this/install_static.rs
📝 Walkthrough

Walkthrough

Reflect namespace members now resolve to concrete runtime thunks for reflection operations. The thunks delegate to proxy helpers, and the namespace installation imports and assigns those bindings instead of using noop stubs.

Changes

Reflect runtime values

Layer / File(s) Summary
Reflect operation thunks
crates/perry-runtime/src/object/global_this/bigint_promise.rs
Adds FFI thunks for Reflect operations that delegate to corresponding proxy helpers.
Reflect namespace installation
crates/perry-runtime/src/object/global_this.rs, crates/perry-runtime/src/object/global_this/install_static.rs, changelog.d/...
Imports the new thunks, installs them for the matching Reflect members, and documents the runtime-value behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • PerryTS/perry#6706: Updates setPrototypeOf invariant handling through the same js_reflect_is_extensible dispatch path.

Suggested labels: bug

Suggested reviewers: proggeramlug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the runtime fix to reify Reflect namespace members as callable values.
Description check ✅ Passed The description is detailed and on-topic, covering the problem, root cause, fix, and test plan, even though it lacks the template headings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@jdalton
jdalton force-pushed the fix/reflect-member-values branch from d34d850 to 799eef1 Compare July 29, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants