diff --git a/changelog.d/10649-stream-subclass-heritage.md b/changelog.d/10649-stream-subclass-heritage.md new file mode 100644 index 0000000000..5e9bb57991 --- /dev/null +++ b/changelog.d/10649-stream-subclass-heritage.md @@ -0,0 +1,10 @@ +### Fixed + +- **`node:stream` subclass overrides (`_transform`/`_write`/`_read`) are no + longer ignored when the heritage reaching `class X extends ` is a + local alias, an indirect subclass, a class expression, or a CJS + destructured `require('stream')` — the shape nodemailer uses in every + stream class it defines. `write()`/`push()` used to throw + `ERR_METHOD_NOT_IMPLEMENTED` because the override was never installed on + `this`; the dynamic `super()` dispatch now recognizes the resolved + bound-export value regardless of how the heritage expression reached it. 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 13cbe05c23..b8e6d4a3bd 100644 --- a/crates/perry-runtime/src/object/global_this/fetch_globals.rs +++ b/crates/perry-runtime/src/object/global_this/fetch_globals.rs @@ -755,6 +755,56 @@ pub unsafe extern "C" fn js_fetch_or_value_super( return dispatch(this_box); } } + // #10448: `class X extends Transform` (and Readable/Writable/Duplex) + // never called the subclass's `_transform`/`_write`/`_read` override + // unless the heritage was a shape `is_genuine_node_stream_parent` + // recognizes statically (`crates/perry-hir/src/lower_decl/class_decl.rs`) + // — a local alias (`const Alias = Transform`), a namespace member reached + // through a CJS destructured `require('stream')`, or an indirect + // subclass all fell through to the ordinary-call dispatch below, + // which invokes the bound `stream` export as a plain constructor and + // drops the result — `this` stayed an empty object, so `write()` threw + // `ERR_METHOD_NOT_IMPLEMENTED`. Recognize the resolved bound-export + // value here, exactly as the WASI arm above does, and run the same + // runtime shim the static `extends Transform` path already uses + // (`js_node_stream_*_subclass_init`, `crates/perry-codegen/src/expr/write_barrier.rs`'s + // `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. + 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() { + *args_ptr + } else { + undef + }; + let handled = match method.as_str() { + "Readable" => Some(crate::node_stream::js_node_stream_readable_subclass_init( + this_box, opts, + )), + "Writable" => Some(crate::node_stream::js_node_stream_writable_subclass_init( + this_box, opts, + )), + "Duplex" => Some(crate::node_stream::js_node_stream_duplex_subclass_init( + this_box, opts, + )), + "Transform" => Some(crate::node_stream::js_node_stream_transform_subclass_init( + this_box, opts, + )), + _ => None, + }; + if handled.is_some() { + return undef; + } + } + } // `class X extends Temporal.` (non-spread `super(a, b)`): a Temporal // constructor returns a fresh NaN-boxed cell and does NOT mutate the // implicit `this`, so the ordinary dispatch below would drop that cell and diff --git a/test-files/gap_10448_stream_subclass_heritage_helper.cjs b/test-files/gap_10448_stream_subclass_heritage_helper.cjs new file mode 100644 index 0000000000..ecd23333f5 --- /dev/null +++ b/test-files/gap_10448_stream_subclass_heritage_helper.cjs @@ -0,0 +1,68 @@ +'use strict'; +// CommonJS half of test_gap_10448_stream_subclass_heritage.ts: the exact +// shape nodemailer uses everywhere (`const { Transform } = +// require('stream'); class X extends Transform`), plus the sibling +// Writable/Readable/Duplex destructured shapes the issue lists as broken +// the same way, and a namespace-member export for comparison. +// +// State is captured via public class fields, not a constructor-body +// assignment after `super(...args)` with a rest-param spread — that shape +// (`constructor(...args) { super(...args); ... }`) hits a separate, +// pre-existing gap (native stream methods go missing) independent of +// heritage shape or this issue; not exercised here to keep this test +// isolated to #10448's own defect. +const { Transform, Writable, Readable, Duplex } = require('stream'); +const stream = require('stream'); + +class CjsTransform extends Transform { + _transform(chunk, _enc, cb) { + cb(null, String(chunk).toUpperCase()); + } +} + +// `require('stream').Transform` reached via a namespace member on a plain +// `require()` result (not destructured) — control: this shape is already +// recognized statically (`is_genuine_node_stream_parent`). +class CjsViaMember extends stream.Transform { + _transform(chunk, _enc, cb) { + cb(null, String(chunk).toUpperCase()); + } +} + +class CjsWritable extends Writable { + captured = ''; + _write(chunk, _enc, cb) { + this.captured += String(chunk).toUpperCase(); + cb(); + } +} + +class CjsReadable extends Readable { + _done = false; + _read() { + if (this._done) return; + this._done = true; + this.push('x'); + this.push('y'); + this.push(null); + } +} + +// Write-half only (no `_read`/push): proves the destructured `Duplex` +// heritage installs `_write` the same way `Writable` does, without +// depending on read/write event-ordering across engines. +class CjsDuplex extends Duplex { + captured = ''; + _write(chunk, _enc, cb) { + this.captured += String(chunk).toUpperCase(); + cb(); + } +} + +module.exports = { + CjsTransform, + CjsViaMember, + CjsWritable, + CjsReadable, + CjsDuplex, +}; diff --git a/test-files/test_gap_10448_stream_subclass_heritage.ts b/test-files/test_gap_10448_stream_subclass_heritage.ts new file mode 100644 index 0000000000..e07d8d7fee --- /dev/null +++ b/test-files/test_gap_10448_stream_subclass_heritage.ts @@ -0,0 +1,167 @@ +// #10448: node:stream subclass overrides (`_transform`/`_write`/`_read`) +// were ignored whenever the heritage reaching `class X extends ` was +// anything OTHER than a shape `is_genuine_node_stream_parent` recognizes +// statically at HIR-lowering time +// (`crates/perry-hir/src/lower_decl/class_decl.rs`) — a local alias +// (`const Alias = Transform`), an indirect subclass, a class expression, or +// a CJS destructured `require('stream')` (the shape nodemailer uses in +// every stream class it defines). `write()`/`push()` then threw +// `ERR_METHOD_NOT_IMPLEMENTED` because the override was never installed on +// `this`. +// +// (`PassThrough` is a separate, deeper gap — HIR never recognizes it +// statically even via a bare import, unlike Readable/Writable/Duplex/ +// Transform, so it needs its own follow-up; not covered by this test, see +// the PR body.) +// +// Each check awaits its own stream before starting the next so output order +// is deterministic regardless of engine event-loop/nextTick scheduling +// differences — only the per-check content is the thing under test. State +// is captured via public class fields (not a `constructor(...args) { +// super(...args); ... }` rest-spread pattern, which hits an unrelated +// pre-existing gap independent of heritage shape). +import { Transform, Writable, Readable, Duplex } from "stream"; +import * as streamNs from "stream"; +import { + CjsTransform, + CjsViaMember, + CjsWritable, + CjsReadable, + CjsDuplex, +} from "./gap_10448_stream_subclass_heritage_helper.cjs"; + +const AliasTransform = Transform; + +class ViaImport extends Transform { + _transform(chunk: any, _enc: string, cb: any) { + cb(null, String(chunk).toUpperCase()); + } +} +class ViaAlias extends AliasTransform { + _transform(chunk: any, _enc: string, cb: any) { + cb(null, String(chunk).toUpperCase()); + } +} +class ViaNamespaceMember extends streamNs.Transform { + _transform(chunk: any, _enc: string, cb: any) { + cb(null, String(chunk).toUpperCase()); + } +} +class Mid extends AliasTransform {} +class ViaIndirect extends Mid { + _transform(chunk: any, _enc: string, cb: any) { + cb(null, String(chunk).toUpperCase()); + } +} +const ViaClassExpr = class extends AliasTransform { + _transform(chunk: any, _enc: string, cb: any) { + cb(null, String(chunk).toUpperCase()); + } +}; + +function runTransform(name: string, T: any): Promise { + return new Promise((resolve) => { + const t = new T(); + let out = ""; + t.on("data", (c: any) => (out += c)); + t.on("end", () => { + console.log(name, JSON.stringify(out)); + resolve(); + }); + try { + t.write("ab"); + t.end("c"); + } catch (e: any) { + console.log(name, "threw", e.code); + resolve(); + } + }); +} + +function runWritable(name: string, W: any): Promise { + return new Promise((resolve) => { + let w: any; + try { + w = new W(); + } catch (e: any) { + console.log(name, "threw (construct)", e.message); + resolve(); + return; + } + w.on("finish", () => { + console.log(name, JSON.stringify(w.captured)); + resolve(); + }); + try { + w.write("ab"); + w.end("c"); + } catch (e: any) { + console.log(name, "threw", e.code); + resolve(); + } + }); +} + +function runReadable(name: string, R: any): Promise { + return new Promise((resolve) => { + let r: any; + try { + r = new R(); + } catch (e: any) { + console.log(name, "threw (construct)", e.message); + resolve(); + return; + } + let out = ""; + r.on("data", (c: any) => (out += c)); + r.on("end", () => { + console.log(name, JSON.stringify(out)); + resolve(); + }); + }); +} + +class WViaWritable extends Writable { + captured = ""; + _write(chunk: any, _enc: string, cb: any) { + this.captured += String(chunk).toUpperCase(); + cb(); + } +} + +class RViaReadable extends Readable { + private _done = false; + _read() { + if (this._done) return; + this._done = true; + this.push("m"); + this.push("n"); + this.push(null); + } +} + +class DViaDuplex extends Duplex { + captured = ""; + _write(chunk: any, _enc: string, cb: any) { + this.captured += String(chunk).toUpperCase(); + cb(); + } +} + +async function main() { + await runTransform("Transform via import ", ViaImport); + await runTransform("Transform via alias ", ViaAlias); + await runTransform("Transform via namespace member", ViaNamespaceMember); + await runTransform("Transform via indirect subclas", ViaIndirect); + await runTransform("Transform via class expression", ViaClassExpr); + await runTransform("Transform CJS destructured ", CjsTransform); + await runTransform("Transform CJS namespace member", CjsViaMember); + await runWritable("Writable CJS destructured ", CjsWritable); + await runWritable("Duplex CJS destructured ", CjsDuplex); + await runReadable("Readable CJS destructured ", CjsReadable); + await runWritable("Writable via import ", WViaWritable); + await runReadable("Readable via import ", RViaReadable); + await runWritable("Duplex via import ", DViaDuplex); +} + +main();