diff --git a/changelog.d/11028-passthrough-subclass.md b/changelog.d/11028-passthrough-subclass.md new file mode 100644 index 0000000000..1f39b18791 --- /dev/null +++ b/changelog.d/11028-passthrough-subclass.md @@ -0,0 +1,5 @@ +### Fixed + +- Honor `_transform` overrides on classes derived from `node:stream`'s + `PassThrough`, including aliased, dynamic, and indirect inheritance forms, + while preserving the default identity transform. diff --git a/crates/perry-codegen/src/codegen/helpers.rs b/crates/perry-codegen/src/codegen/helpers.rs index 584ba953f8..a1553d9dab 100644 --- a/crates/perry-codegen/src/codegen/helpers.rs +++ b/crates/perry-codegen/src/codegen/helpers.rs @@ -927,6 +927,7 @@ pub(super) fn node_stream_parent_kind( "Readable" => return Some("readable"), "Duplex" => return Some("duplex"), "Transform" => return Some("transform"), + "PassThrough" => return Some("passthrough"), _ => {} } cur = classes diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 0e77806ec2..94665e9bbf 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -900,6 +900,7 @@ pub(super) fn compile_method( Some("Writable") => Some("js_node_stream_writable_subclass_init"), Some("Duplex") => Some("js_node_stream_duplex_subclass_init"), Some("Transform") => Some("js_node_stream_transform_subclass_init"), + Some("PassThrough") => Some("js_node_stream_passthrough_subclass_init"), _ => None, }; let mut effective_parent: Option<&str> = if builtin_parent_runtime.is_some() { @@ -936,7 +937,8 @@ pub(super) fn compile_method( // inline path for dynamic-parent classes. if let Some(pname) = effective_parent.filter(|_| dynamic_parent_owner.is_none()) { let pname_owned = pname.to_string(); - let node_stream_kind = if pname_owned == "Readable" { + let node_stream_kind = if matches!(pname_owned.as_str(), "Readable" | "PassThrough") + { node_stream_parent_kind(ctx.classes, class) } else { None @@ -958,6 +960,7 @@ pub(super) fn compile_method( "readable" => "js_node_stream_readable_subclass_init", "duplex" => "js_node_stream_duplex_subclass_init", "transform" => "js_node_stream_transform_subclass_init", + "passthrough" => "js_node_stream_passthrough_subclass_init", _ => unreachable!("node stream parent kind {}", kind), }; ctx.block().call( diff --git a/crates/perry-codegen/src/expr/this_super_call.rs b/crates/perry-codegen/src/expr/this_super_call.rs index f0d5af3d44..0c4d74d0d7 100644 --- a/crates/perry-codegen/src/expr/this_super_call.rs +++ b/crates/perry-codegen/src/expr/this_super_call.rs @@ -623,6 +623,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { | "Writable" | "Duplex" | "Transform" + | "PassThrough" | "ReadableStream" | "WritableStream" | "TransformStream" @@ -785,6 +786,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "Writable" => Some("writable"), "Duplex" => Some("duplex"), "Transform" => Some("transform"), + "PassThrough" => Some("passthrough"), _ => None, }; if let Some(kind) = node_stream_kind { @@ -851,6 +853,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "Writable" => Some("writable"), "Duplex" => Some("duplex"), "Transform" => Some("transform"), + "PassThrough" => Some("passthrough"), _ => None, }; if let Some(kind) = node_stream_kind { diff --git a/crates/perry-codegen/src/expr/write_barrier.rs b/crates/perry-codegen/src/expr/write_barrier.rs index a1b6c65c82..2036188faf 100644 --- a/crates/perry-codegen/src/expr/write_barrier.rs +++ b/crates/perry-codegen/src/expr/write_barrier.rs @@ -1202,6 +1202,7 @@ pub(crate) fn lower_node_stream_super_init( "writable" => "js_node_stream_writable_subclass_init", "duplex" => "js_node_stream_duplex_subclass_init", "transform" => "js_node_stream_transform_subclass_init", + "passthrough" => "js_node_stream_passthrough_subclass_init", _ => unreachable!( "lower_node_stream_super_init: unexpected Node stream kind {}", kind diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index ab8ed03fdf..2b641a2f1a 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -1092,6 +1092,7 @@ fn lower_new_impl_inner<'a>( Some("Writable") => Some("js_node_stream_writable_subclass_init"), Some("Duplex") => Some("js_node_stream_duplex_subclass_init"), Some("Transform") => Some("js_node_stream_transform_subclass_init"), + Some("PassThrough") => Some("js_node_stream_passthrough_subclass_init"), _ => None, } } else { @@ -1390,6 +1391,7 @@ fn lower_new_impl_inner<'a>( "readable" => "js_node_stream_readable_subclass_init", "duplex" => "js_node_stream_duplex_subclass_init", "transform" => "js_node_stream_transform_subclass_init", + "passthrough" => "js_node_stream_passthrough_subclass_init", _ => unreachable!("node stream parent kind {}", kind), }; ctx.block().call( diff --git a/crates/perry-codegen/src/lower_call/new_helpers.rs b/crates/perry-codegen/src/lower_call/new_helpers.rs index 3ed72efe24..043124c67a 100644 --- a/crates/perry-codegen/src/lower_call/new_helpers.rs +++ b/crates/perry-codegen/src/lower_call/new_helpers.rs @@ -620,6 +620,7 @@ pub(super) fn node_stream_parent_kind( "Readable" => return Some("readable"), "Duplex" => return Some("duplex"), "Transform" => return Some("transform"), + "PassThrough" => return Some("passthrough"), _ => {} } if ctx.imported_class_ctors.contains_key(name) { diff --git a/crates/perry-codegen/src/runtime_decls/stdlib_ffi/streams_events.rs b/crates/perry-codegen/src/runtime_decls/stdlib_ffi/streams_events.rs index 9b7c51395c..bc4cfba3a0 100644 --- a/crates/perry-codegen/src/runtime_decls/stdlib_ffi/streams_events.rs +++ b/crates/perry-codegen/src/runtime_decls/stdlib_ffi/streams_events.rs @@ -47,6 +47,11 @@ pub(crate) fn declare_streams_events(module: &mut LlModule) { &[DOUBLE, DOUBLE], ); module.declare_function("js_node_stream_passthrough_new", DOUBLE, &[DOUBLE]); + module.declare_function( + "js_node_stream_passthrough_subclass_init", + DOUBLE, + &[DOUBLE, DOUBLE], + ); module.declare_function("js_node_stream_readable_from", DOUBLE, &[DOUBLE]); module.declare_function( "js_node_stream_readable_from_options", diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 7ad4e18935..75adeb6b7b 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -1986,5 +1986,6 @@ mod subclass_ctor_inherited_method; mod ui_widget_add_child; mod issue_10623_require_destructured_native_super; +mod issue_10745_passthrough_heritage; mod hoisted_sibling_in_later_closure; diff --git a/crates/perry-hir/src/lower/tests/issue_10745_passthrough_heritage.rs b/crates/perry-hir/src/lower/tests/issue_10745_passthrough_heritage.rs new file mode 100644 index 0000000000..ebf11535ed --- /dev/null +++ b/crates/perry-hir/src/lower/tests/issue_10745_passthrough_heritage.rs @@ -0,0 +1,28 @@ +//! #10745: `PassThrough` is a classic `node:stream` native parent just like +//! `Transform`. Both class-lowering paths must retain that identity so codegen +//! can initialize the derived object in place and honor its `_transform`. + +#[test] +fn passthrough_import_alias_is_a_native_parent_for_decls_and_expressions() { + let source = r#" + import { PassThrough as PT } from "node:stream"; + class Decl extends PT { _transform(chunk, enc, cb) { cb(null, chunk); } } + const Expr = class extends PT { _transform(chunk, enc, cb) { cb(null, chunk); } }; + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + + for name in ["Decl", "Expr"] { + let class = hir + .classes + .iter() + .find(|class| class.name == name) + .unwrap_or_else(|| panic!("{name} is lowered")); + assert_eq!(class.extends_name.as_deref(), Some("PassThrough")); + assert_eq!( + class.native_extends, + Some(("node_stream".to_string(), "PassThrough".to_string())) + ); + assert!(class.extends_expr.is_none()); + } +} diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 56c34a7975..05de5e433c 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -12,7 +12,10 @@ use crate::lower_types::*; /// imports are registered under the local binding while preserving this export. fn canonical_native_parent_name<'a>(ctx: &'a LoweringContext, name: &str) -> Option<&'a str> { match ctx.lookup_native_module(name) { - Some(("stream", Some(class @ ("Readable" | "Writable" | "Duplex" | "Transform")))) + Some(( + "stream", + Some(class @ ("Readable" | "Writable" | "Duplex" | "Transform" | "PassThrough")), + )) | Some(("events", Some(class @ ("EventEmitter" | "EventEmitterAsyncResource")))) | Some(("async_hooks", Some(class @ ("AsyncLocalStorage" | "AsyncResource")))) | Some(("ws", Some(class @ "WebSocketServer"))) @@ -30,10 +33,16 @@ fn canonical_native_parent_name<'a>(ctx: &'a LoweringContext, name: &str) -> Opt /// minified local binding such as `Readable as ut`. fn is_genuine_node_stream_parent(ctx: &LoweringContext, name: &str) -> bool { match ctx.lookup_native_module(name) { - Some(("stream", Some("Readable" | "Writable" | "Duplex" | "Transform"))) => true, + Some(( + "stream", + Some("Readable" | "Writable" | "Duplex" | "Transform" | "PassThrough"), + )) => true, // Preserve the historical name-based treatment of a namespace/default // binding whose local name itself is a classic stream constructor. - Some(("stream", None)) => matches!(name, "Readable" | "Writable" | "Duplex" | "Transform"), + Some(("stream", None)) => matches!( + name, + "Readable" | "Writable" | "Duplex" | "Transform" | "PassThrough" + ), _ => false, } } @@ -236,7 +245,7 @@ pub fn lower_class_decl( // so a userland stream-shim binding (readable-stream's // `Transform`, winston) falls through to the dynamic // `extends_expr` parent path and runs its real constructor. - "Readable" | "Writable" | "Duplex" | "Transform" + "Readable" | "Writable" | "Duplex" | "Transform" | "PassThrough" if is_genuine_node_stream_parent(ctx, &parent_name) => { Some(("node_stream".to_string(), canonical_parent_name.clone())) diff --git a/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs b/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs index cf9f617a4b..6b3c554583 100644 --- a/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs +++ b/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs @@ -117,7 +117,7 @@ pub(crate) fn lower_class_from_ast( // `is_genuine_node_stream_parent` so a userland stream-shim // binding (readable-stream's `Transform`) falls through to the // dynamic `extends_expr` parent path. - "Readable" | "Writable" | "Duplex" | "Transform" + "Readable" | "Writable" | "Duplex" | "Transform" | "PassThrough" if is_genuine_node_stream_parent(ctx, &parent_name) => { Some(("node_stream".to_string(), canonical_parent_name.clone())) diff --git a/crates/perry-runtime/src/node_stream_constructors.rs b/crates/perry-runtime/src/node_stream_constructors.rs index 1dae81e4f5..50bf98b33a 100644 --- a/crates/perry-runtime/src/node_stream_constructors.rs +++ b/crates/perry-runtime/src/node_stream_constructors.rs @@ -363,10 +363,11 @@ 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_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, + js_node_stream_passthrough_subclass_init, 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 0f9a5bff6a..5f2cbc7a08 100644 --- a/crates/perry-runtime/src/node_stream_constructors/builders.rs +++ b/crates/perry-runtime/src/node_stream_constructors/builders.rs @@ -566,6 +566,24 @@ pub extern "C" fn js_node_stream_passthrough_new(opts: f64) -> f64 { passthrough } +/// Initialize `class X extends PassThrough` without replacing the derived +/// instance. A subclass-provided `_transform` wins; otherwise retain +/// PassThrough's identity transform instead of falling into Transform's +/// missing-method error. +#[no_mangle] +pub extern "C" fn js_node_stream_passthrough_subclass_init(this: f64, opts: f64) -> f64 { + let passthrough = js_node_stream_transform_subclass_init(this, opts); + if transform_hidden_callback(passthrough).is_none() { + set_hidden_value( + passthrough, + hidden_transform_passthrough_key(), + f64::from_bits(TAG_TRUE), + ); + } + init_constructor(passthrough, "PassThrough"); + passthrough +} + /// `Readable.from(iterable)` — Node's static factory. Returns a /// Readable object and retains simple iterable chunks so /// `node:stream/consumers` can drain the current stub stream surface. diff --git a/crates/perry-runtime/src/node_stream_keepalive.rs b/crates/perry-runtime/src/node_stream_keepalive.rs index 9c1654d74f..b168bc121e 100644 --- a/crates/perry-runtime/src/node_stream_keepalive.rs +++ b/crates/perry-runtime/src/node_stream_keepalive.rs @@ -196,6 +196,10 @@ static KEEP_NS_TRANSFORM_NEW: extern "C" fn(f64) -> f64 = super::js_node_stream_ static KEEP_NS_PASSTHROUGH_NEW: extern "C" fn(f64) -> f64 = super::js_node_stream_passthrough_new; #[cfg(feature = "keepalive-anchors")] #[used(compiler)] +static KEEP_NS_PASSTHROUGH_SUBCLASS_INIT: extern "C" fn(f64, f64) -> f64 = + super::js_node_stream_passthrough_subclass_init; +#[cfg(feature = "keepalive-anchors")] +#[used(compiler)] static KEEP_NS_READABLE_FROM: extern "C" fn(f64) -> f64 = super::js_node_stream_readable_from; #[cfg(feature = "keepalive-anchors")] #[used(compiler)] diff --git a/crates/perry-runtime/src/node_stream_state_tests.rs b/crates/perry-runtime/src/node_stream_state_tests.rs index 4cc4fb0547..1e61ec2241 100644 --- a/crates/perry-runtime/src/node_stream_state_tests.rs +++ b/crates/perry-runtime/src/node_stream_state_tests.rs @@ -116,6 +116,39 @@ fn stream_object_mode_flags_default_false_and_follow_options() { ); } +#[test] +fn passthrough_subclass_uses_override_or_identity_transform() { + crate::closure::js_register_closure_arity(super::tests::noop_listener as *const u8, 0); + let callback = + box_pointer(js_closure_alloc(super::tests::noop_listener as *const u8, 0) as *const u8); + + let overridden_obj = crate::object::js_object_alloc(0, 1); + js_object_set_field_by_name(overridden_obj, hidden_key(b"_transform"), callback); + let overridden = js_node_stream_passthrough_subclass_init( + box_pointer(overridden_obj as *const u8), + f64::from_bits(TAG_UNDEFINED), + ); + assert_eq!( + transform_hidden_callback(overridden).map(f64::to_bits), + Some(callback.to_bits()) + ); + assert!(!has_truthy_hidden( + overridden, + hidden_transform_passthrough_key() + )); + + let default_obj = crate::object::js_object_alloc(0, 0); + let default = js_node_stream_passthrough_subclass_init( + box_pointer(default_obj as *const u8), + f64::from_bits(TAG_UNDEFINED), + ); + assert!(transform_hidden_callback(default).is_none()); + assert!(has_truthy_hidden( + default, + hidden_transform_passthrough_key() + )); +} + #[test] fn stream_dynamic_instanceof_follows_node_stream_inheritance() { let readable = crate::object::bound_native_callable_export_value("stream", "Readable"); 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 745c2b8d87..7ae27ab868 100644 --- a/crates/perry-runtime/src/object/global_this/fetch_globals.rs +++ b/crates/perry-runtime/src/object/global_this/fetch_globals.rs @@ -790,14 +790,6 @@ pub unsafe extern "C" fn js_fetch_or_value_super( // `lower_node_stream_super_init`), so every heritage shape installs the // override onto `this` identically. // - // `PassThrough` is deliberately NOT handled here: HIR never recognizes - // it as a node:stream native parent at all, even via a bare import - // (`canonical_native_parent_name` lists Readable/Writable/Duplex/ - // Transform but not PassThrough), so the hidden `_transform` field this - // 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 @@ -840,6 +832,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, )), + "PassThrough" => Some( + crate::node_stream::js_node_stream_passthrough_subclass_init(this_box, opts), + ), "Stream" => Some(crate::node_stream::js_node_stream_legacy_subclass_init( this_box, )), diff --git a/test-files/test_gap_10745_passthrough_subclass.ts b/test-files/test_gap_10745_passthrough_subclass.ts new file mode 100644 index 0000000000..d4d645338b --- /dev/null +++ b/test-files/test_gap_10745_passthrough_subclass.ts @@ -0,0 +1,54 @@ +import { PassThrough, PassThrough as PT } from "node:stream"; + +class Direct extends PassThrough { + _transform(chunk: any, _encoding: string, callback: any) { + callback(null, "direct:" + String(chunk).toUpperCase()); + } +} + +class ImportedAlias extends PT { + _transform(chunk: any, _encoding: string, callback: any) { + callback(null, "import-alias:" + String(chunk).toUpperCase()); + } +} + +const LocalAlias = PassThrough; +const ClassExpression = class extends LocalAlias { + _transform(chunk: any, _encoding: string, callback: any) { + callback(null, "class-expr:" + String(chunk).toUpperCase()); + } +}; + +class Middle extends PassThrough {} +class Indirect extends Middle { + _transform(chunk: any, _encoding: string, callback: any) { + callback(null, "indirect:" + String(chunk).toUpperCase()); + } +} + +class DefaultPassThrough extends PassThrough {} + +function run(name: string, Constructor: any): Promise { + return new Promise((resolve) => { + const stream = new Constructor(); + let output = ""; + stream.on("data", (chunk: any) => (output += String(chunk))); + stream.on("error", (error: any) => { + console.log(name, "error", error.code || error.message); + resolve(); + }); + stream.on("end", () => { + console.log(name, JSON.stringify(output)); + resolve(); + }); + stream.end("ab"); + }); +} + +(async () => { + await run("direct", Direct); + await run("import-alias", ImportedAlias); + await run("class-expr", ClassExpression); + await run("indirect", Indirect); + await run("default", DefaultPassThrough); +})();