Skip to content

fix(runtime): WeakMap/WeakSet method VALUE reads resolve the prototype thunk - #6225

Merged
proggeramlug merged 1 commit into
mainfrom
fix/weak-method-value-read
Jul 10, 2026
Merged

fix(runtime): WeakMap/WeakSet method VALUE reads resolve the prototype thunk#6225
proggeramlug merged 1 commit into
mainfrom
fix/weak-method-value-read

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Problem

A value read of a WeakMap/WeakSet method (w.add, wm.set,
typeof w.has) returned undefined. Method calls (w.add(k)) dispatch
through js_native_call_method's weak arms, but the by-name property-read path
(js_object_get_field_by_name) had no equivalent, so a bare read yielded
undefined and any subsequent call or .bind threw:

const s = new WeakSet();
const f = s.add.bind(s, obj);   // TypeError: Bind must be called on a function

react-server-dom's client-chunk preload dedup does exactly
u.add.bind(u, chunk) on a module-level WeakSet, so every Next.js App Router
dynamic route 500'd during flight resolution (#5989).

Fix

In js_object_get_field_by_name, detect a WeakMap/WeakSet receiver (via the
existing GC-header-safe weak_class_id_from_receiver, already guarded against
the small-handle band) and, for a known method name with no own-key shadow,
return the brand-checking prototype thunk from
collection_proto_method_value — the same thunk Map/Set value reads already
resolve. Own keys keep precedence (a fresh instance only carries the internal
__perry_wk_entries sentinel, which is not a method name).

Validation

New gap test test_gap_weak_method_value_read.ts, byte-for-byte against
node --experimental-strip-types: typeof reads, .bind(...) + call,
stored-method .call(...), WeakMap.set/get round-trip, the bound value being
a real function (.bind/.call present), a method-call regression guard, and
own-key precedence over a shadowed method name.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed method lookups on WeakMap and WeakSet instances so methods such as get, set, has, delete, and add return callable functions as expected.
    • Preserved support for detached and bound method calls.
    • Ensured custom instance properties continue to override prototype methods.

…e thunk

A value read of a WeakMap/WeakSet method (`w.add`, `wm.set`, `typeof w.has`)
returned undefined: method CALLS dispatch via js_native_call_method's weak
arms, but the by-name read path had no equivalent, so `const f = w.add` /
`w.add.bind(w, k)` yielded undefined and the subsequent bind/call threw
"Bind must be called on a function".

react-server-dom's chunk-preload dedup does `u.add.bind(u, chunk)` on a
module-level WeakSet, 500'ing every Next.js App Router dynamic route.

Detect a WeakMap/WeakSet receiver in js_object_get_field_by_name (via the
GC-header-safe weak_class_id_from_receiver) and return the brand-checking
prototype thunk for a known method name with no own-key shadow — mirroring the
Map/Set value-read path.

Adds test_gap_weak_method_value_read.ts, byte-for-byte against node.
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b5c2f72-17fa-47ea-8918-4a903388f0b1

📥 Commits

Reviewing files that changed from the base of the PR and between 7b54ac6 and 089ebf4.

📒 Files selected for processing (2)
  • crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
  • test-files/test_gap_weak_method_value_read.ts

📝 Walkthrough

Walkthrough

WeakMap and WeakSet field lookup now returns callable prototype methods for recognized method names when no own property shadows them. A regression test covers bound calls, method-value calls, direct calls, function characteristics, and own-property precedence.

Changes

Weak collection method values

Layer / File(s) Summary
WeakMap and WeakSet method resolution
crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
Adds a weak collection slow path that maps recognized method names to prototype thunk values while preserving own-property lookup precedence.
Method value regression coverage
test-files/test_gap_weak_method_value_read.ts
Tests WeakMap and WeakSet method reads through bind and call, direct invocation, callable function properties, and own-property shadowing.

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

Possibly related PRs

  • PerryTS/perry#5550: Addresses related WeakMap/WeakSet method handling during HIR pre-scanning and lowering.
  • PerryTS/perry#5851: Modifies related runtime field resolution and weak method prototype dispatch.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it doesn't follow the repository template and omits required sections like Summary, Changes, Related issue, and Test plan. Rewrite it to match the template headings and add a Related issue, a concrete Test plan with commands run, and the checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main runtime change: WeakMap/WeakSet method value reads now resolve to prototype thunks.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/weak-method-value-read

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.

@proggeramlug
proggeramlug merged commit 9adddea into main Jul 10, 2026
25 checks passed
@proggeramlug
proggeramlug deleted the fix/weak-method-value-read branch July 10, 2026 09:12
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.

1 participant