fix(link): route emitted js_event_emitter_* symbols to perry-ext-events (#5140)#5185
Conversation
…ts (#5140) `new EventEmitter()` / `.on` / `.emit` / `.removeAllListeners` lower to `js_event_emitter_*` helpers purely off the class NAME being `EventEmitter` (`lower_call/builtin.rs` + the events native-table rows), regardless of which package the binding came from. The canonical implementations live in `perry-ext-events` — the `[bindings.events]` well-known crate — and `import "events"` flips that wrapper onto the link line via `ctx.native_module_imports`. But a program that gets `EventEmitter` from a *different* package — e.g. `import EventEmitter from "eventemitter3"` under `perry.compilePackages` — never inserts `"events"` into the import set, so the wrapper stays off the link line and the build fails with: Undefined symbols for architecture arm64: "_js_event_emitter_emit" "_js_event_emitter_new_with_options" "_js_event_emitter_on" "_js_event_emitter_remove_all_listeners" Tag the emitted core EventEmitter symbols in the codegen FFI provenance registry as `WellKnown("events")` so the well-known flip fires off codegen provenance instead of imports — the same mechanism the #846 / #3954 http/net rows already use. Flipping `"events"` links perry-ext-events, strips the duplicate `bundled-events` stdlib feature, and activates `external-events-construct` (the default-import dynamic-new path, #4995). Verified: `import EventEmitter from "eventemitter3"; …` now links and prints `5` (matching Node); `import { EventEmitter } from "events"` still prints `5`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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)
📝 WalkthroughWalkthrough
ChangesEventEmitter Symbol Routing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Summary
Fixes #5140 — compiling a program that imports
eventemitter3(or any package whose default/named export is anEventEmitterclass) fails at link time withUndefined symbols: _js_event_emitter_*.Root cause
new EventEmitter()/.on/.emit/.removeAllListenerslower to the nativejs_event_emitter_*helpers purely off the class name beingEventEmitter(lower_call/builtin.rs+ the events native-table rows) — regardless of which package the binding was imported from. The canonical implementations live inperry-ext-events, the[bindings.events]well-known crate.import "events"flips that wrapper onto the link line by inserting"events"intoctx.native_module_imports.A program that gets
EventEmitterfrom a different package — e.g.import EventEmitter from "eventemitter3"underperry.compilePackages— never inserts"events"into the import set, soperry-ext-eventsstays off the link line even though codegen emitted calls into it. The link then fails:Fix
Tag the codegen-emitted core
js_event_emitter_*symbols in the FFI provenance registry (ext_registry.rs) asWellKnown("events"). This makes the well-known flip fire off codegen provenance instead of imports — the exact same mechanism the existing#846/#3954http/net rows already use for symbols emitted without a matching source-level import.Flipping
"events"does three things, all already wired:perry-ext-eventsonto the link line,bundled-eventsperry-stdlib feature (so there's no duplicate-symbol conflict —optimized_libs.rsline ~366), andexternal-events-construct, which the default-import dynamic-newpath relies on (#4995).Only the core surface defined by
perry-ext-eventsis listed; thejs_event_emitter_async_resource_*helpers live in perry-stdlib and are out of scope (EventEmitterAsyncResourceisnode:events-only).Testing
emitted_event_emitter_symbols_route_to_events(all 5ext_registrytests pass).import EventEmitter from "eventemitter3") now links and prints5(matching Node).import { EventEmitter } from "events"still links and prints5(no regression).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests