fix(runtime): EventTarget methods must live on the prototype so subclasses inherit them (#6301)#6311
Conversation
|
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 (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughEventTarget instances now carry a runtime class id, subclasses inherit EventTarget method values and dispatch behavior, listener state is seeded through class-chain recognition, and regression tests cover inheritance, overrides, binding, listeners, ChangesEventTarget subclass support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant JavaScript
participant ObjectPropertyLookup
participant EventTargetRuntime
participant NativeMethodDispatch
JavaScript->>ObjectPropertyLookup: Read inherited EventTarget method
ObjectPropertyLookup->>EventTargetRuntime: Bind method to subclass receiver
EventTargetRuntime-->>JavaScript: Return bound function
JavaScript->>NativeMethodDispatch: Invoke inherited method
NativeMethodDispatch->>EventTargetRuntime: Resolve receiver-bound method
EventTargetRuntime-->>NativeMethodDispatch: Execute EventTarget operation
NativeMethodDispatch-->>JavaScript: Return operation result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
proggeramlug
force-pushed
the
fix/6301-eventtarget-subclass
branch
from
32e7b96 to
63aa5ad
Compare
![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 `@crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs`:
- Around line 1971-1988: Move the EventTarget fallback binding from the
tail-only block into the keyless branch of get_field_by_name_object_tail, after
existing own-property, override, and prototype checks. Reuse
is_event_target_method_name and event_target_method_bind for keyless class
instances so inherited addEventListener, removeEventListener, and dispatchEvent
resolve correctly while subclass overrides continue to win; retain the existing
tail fallback for keyed objects.
🪄 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: b12fb1f6-19cc-43d2-8c71-4d8736a3de2d
📒 Files selected for processing (6)
crates/perry-codegen/src/expr/instance_misc1.rscrates/perry-runtime/src/event_target.rscrates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rscrates/perry-runtime/src/object/instanceof.rscrates/perry-runtime/src/object/native_call_method.rstest-files/test_gap_6301_event_target_subclass.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/perry-runtime/src/object/native_call_method.rs
- crates/perry-runtime/src/event_target.rs
| // #6301: `EventTarget`'s method surface, read as a VALUE | ||
| // (`typeof b.dispatchEvent`, `const add = t.addEventListener`). The | ||
| // methods have no prototype home — they were only ever compile-time | ||
| // lowerings keyed on a receiver statically named `EventTarget` — so a | ||
| // plain `new EventTarget()` read them as `undefined` and a | ||
| // `class Bus extends EventTarget {}` instance inherited nothing. | ||
| // Materialize a bound method for a receiver that is an event target by | ||
| // marker (the plain instance) or by class chain (a subclass at any | ||
| // depth). Deliberately LAST in the tail: an own property and a real | ||
| // class-vtable method (a subclass that *overrides* `dispatchEvent`) | ||
| // are resolved earlier and keep winning. | ||
| if !key.is_null() && crate::event_target::is_event_target_method_name(key_bytes) { | ||
| if let Some(bound) = | ||
| crate::event_target::event_target_method_bind(obj as *mut ObjectHeader, key_bytes) | ||
| { | ||
| return JSValue::from_bits(bound.to_bits()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Move this fallback before the keyless early return.
For class instances with keys_array == NULL, get_field_by_name_object_tail returns at Line [1521] before reaching this block. Since EventTarget methods have no prototype home, the keyless path will still return undefined for inherited addEventListener, removeEventListener, and dispatchEvent.
Invoke the same binding logic in the keyless branch after existing override/prototype checks, preserving subclass overrides.
Proposed fix
@@ keyless `keys.is_null()` branch, before its final return
+ if !key.is_null()
+ && crate::event_target::is_event_target_method_name(key_bytes)
+ {
+ if let Some(bound) = crate::event_target::event_target_method_bind(
+ obj as *mut ObjectHeader,
+ key_bytes,
+ ) {
+ return JSValue::from_bits(bound.to_bits());
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // #6301: `EventTarget`'s method surface, read as a VALUE | |
| // (`typeof b.dispatchEvent`, `const add = t.addEventListener`). The | |
| // methods have no prototype home — they were only ever compile-time | |
| // lowerings keyed on a receiver statically named `EventTarget` — so a | |
| // plain `new EventTarget()` read them as `undefined` and a | |
| // `class Bus extends EventTarget {}` instance inherited nothing. | |
| // Materialize a bound method for a receiver that is an event target by | |
| // marker (the plain instance) or by class chain (a subclass at any | |
| // depth). Deliberately LAST in the tail: an own property and a real | |
| // class-vtable method (a subclass that *overrides* `dispatchEvent`) | |
| // are resolved earlier and keep winning. | |
| if !key.is_null() && crate::event_target::is_event_target_method_name(key_bytes) { | |
| if let Some(bound) = | |
| crate::event_target::event_target_method_bind(obj as *mut ObjectHeader, key_bytes) | |
| { | |
| return JSValue::from_bits(bound.to_bits()); | |
| } | |
| } | |
| if !key.is_null() | |
| && crate::event_target::is_event_target_method_name(key_bytes) | |
| { | |
| if let Some(bound) = crate::event_target::event_target_method_bind( | |
| obj as *mut ObjectHeader, | |
| key_bytes, | |
| ) { | |
| return JSValue::from_bits(bound.to_bits()); | |
| } | |
| } |
🤖 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 `@crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs`
around lines 1971 - 1988, Move the EventTarget fallback binding from the
tail-only block into the keyless branch of get_field_by_name_object_tail, after
existing own-property, override, and prototype checks. Reuse
is_event_target_method_name and event_target_method_bind for keyless class
instances so inherited addEventListener, removeEventListener, and dispatchEvent
resolve correctly while subclass overrides continue to win; retain the existing
tail fallback for keyed objects.
There was a problem hiding this comment.
Verified empirically, and this one doesn't hold — the premise that a class instance can reach here with keys_array == NULL is false. Not moving the fallback, but I've documented the invariant and pinned it with fixtures so the next reader doesn't have to re-derive it.
Why a class X extends EventTarget instance never takes the keyless branch
Every class-instance allocator installs the shape-cached keys array unconditionally:
js_object_alloc_class_inline_keys—set_object_keys_array(ptr, keys_array)(object/alloc.rs:240)js_object_alloc_class_with_keys—set_object_keys_array(ptr, keys_arr)(object/alloc.rs:402)js_object_alloc_class_dynamic_parent— merges parent+own keys, else delegates to_with_keys(object/alloc.rs:436)
and the array they install is never NULL for a fieldless class — js_build_class_keys_array returns a zero-length array:
if field_count == 0 || packed_keys_len == 0 || packed_keys.is_null() {
let arr = crate::array::js_array_alloc_with_length_longlived(0); // <- empty, not null
...
return arr;
}(object/alloc.rs:284)
keys_array == NULL is the state of a freshly bump-allocated object (js_object_alloc, js_object_alloc_fast) before its first by-name store — i.e. Object.create(proto) results and bare js_new_function_construct instances. A class instance is never in that state.
Empirical check. I instrumented the tail with a temporary print at the keys.is_null() test and at the keyless dead-end return, and ran every shape I could think of that might produce an empty-shaped event target:
| shape | keys_array |
typeof x.dispatchEvent |
|---|---|---|
class Bus extends EventTarget {} (no fields, no ctor) |
non-null (empty) | function |
| subclass with a ctor that assigns nothing | non-null (empty) | function |
two-level B extends A extends EventTarget |
non-null (empty) | function |
new Ctor() through a variable (dynamic new) |
non-null | function |
Reflect.construct(Bus, []) |
non-null | function |
const K = class extends EventTarget {} |
non-null | function |
| class returned from a factory | non-null | function |
| subclass declared inside a function body | non-null | function |
instance whose only own prop was deleted |
non-null | function |
Object.freeze(new Bus()) |
non-null | function |
Object.create(Bus.prototype) |
NULL | function |
The KEYLESS-DEAD-END probe never fired once — for any of these, for any of the three method names.
The only receiver that does reach the keyless branch is Object.create(Bus.prototype), and it already resolves correctly inside that branch: resolve_proto_chain_field_with_receiver (the first thing the keyless branch tries, well before its return undefined()) hops to the prototype object, which is itself a shaped, class-id-stamped receiver, and that hop reaches this fallback. I also checked it doesn't leak listeners onto the shared prototype — a listener added via an Object.created receiver does not fire on a sibling new Bus(). (Node actually throws Cannot read properties of undefined (reading 'get') on that shape, since there's no internal slot — so we're strictly more permissive there, not less.)
What I changed in response
Rather than move the block (which would put it ahead of the own-property and class-vtable lookups and break the subclass-override ordering this PR exists to get right), I:
- Documented the invariant on
event_target::event_target_value_read, so the placement is justified where the mechanism lives. - Added the empty-shape cases to
test-files/test_gap_6301_event_target_subclass.ts— empty ctor, dynamicnew,Reflect.construct, class expression, function-local subclass, deleted-own-prop, frozen instance. All byte-identical tonode --experimental-strip-types, also underPERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1.
Thanks for the push on this — the invariant was load-bearing and undocumented, which is exactly what made the finding plausible.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
fd -t f alloc.rs -p crates/perry-runtime/src/object
echo "---"
rg -n "fn js_object_alloc_class_inline_keys|fn js_object_alloc_class_with_keys|fn js_object_alloc_class_dynamic_parent|fn js_build_class_keys_array" crates/perry-runtime/src/object/alloc.rs
echo "---"
rg -n -A15 "fn js_build_class_keys_array" crates/perry-runtime/src/object/alloc.rs
echo "---"
rg -n "set_object_keys_array" crates/perry-runtime/src/object/alloc.rsLength of output: 5176
Confirms the claim: every class-instance allocator path (js_object_alloc_class_inline_keys, js_object_alloc_class_with_keys, js_object_alloc_class_dynamic_parent, and the two others found at lines 523/609/681/747) calls set_object_keys_array unconditionally, and js_build_class_keys_array returns an empty (non-null) array rather than null when field_count == 0. So the keyless branch is genuinely unreachable for class instances, matching the empirical results.
@proggeramlug Thanks for the thorough empirical verification — I checked the allocator call sites directly and confirm your analysis holds: js_object_alloc_class_inline_keys, js_object_alloc_class_with_keys, and js_object_alloc_class_dynamic_parent all call set_object_keys_array unconditionally, and js_build_class_keys_array returns an empty (non-null) ArrayHeader — not NULL — when field_count == 0. So keys_array == NULL genuinely can't occur for a class instance (fieldless or not), only for the pre-first-store bump-allocated shapes you identified.
Withdrawing the original suggestion — the current placement after the keys.is_null() early return is correct, and the ordering constraint (own-property/vtable-override resolution before the EventTarget fallback) is preserved. Appreciate the documentation and fixture coverage added for this invariant.
ヽ(・∀・)ﾉ
🧬 Code Graph Analysis Results
Relevant snippets
crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs
- (around the tail’s
keys_arraynull fast-path and EventTarget value read)let keys = (*obj).keys_array;if keys.is_null() { ... }block that handles keyless receivers (including prototype-chain resolution and prototype-method lookup/vtable getter walk) and returnsJSValue::undefined()if nothing resolves.- Later (deliberately last in the tail), the
EventTarget“method surface read as VALUE” hook:if !key.is_null() { if let Some(bound) = crate::event_target::event_target_value_read(obj as *mut ObjectHeader, key_bytes) { ... } }
- Includes the in-code ordering/invariant rationale: the
EventTargetarm is after thekeys_array.is_null()early return so subclass overrides and own-property precedence remain correct.
crates/perry-runtime/src/event_target.rs
- [lines 1155-1160]
pub(crate) fn event_target_value_read(target: *mut ObjectHeader, name: &[u8]) -> Option<f64> {
if !is_event_target_method_name(name) {
return None;
}
event_target_method_bind(target, name)
}crates/perry-runtime/src/object/class_registry/construct.rs
- [lines 1827-1847]
lookup_prototype_method(...)(used by the keyless branch for JS-classic prototype method reads)
pub(crate) fn lookup_prototype_method(class_id: u32, name: &str) -> Option<f64> {
let guard = CLASS_PROTOTYPE_METHODS.read().ok()?;
let map = guard.as_ref()?;
let mut cid = class_id;
let mut depth = 0usize;
while depth < 32 {
if let Some(per_class) = map.get(&cid) {
if let Some(&bits) = per_class.get(name) {
return Some(f64::from_bits(bits));
}
}
match get_parent_class_id(cid) {
Some(p) if p != 0 && p != cid => {
cid = p;
depth += 1;
}
_ => break,
}
}
None
}crates/perry-runtime/src/object/class_registry/parent_static.rs
- [lines 1346-1349]
get_parent_class_id(...)(used during class/vtable/prototype chain walking)
pub(crate) fn get_parent_class_id(class_id: u32) -> Option<u32> {
let registry = CLASS_REGISTRY.read().unwrap();
registry.as_ref().and_then(|r| r.get(&class_id).copied())
}crates/perry-runtime/src/object/class_registry/parent_static.rs
- [lines 1356-1377]
lookup_class_method_in_chain(...)(method lookup across class parent chain; relevant to the keyless branch’s “fallback to class method in chain” logic)
pub fn lookup_class_method_in_chain(class_id: u32, name: &str) -> Option<(usize, u32, bool, bool)> {
let registry = CLASS_VTABLE_REGISTRY.read().unwrap();
let reg = registry.as_ref()?;
let mut cur = class_id;
for _ in 0..32 {
if let Some(vt) = reg.get(&cur) {
if let Some(entry) = vt.methods.get(name) {
return Some((
entry.func_ptr,
entry.param_count,
entry.has_synthetic_arguments,
entry.has_rest,
));
}
}
match get_parent_class_id(cur) {
Some(pid) if pid != 0 => cur = pid,
_ => return None,
}
}
None
}…asses inherit them (#6301) `addEventListener` / `removeEventListener` / `dispatchEvent` existed only as a compile-time lowering keyed on a receiver whose STATIC class name was literally `EventTarget` (perry-codegen/src/lower_call/event_target.rs). Nothing lived on a prototype and nothing was resolvable at runtime, so: * on a plain `new EventTarget()`, `t.addEventListener(...)` worked as a CALL but `typeof t.addEventListener` was `undefined` — there was no property; * a `class Bus extends EventTarget {}` instance has the static class name `Bus`, so the lowering never fired and it inherited NOTHING: `typeof b.dispatchEvent === "undefined"` and calling it threw `TypeError: value is not a function`. Two-level subclassing was equally dead. The `EventTarget` global also had no reserved class id, so `js_register_class_parent_dynamic` — which resolves a builtin superclass by name through `global_builtin_constructor_class_id` — left every `extends EventTarget` subclass parentless in the class registry. There was no edge to inherit across. This is the root cause of #5931: cac v7's `CAC` is `class CAC extends EventTarget` and its matched-command path calls `this.dispatchEvent(...)`, so every matching command died with `TypeError: value is not a function`. Resolve the methods off the receiver's CLASS CHAIN — the mechanism `Event` subclasses already use (`is_event_instance`) — and materialize them as bound-method closures, the established shape for this problem (`abort_signal_method_bind`, and the value-read-materializes-a-bound-method work of #6281): * event_target.rs: reserve CLASS_ID_EVENT_TARGET (0xFFFF_2406); `js_event_target_new` stamps it on the instance header (mirroring `construct_event`'s CLASS_ID_EVENT). `class_chain_is_event_target` walks the registered parent chain to it; `is_event_target` accepts a receiver that is a target either by the legacy `_eventTarget` marker (the plain instance) or by class chain (a subclass at any depth). `event_target_method_bind` mints a 1-capture bound-method closure, receiver stored as NaN-boxed bits so the GC's closure scan keeps it alive and relocates it. * object/instanceof.rs: map "EventTarget" to that id in `global_builtin_constructor_class_id`, so the `Subclass -> EventTarget` registry edge is wired at class-definition time. This is what makes inheritance work at any depth — `B extends A extends EventTarget` reaches the base through the ordinary `A -> EventTarget` edge, with no codegen walking. * object/field_get_set/get_field_by_name_tail.rs: the VALUE-read path (`typeof b.dispatchEvent`). Deliberately LAST in the tail, so an own property and a real class-vtable method (a subclass that OVERRIDES `dispatchEvent`) are resolved earlier and keep winning. * object/native_call_method.rs: the dynamic-CALL path (a fused `bus.dispatchEvent(ev)`). Placed immediately before the terminal `"<m> is not a function"` throw, so it can only turn a guaranteed failure into a working call. * perry-codegen/src/expr/instance_misc1.rs: the literal-RHS `instanceof` id table, keeping `x instanceof EventTarget` consistent with the runtime id. A subclass instance never runs `js_event_target_new`, so its hidden state (marker + listener bag + max-listener count) is seeded lazily on first listener/dispatch use (`seed_event_target_state`) — the subclass constructor lowering stays completely untouched. Every heap value there is rooted in a `RuntimeHandleScope` and re-read from its handle at each use, since both `js_object_alloc` and `key()` (string interning) can allocate and GC-move the receiver. test-files/test_gap_6301_event_target_subclass.ts is byte-identical to `node --experimental-strip-types` (also under PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1) and covers method value-reads + typeof on base and subclass, listeners firing through a subclass, two-level subclassing, a subclass with its own ctor calling `this.dispatchEvent(...)` (the cac shape), a detached bound method value, removeEventListener, per-instance listener bags, a subclass override still winning, `{ once: true }`, `instanceof EventTarget`, and Event / CustomEvent construction + `.detail` unchanged. cac v7 with a default command now runs and is byte-identical to Node (it fails with `TypeError: value is not a function` on main). Fixes #6301 Fixes #5931
…alue-read fallback Review follow-up on the placement of the #6301 value-read fallback in `get_field_by_name_object_tail`: it sits after the `keys_array == null` early return, which raised the question of whether a class instance with no own properties would return `undefined` for an inherited `addEventListener` / `removeEventListener` / `dispatchEvent`. It cannot. Every class-instance allocator (`js_object_alloc_class_inline_keys`, `js_object_alloc_class_with_keys`, `js_object_alloc_class_dynamic_parent`) installs the shape-cached keys array unconditionally, and `js_build_class_keys_array` returns a zero-LENGTH array — never NULL — for a class that declares no fields. So `class Bus extends EventTarget {}` lands on the shaped path with an empty keys array and reaches the fallback; the keyless branch is only for `Object.create(proto)` / bare `js_new_function_construct` receivers, which resolve an inherited method through the prototype-object hop (`resolve_proto_chain_field_with_receiver`) before reaching its dead end. Document that invariant at the fallback, and pin it with fixture cases whose own-key set is empty when the method is read: an empty constructor, a dynamic `new` through a variable, `Reflect.construct`, a class expression, a function-local subclass, an instance whose only own property was deleted, and a frozen instance. All byte-identical to `node --experimental-strip-types`.
…t_value_read Keeps `get_field_by_name_tail.rs` under the 2000-line file-size cap (the added rationale had pushed it to 2009) and puts the explanation next to the mechanism it describes rather than at the call site. No behaviour change: `event_target_value_read` is the existing name gate plus `event_target_method_bind`, in the same order, at the same point in the property walk.
proggeramlug
force-pushed
the
fix/6301-eventtarget-subclass
branch
from
63aa5ad to
f481f05
Compare
Rebased on
|
1 participant
Root cause
EventTarget's method surface never existed as values.addEventListener/removeEventListener/dispatchEventwere only a compile-time lowering(
perry-codegen/src/lower_call/event_target.rs) that fires when the receiver'sstatic class name is literally
EventTarget. Nothing lived on a prototype andnothing was resolvable at runtime, which produced two symptoms:
new EventTarget(),t.addEventListener(...)worked as a call(the static lowering caught it) but
typeof t.addEventListenerwas"undefined"— there was no property to read;class Bus extends EventTarget {}instance has the static class nameBus,so the lowering never fired and nothing was inherited at all:
typeof b.dispatchEvent === "undefined", and calling it threwTypeError: value is not a function. Two-level (class B extends A extends EventTarget) was equally dead.The
EventTargetglobal also had no reserved class id, sojs_register_class_parent_dynamic— which resolves a builtin superclass by namethrough
global_builtin_constructor_class_id— left everyextends EventTargetsubclass parentless in the class registry. There was no edge to inherit
across.
This is the actual root cause of #5931 ("Using cac with default command
resulting in error"): cac v7's
CACisclass CAC extends EventTarget, and itsmatched-command path calls
this.dispatchEvent(new CustomEvent(...))(
cac/dist/index.js:543), so every matching command died withTypeError: value is not a function. Verified directly — see below.Fix
Resolve the methods off the receiver's class chain — the same mechanism
Eventsubclasses already use (is_event_instance) — and materialize them asbound-method closures, which is the established shape for exactly this problem
(
abort_signal_method_bindfor AbortSignal, and thevalue-read-materializes-a-bound-method work of #6281).
event_target.rs— reserveCLASS_ID_EVENT_TARGET(0xFFFF_2406).js_event_target_newstamps it on the instance header (mirroringconstruct_event'sCLASS_ID_EVENT).class_chain_is_event_targetwalks theregistered parent chain to it, and
is_event_targetaccepts a receiver that isa target either by the legacy
_eventTargetmarker (the plain instance) orby class chain (a subclass at any depth).
event_target_method_bindmints a1-capture bound-method closure per name, with the receiver stored as NaN-boxed
bits so the GC's closure scan keeps it alive and relocates it.
object/instanceof.rs— map"EventTarget"to that id inglobal_builtin_constructor_class_id, sojs_register_class_parent_dynamicwires the
Subclass → EventTargetregistry edge at class-definition time. Thissingle line is what makes inheritance work at any depth:
B extends A extends EventTargetreaches the base through the ordinaryA → EventTargetedge, withno codegen chain-walking.
object/field_get_set/get_field_by_name_tail.rs— the value-read path(
typeof b.dispatchEvent,const add = t.addEventListener). Placeddeliberately last in the tail, so an own property and a real class-vtable
method (a subclass that overrides
dispatchEvent) are resolved earlier andkeep winning.
object/native_call_method.rs— the dynamic-call path (a fusedbus.dispatchEvent(ev)/this.addEventListener(...)). Placed immediatelybefore the terminal
"<m> is not a function"throw, so it can only ever turn aguaranteed failure into a working call; every own-field / vtable /
prototype-chain lookup above it is untouched.
perry-codegen/src/expr/instance_misc1.rs— the literal-RHSinstanceofid table, keeping
x instanceof EventTargetconsistent with the runtime id.A subclass instance never runs
js_event_target_new, so its hidden state(marker + listener bag + max-listener count) is seeded lazily on first
listener/dispatch use (
seed_event_target_state). That keeps the subclassconstructor lowering completely untouched — no
super()arm, no implicit-ctorarm, no new codegen path. Every heap value there is rooted in a
RuntimeHandleScopeand re-read from its handle at each use, because bothjs_object_allocandkey()(string interning) can allocate and GC-move thereceiver.
Before / after
main@0960415adtypeof b.dispatchEventundefinedfunctionfunctiontypeof b.addEventListenerundefinedfunctionfunctiontypeof t.addEventListener(plain base)undefinedfunctionfunctionb.dispatchEvent(new Event("x"))TypeError: value is not a functionclass B extends A extends EventTargetundefinedfunctionfunctionb instanceof EventTargetfalsetruetruecac / #5931 — verified end to end
npm i cac@7,perry.compilePackages: ["cac"], a default command plus a namedcommand,
cli.parse([...]):main@0960415ad:TypeError: value is not a function— the exactUsing cac with default command resulting in error #5931 failure.
node --experimental-strip-types.Verified
test-files/test_gap_6301_event_target_subclass.ts(new) — byte-identicalto
node --experimental-strip-types. Covers: method value-reads +typeofonthe plain base and on a subclass; listeners actually firing through a subclass;
two-level
B extends A extends EventTarget; a subclass with its ownconstructor + fields calling
this.dispatchEvent(...)(the cac shape); adetached
.bind()-ed method value;removeEventListeneractually removing;per-instance listener bags (no cross-instance leakage); a subclass override
still winning over the inherited method;
{ once: true };instanceof EventTargetfrom base, subclass and two-level subclass; andEvent/CustomEventconstruction +.detailunchanged.PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1(the lazy seeding and the bound-method capturessurvive a moving collection).
cargo test -p perry-runtime: 1264 passed, 0 failed (--test-threads=1).The parallel run shows one failure in
url::node_compat::tests::path_to_file_url_posix_preserves_relative_trailing_slash— a cwd-dependent test that passes in isolation and on an unmodified
origin/mainworktree; unrelated to this change.cargo fmt --all -- --checkclean;scripts/check_file_size.sh,scripts/gc_store_site_inventory.pyand the addr-class audit all pass on thefiles this PR touches. (The addr-class gate is already red on clean
mainforcrates/perry-runtime/src/map.rs— the pre-existing ratchet that fix(runtime): Map/BigInt bare-address branches must reject the handle bands (unblocks main) #6297addresses, not this branch.)
origin/mainbuild:test_gap_events_import_4995andtest_issue_850_eventemitterdivergeidentically before and after — both are pre-existing EventEmitter gaps, not
regressions from this PR.
Scope note
super.dispatchEvent(event)from inside a subclass override still does not reachthe base. That is a pre-existing, orthogonal limitation of
super.<method>()into any native base —
class E extends EventEmitter { emit() { … super.emit(…) } }is worse today (the override does not even run) — and it wantsits own issue rather than a change to the super-call lowering here.
Fixes #6301
Fixes #5931
Summary by CodeRabbit
EventTargetnow exposesaddEventListener,removeEventListener, anddispatchEventas real method values that work when inherited and accessed dynamically (including detached reads).EventTargetis now correctly recognized byinstanceof, covering plain instances and subclasses.EventTargetinstances to support consistent per-instance behavior.{ once: true }, and listener removal.dispatchEventoverrides take precedence over the inherited implementation.