diff --git a/changelog.d/10805-stream-legacy-heritage.md b/changelog.d/10805-stream-legacy-heritage.md new file mode 100644 index 0000000000..6d103fc69e --- /dev/null +++ b/changelog.d/10805-stream-legacy-heritage.md @@ -0,0 +1,12 @@ +### Fixed + +- **`class X extends Stream` (the bare `node:stream` base) — `pipe()`, the + EventEmitter listener/emit surface, and `instanceof Stream` are no longer + missing.** `#10649` fixed the analogous dynamic-heritage dispatch for + `Readable`/`Writable`/`Duplex`/`Transform` but stopped short of `Stream` — + the base those four derive from. Every heritage shape (bare import, + namespace member, or CJS destructured `require('stream')`) now installs + the correct surface, matching Node byte-for-byte. `instanceof Stream` is + scoped to genuine `extends Stream` subclasses only — a plain + `extends EventEmitter` class does not newly satisfy it. `PassThrough` + (`#10745`) is a separate, deeper gap and remains unaffected. diff --git a/crates/perry-runtime/src/node_stream_constructors.rs b/crates/perry-runtime/src/node_stream_constructors.rs index 5476ca631f..1dae81e4f5 100644 --- a/crates/perry-runtime/src/node_stream_constructors.rs +++ b/crates/perry-runtime/src/node_stream_constructors.rs @@ -362,11 +362,11 @@ mod web_adapter; pub use builders::{ js_array_subclass_init, js_event_emitter_async_resource_subclass_init, js_event_emitter_subclass_init, js_node_stream_duplex_new, js_node_stream_duplex_subclass_init, - js_node_stream_passthrough_new, js_node_stream_readable_from, - js_node_stream_readable_from_options, js_node_stream_readable_new, - js_node_stream_readable_subclass_init, js_node_stream_transform_new, - js_node_stream_transform_subclass_init, js_node_stream_writable_new, - js_node_stream_writable_subclass_init, + js_node_stream_legacy_subclass_init, js_node_stream_passthrough_new, + js_node_stream_readable_from, js_node_stream_readable_from_options, + js_node_stream_readable_new, js_node_stream_readable_subclass_init, + js_node_stream_transform_new, js_node_stream_transform_subclass_init, + js_node_stream_writable_new, js_node_stream_writable_subclass_init, }; pub use introspection::{ diff --git a/crates/perry-runtime/src/node_stream_constructors/builders.rs b/crates/perry-runtime/src/node_stream_constructors/builders.rs index bae862622e..0f9a5bff6a 100644 --- a/crates/perry-runtime/src/node_stream_constructors/builders.rs +++ b/crates/perry-runtime/src/node_stream_constructors/builders.rs @@ -93,6 +93,37 @@ pub extern "C" fn js_event_emitter_subclass_init(this: f64) -> f64 { this } +/// #10798: install the legacy `node:stream` `Stream` base surface onto +/// `this` for a source-compiled `class X extends Stream` (the bare +/// `node:stream` base — NOT one of its Readable/Writable/Duplex/Transform +/// subclasses). In Node, `Stream` is EventEmitter plus exactly one added +/// prototype method: `pipe()` (`lib/internal/streams/legacy.js`). It has no +/// `_readableState`/`_writableState`/etc, so — unlike the stream-state +/// inits above — there is no option-driven state to seed here either; this +/// is `js_event_emitter_subclass_init` plus the one extra method. `ns_pipe2` +/// is the same generic, receiver-keyed pipe implementation +/// Readable/Duplex/Transform install (`readable_methods`/ +/// `duplex_methods` in `node_stream_readwrite.rs` / +/// `node_stream_duplex_methods.rs`); it drives itself entirely off `on`/ +/// `emit` on the source and destination, so it works unmodified on a plain +/// EventEmitter-shaped receiver that never went through a stream +/// constructor. +#[no_mangle] +pub extern "C" fn js_node_stream_legacy_subclass_init(this: f64) -> f64 { + let raw = raw_ptr_from_value(this); + if raw == 0 { + return this; + } + if unsafe { gc_type_for_ptr(raw) } != Some(crate::gc::GC_TYPE_OBJECT) { + return this; + } + let obj = raw as *mut ObjectHeader; + let mut methods: Vec<(&str, StubFn)> = emitter_methods().to_vec(); + methods.push(("pipe", cast2(ns_pipe2))); + install_methods_on_existing_object(obj, this, &methods, &[]); + this +} + /// Initialize a source-compiled subclass of EventEmitterAsyncResource on its /// already-allocated `this` object. The listener surface remains the generic /// object-backed EventEmitter implementation; a hidden AsyncResource supplies diff --git a/crates/perry-runtime/src/object/class_registry/parent_static.rs b/crates/perry-runtime/src/object/class_registry/parent_static.rs index bedfd46ad9..9820df8c63 100644 --- a/crates/perry-runtime/src/object/class_registry/parent_static.rs +++ b/crates/perry-runtime/src/object/class_registry/parent_static.rs @@ -221,13 +221,31 @@ pub extern "C" fn js_register_class_parent_dynamic(class_id: u32, mut parent_val // #10430: the legacy `Stream` constructor extends EventEmitter, so a // `class X extends require('stream')` subclass inherits the same // EventEmitter parent edge (`new X() instanceof EventEmitter`). - let parent = match method.as_str() { - "EventEmitter" | "Stream" => 0xFFFF0076, - "EventEmitterAsyncResource" => 0xFFFF0077, - _ => 0, - }; - if parent != 0 { - register_class(class_id, parent); + // + // #10798: `Stream` gets its OWN hop in the chain — the reserved id + // `instanceof/static_dispatch.rs` already uses to NAME 0xFFFF0070 + // as "Stream" — rather than collapsing straight onto EventEmitter's + // id. `js_instanceof` walks the full class-id chain + // (`subclass_of_builtin_reaches` / `class_chain_reaches`), so + // registering `class_id -> CLASS_ID_STREAM -> CLASS_ID_EVENT_EMITTER` + // keeps `instanceof EventEmitter` true transitively while making + // `instanceof Stream` true ONLY for a genuine `extends Stream` + // subclass — a plain `extends EventEmitter` class (registered + // directly on 0xFFFF0076, no Stream hop) must NOT satisfy + // `instanceof Stream`, and collapsing both onto the same id would + // have made it. The Stream->EventEmitter edge is registered on + // every call; `register_class` no-ops when the edge already + // matches, so this is idempotent. + match method.as_str() { + "EventEmitter" => register_class(class_id, 0xFFFF0076), + "Stream" => { + const CLASS_ID_STREAM: u32 = 0xFFFF0070; + const CLASS_ID_EVENT_EMITTER: u32 = 0xFFFF0076; + register_class(CLASS_ID_STREAM, CLASS_ID_EVENT_EMITTER); + register_class(class_id, CLASS_ID_STREAM); + } + "EventEmitterAsyncResource" => register_class(class_id, 0xFFFF0077), + _ => {} } } return; diff --git a/crates/perry-runtime/src/object/global_this/fetch_globals.rs b/crates/perry-runtime/src/object/global_this/fetch_globals.rs index b8e6d4a3bd..ca5d1051f2 100644 --- a/crates/perry-runtime/src/object/global_this/fetch_globals.rs +++ b/crates/perry-runtime/src/object/global_this/fetch_globals.rs @@ -778,6 +778,29 @@ pub unsafe extern "C" fn js_fetch_or_value_super( // shim reads is never pre-seeded for ANY `PassThrough` heritage shape — // that's a separate, deeper HIR-level gap needing its own fix; adding an // arm here alone was confirmed (empirically) to change nothing. + // + // #10798: `Stream` (the legacy `node:stream` base that `Readable` and + // friends themselves derive from) is a DIFFERENT shape than + // `PassThrough`: it carries no hidden per-instance state at all — in + // Node it is literally `EventEmitter` plus a `pipe()` prototype method + // (`lib/internal/streams/legacy.js`: `Stream(opts) { EventEmitter.call(this, + // opts); }`), so there is no `_readableState`/`_transform`-shaped field + // that needs pre-seeding, and no `js_node_stream_stream_subclass_init` + // is needed (there isn't one, and adding one would duplicate + // `js_event_emitter_subclass_init` for no reason). `canonical_native_parent_name` + // does not list `Stream` either, so — unlike Readable/Writable/Duplex/ + // Transform, which have a fast STATIC path for a plain `import` and only + // fall here for the aliased/namespace/CJS-destructured shapes — every + // `extends Stream` heritage shape (bare ident, namespace member, + // destructured CJS `require`) already reaches this dynamic dispatch + // uniformly. Reuse the existing EventEmitter shim rather than adding a + // stream-specific one: it installs the identical `.on`/`.emit`/`.once`/… + // surface Stream needs, and `pipe()` resolves through the ordinary + // prototype chain once the parent edge is wired (unaffected by this + // arm). `Stream` IS a real constructor with a usable prototype in + // Perry's runtime (`bound_native_callable_export_value("stream", + // "Stream")`, #10430's `new Stream()` fix), so — unlike `PassThrough` — + // this one-line dispatch arm is not a no-op. if let Some((module, method)) = bound_native_parent.as_ref() { if super::super::native_module::normalize_native_module_alias(module.as_str()) == "stream" { let opts = if args_len >= 1 && !args_ptr.is_null() { @@ -798,6 +821,9 @@ pub unsafe extern "C" fn js_fetch_or_value_super( "Transform" => Some(crate::node_stream::js_node_stream_transform_subclass_init( this_box, opts, )), + "Stream" => Some(crate::node_stream::js_node_stream_legacy_subclass_init( + this_box, + )), _ => None, }; if handled.is_some() { diff --git a/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs b/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs index f768a52cbb..ef93b4d17a 100644 --- a/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs +++ b/crates/perry-runtime/src/object/instanceof/dynamic_dispatch.rs @@ -169,7 +169,18 @@ pub extern "C" fn js_instanceof_dynamic(value: f64, type_ref: f64) -> f64 { "Readable" | "Writable" | "Duplex" | "Transform" | "PassThrough" | "Stream" ) && (crate::node_stream::is_classic_stream_instance_of(value, method.as_str()) - || super::tls_constructor_prototype_is_instance_of(value, method.as_str())) + || super::tls_constructor_prototype_is_instance_of(value, method.as_str()) + // #10798: a genuine `class X extends Stream` subclass is a real + // ObjectHeader carrying its own class id, chained through the + // dedicated Stream hop (`class_registry::parent_static`'s + // `js_register_class_parent_dynamic`) rather than prototype- + // linked to the real `Stream.prototype` — so it is invisible to + // `is_classic_stream_instance_of`'s own-field probe above + // (which answers the DIRECT `new Readable()`-shaped case). Walk + // the class-id chain the same way the EventEmitter branch below + // does for its own subclass case. + || (method == "Stream" + && js_instanceof(value, 0xFFFF0070).to_bits() == crate::value::TAG_TRUE)) { return f64::from_bits(crate::value::TAG_TRUE); } diff --git a/test-files/test_gap_10798_stream_bare_heritage.ts b/test-files/test_gap_10798_stream_bare_heritage.ts new file mode 100644 index 0000000000..e6d195d87b --- /dev/null +++ b/test-files/test_gap_10798_stream_bare_heritage.ts @@ -0,0 +1,65 @@ +// #10798: `class X extends Stream` (the bare legacy `node:stream` base, not +// one of its subclasses) threw `TypeError: ... is not a constructor`. This +// blocked `nodemailer`, which loads `class XOAuth2 extends Stream` on +// import. +// +// PR #10649 fixed the analogous dynamic-heritage-dispatch gap in +// `js_fetch_or_value_super` (crates/perry-runtime/src/object/global_this/ +// fetch_globals.rs) for `Readable`/`Writable`/`Duplex`/`Transform`, but its +// match list stopped short of `Stream` — the base those four themselves +// derive from. +// +// Unlike #10745 (`PassThrough`), this is NOT the deeper HIR-level gap: +// `canonical_native_parent_name` (crates/perry-hir/src/lower_decl/ +// class_decl.rs) never recognized ANY spelling of `Stream` as a native +// parent — unlike Readable/Writable/Duplex/Transform, which have a fast +// static path for a plain `import`, EVERY `extends Stream` heritage shape +// (bare ident, namespace member, CJS destructured `require`) already +// reaches the same dynamic `js_fetch_or_value_super` dispatch #10649 +// patches, uniformly. And `Stream` carries no hidden per-instance state to +// pre-seed in the first place — in Node it is literally `EventEmitter` plus +// a `pipe()` prototype method (`lib/internal/streams/legacy.js`), so the +// fix reuses the existing `js_event_emitter_subclass_init` shim rather than +// adding a stream-specific one. +import { Stream as ImportedStream } from "node:stream"; +import * as streamNs from "node:stream"; +import { createRequire } from "node:module"; + +const req = createRequire(import.meta.url); +const cjsStreamModule: any = req("stream"); +const { Stream: RequiredStream } = cjsStreamModule; + +class BareTap extends ImportedStream {} +class NamespaceTap extends streamNs.Stream {} +class CjsTap extends RequiredStream {} + +function run(name: string, T: any) { + let t: any; + try { + t = new T(); + } catch (e) { + console.log(name, "THREW (construct)", (e as Error).message); + return; + } + let got = 0; + const seen: string[] = []; + t.on("data", (c: any) => { + got++; + seen.push(String(c)); + }); + t.emit("data", "a"); + t.emit("data", "b"); + console.log( + name, + "count:", got, + "values:", seen.join(","), + "typeof pipe:", typeof t.pipe, + "typeof on:", typeof t.on, + "typeof once:", typeof t.once, + "instanceof Stream:", t instanceof ImportedStream, + ); +} + +run("bare extends Stream ", BareTap); +run("namespace extends stream.Stream", NamespaceTap); +run("cjs destructured extends Stream", CjsTap);