fix(hir,runtime,stdlib): bare events specifier resolves to full EventEmitter (#4995)#5002
Merged
Merged
Conversation
…ntEmitter (#4995) The bare core specifier `events` (default import, namespace import, or `require('events')`) produced an EventEmitter whose instances had an empty prototype — `.on`/`.emit`/`.setMaxListeners` were all undefined — while `node:events` worked. This blocked signal-exit (cli-cursor → restore-cursor), the next ink wall after #4993. Two layers, both fixed: - HIR (expr_new.rs, expr_member.rs): `new EE()` over an events default import / namespace import / CJS alias now lowers to the real `EventEmitter` constructor (Expr::New { class_name: "EventEmitter" }) instead of the empty-object placeholder. Emitter method-value reads on a native `events` instance are gated on module name (not the canonical class name) so an aliased constructor binding's reads stay PropertyGet rather than calling `events.on()` with no receiver. - Runtime + stdlib: a new `js_set_native_events_construct` dispatcher lets `js_new_function_construct` build a real emitter when `new` lands on a bound `events.EventEmitter` export value. The handle-dispatch arms in perry-stdlib now call the linker-resolved `extern "C"` events symbols (probe / on / constructors) instead of in-crate `crate::events::*`, so when the well-known flip links perry-ext-events, dynamic dispatch consults the same handle registry the constructors used — previously every dynamic `.on`/`.emit` on an emitter silently no-op'd. New `external-events-construct` feature wires the constructor registration when the flip strips `bundled-events`. Mirrors the sqlite duplicate-symbol contract (#643). New gap test test_gap_events_import_4995.ts — byte-identical vs node --experimental-strip-types. Zero gap-suite regressions vs baseline.
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
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.
Summary
Fixes #4995. The bare core specifier
events(default import, namespace import, orrequire('events')) produced anEventEmitterwhose instances had an empty prototype —.on/.emit/.setMaxListenersallundefined— whilenode:eventsworked. This blocked signal-exit (cli-cursor→restore-cursor), the next ink (#348) wall after #4993.Root cause — two layers, both real
1. HIR lowering.
new EE()over an events default import / namespace import / CJS alias lowered to the empty-object placeholder instead of the realEventEmitter. Separately, emitter method-value reads (typeof emitter.on) were gated on the canonical class name — butvar EE = require('events')registers the native instance under class name"EE", so an aliased read fell through to a zero-argevents.on()native call that threwERR_INVALID_ARG_TYPE.2. A latent, broader bug. Even
node:eventsdynamic dispatch (const x: any = e; x.on(...)) was fully broken in flip binaries. perry-stdlib's handle-dispatch arms called the in-cratecrate::events::*registry, but the well-known flip links perry-ext-events, whose handles that registry never sees — so the probe returned false and every dynamic.on/.emit/.setMaxListenerssilently no-op'd or readundefined.Fix
expr_new.rs,expr_member.rs): route events default/namespace/aliasnewtoExpr::New { class_name: "EventEmitter" }; gate emitter method-value reads on the module name, not the canonical class name.tags.rs,handle.rs,class_registry.rs): newjs_set_native_events_constructdispatcher sojs_new_function_constructbuilds a real emitter whennewlands on a boundevents.EventEmitterexport value.common/dispatch.rs,events/domain.rs): handle-dispatch arms (probe /on/ constructors) now call the linker-resolvedextern "C"events symbols instead of in-cratecrate::events::*, so dynamic dispatch consults the same registry the constructors used — mirroring the sqlite duplicate-symbol contract (drizzle: SQLiteInsertBase methods (_prepare, prepare, execute, returning, etc) return undefined when accessed via Any-typed receiver #643). Newexternal-events-constructfeature registers the constructor when the well-known flip stripsbundled-events.Validation
node:) + the exact signal-exitrequire('events')pattern all pass.test_gap_events_import_4995.ts— byte-identical vsnode --experimental-strip-types.test_gap_node_fsis flaky and unrelated). Runtime tests 1022/1023 (the one failure is the pre-existing macOSdatetest).Code-only per the maintainer-folds-metadata-at-merge convention (no version bump / changelog).