Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions changelog.d/10551-stream-module-constructor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
`require('stream')` and `import Stream from "node:stream"` are now the legacy
`Stream` constructor, as in Node, instead of a separate namespace object
(#10430, #10431). Before, `x instanceof Stream` threw "Right-hand side of
'instanceof' is not callable" for both forms (node-fetch's `body instanceof
Stream`), `Stream !== NamedStream`, and nothing inherited from EventEmitter:
`require('stream').EventEmitter` was undefined, so redis's
`class ClientSideCacheProvider extends stream_1.EventEmitter` threw "Class
extends value is not a constructor" at module init. `new Stream()` also built an
empty placeholder with no `on`/`emit`, and `class X extends require('stream')`
instances had no EventEmitter methods.

Root cause: `cjs_default_export_value` had no `stream` arm, so the CommonJS
module value fell back to the namespace object; the HIR lowered the default
import's value to the bare `NativeModuleRef("stream")` (only its `typeof` was
folded to "function"); and `attach_stream_legacy_prototype` never linked
`Stream`/`Stream.prototype` to EventEmitter or hung the exports on the
constructor.

Fix: the CommonJS value and the default binding's value both resolve to the
named `Stream` export. The constructor carries every module export as an own
static (Node's own-key order, `Stream.Stream === Stream`), and gets Node's two
`ObjectSetPrototypeOf` edges (`Stream` → `EventEmitter`, `Stream.prototype` →
`EventEmitter.prototype`). `new Stream()` builds an instance of
`Stream.prototype`, and a dynamic `extends` of `Stream` gets the EventEmitter
parent edge and EventEmitter init on `super()`. The attach now roots the
constructor and prototype across its allocations. A namespace import stays the
namespace object, and member reads and calls on the default binding keep their
static lowering.

Validation: `test_gap_10430_stream_module_constructor` differs from Node on
7661bc05fe and matches it here, in both no-auto and auto-optimize modes. HIR and
runtime unit tests were added. The full gap suite matches the snapshot, with the
same 6 known mismatches as the baseline. The stream + events node-suite is
868/872 on both baseline and fix, with zero per-test deltas. Stream data paths
are flat in `instructions:u` (+0.05% and +0.22%). `instanceof Stream` alone is
+0.56–0.71% (median of 7), within the ±1% band. Startup cost is +1.1 M
instructions, paid once when the constructor is first minted.
31 changes: 31 additions & 0 deletions crates/perry-hir/src/lower/expr_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,37 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R
args,
});
}
// #10430: `new Stream()` for the legacy `node:stream` `Stream`
// constructor — the named export (any alias) or the default import,
// which IS that constructor. The by-name `Expr::New { "Stream" }`
// fallback built a prototype-less placeholder with no `on`/`emit`;
// construct the export value instead, so the runtime makes the
// instance inherit `Stream.prototype` (and through it EventEmitter).
// A namespace import keeps its builtin-module alias and is excluded.
let callee_name = callee_ident.sym.as_ref();
let is_stream_constructor_value = ctx.lookup_local(callee_name).is_none()
&& match ctx.lookup_native_module(callee_name) {
Some(("stream" | "node:stream", Some("Stream"))) => true,
Some(("stream" | "node:stream", None)) => {
ctx.lookup_builtin_module_alias(callee_name).is_none()
}
_ => false,
};
let has_spread_arg = new_expr
.args
.as_deref()
.is_some_and(|args| args.iter().any(|arg| arg.spread.is_some()));
if is_stream_constructor_value && !has_spread_arg {
return Ok(Expr::NewDynamic {
callee: Box::new(Expr::PropertyGet {
byte_offset: 0,
object: Box::new(Expr::NativeModuleRef("stream".to_string())),
property: "Stream".to_string(),
}),
args: lower_optional_args(ctx, new_expr.args.as_deref())?,
byte_offset: new_byte_offset,
});
}
// #4995: `new EE()` where `EE` is the events module *value* — the
// default import (`import EE from 'events'`) or a CJS alias
// (`var EE = require('events')`). Node's `events` module exports the
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-hir/src/lower/lower_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ mod assignment;
mod helpers;
mod json_literal;
mod reactive_text;
#[cfg(test)]
mod stream_module_value_tests;

pub(crate) use arm_bin::lower_bin_expr;
pub(crate) use arm_class::lower_class_expr;
Expand Down
20 changes: 20 additions & 0 deletions crates/perry-hir/src/lower/lower_expr/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,26 @@ pub(crate) fn native_module_binding_value(ctx: &LoweringContext, name: &str) ->
property: "default".to_string(),
};
}
// #10431: `import Stream from "node:stream"` binds `module.exports`, and
// for `stream` that is the legacy `Stream` constructor itself (which also
// carries every export as a static) — not a namespace object. Read the
// `default` export so the binding's VALUE is that constructor:
// `x instanceof Stream` needs a callable right-hand side and
// `Stream === NamedStream` needs the same object. Member reads/calls
// (`Stream.Readable`, `Stream.pipeline(…)`) are lowered from the binding,
// not from this value, and keep their static dispatch. A namespace import
// (`import * as ns`) registers a builtin-module alias and stays the
// namespace object; this is the same discriminator the `typeof` fold in
// `arm_unary` uses to report "function" for the default binding only.
if matches!(module_name, "stream" | "node:stream")
&& ctx.lookup_builtin_module_alias(name).is_none()
{
return Expr::PropertyGet {
byte_offset: 0,
object: Box::new(Expr::NativeModuleRef("stream".to_string())),
property: "default".to_string(),
};
}
// Native module reference (e.g., mysql from 'mysql2/promise')
Expr::NativeModuleRef(module_name.to_string())
}
Expand Down
92 changes: 92 additions & 0 deletions crates/perry-hir/src/lower/lower_expr/stream_module_value_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//! #10430 / #10431: the `node:stream` module value is the legacy `Stream`
//! constructor. The default binding's VALUE must read the `default` export
//! (the constructor) rather than evaluate to the namespace object, while a
//! namespace import stays the namespace; `new` of either constructor binding
//! must construct the export value instead of the by-name placeholder.

use crate::ir::{Expr, Stmt};

fn lower(source: &str) -> crate::Module {
let ast = perry_parser::parse_typescript(source, "main.ts").unwrap();
let hir = crate::lower::lower_module(&ast, "main", "main.ts").unwrap();
crate::ir::clear_current_module_source();
hir
}

fn let_init<'a>(hir: &'a crate::Module, binding: &str) -> &'a Expr {
hir.init
.iter()
.find_map(|stmt| match stmt {
Stmt::Let {
name,
init: Some(init),
..
} if name == binding => Some(init),
_ => None,
})
.unwrap_or_else(|| panic!("no `let {binding}` with an initializer"))
}

fn is_stream_export_read(expr: &Expr, export: &str) -> bool {
matches!(
expr,
Expr::PropertyGet { object, property, .. }
if property == export
&& matches!(object.as_ref(), Expr::NativeModuleRef(module) if module == "stream")
)
}

#[test]
fn default_import_value_is_the_stream_constructor() {
let hir = lower(
r#"
import Stream from "node:stream";
import Bare from "stream";
import * as ns from "node:stream";
const fromNodeSpecifier: any = Stream;
const fromBareSpecifier: any = Bare;
const namespace: any = ns;
const check = ({} as any) instanceof Stream;
"#,
);
assert!(is_stream_export_read(
let_init(&hir, "fromNodeSpecifier"),
"default"
));
assert!(is_stream_export_read(
let_init(&hir, "fromBareSpecifier"),
"default"
));
assert!(
matches!(let_init(&hir, "namespace"), Expr::NativeModuleRef(module) if module == "stream"),
"a namespace import must stay the namespace object"
);
let Expr::InstanceOf {
ty_expr: Some(rhs), ..
} = let_init(&hir, "check")
else {
panic!("expected a dynamic instanceof");
};
assert!(
is_stream_export_read(rhs, "default"),
"`x instanceof Stream` needs the callable constructor on the right-hand side"
);
}

#[test]
fn new_of_a_stream_constructor_binding_constructs_the_export_value() {
let hir = lower(
r#"
import Stream from "node:stream";
import { Stream as Aliased } from "node:stream";
const fromDefault = new Stream();
const fromAlias = new Aliased();
"#,
);
for binding in ["fromDefault", "fromAlias"] {
let Expr::NewDynamic { callee, .. } = let_init(&hir, binding) else {
panic!("`{binding}` must construct through NewDynamic");
};
assert!(is_stream_export_read(callee, "Stream"), "{binding}");
}
}
31 changes: 31 additions & 0 deletions crates/perry-runtime/src/object/class_registry/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,37 @@ pub(crate) unsafe fn nm_ctor_stream(
_ => unreachable!(),
});
}
// #10430: `new Stream()` (legacy `Stream`, i.e. `new (require('stream'))()`)
// is an ordinary instance of `Stream.prototype`, whose EventEmitter methods
// act on the receiver. Build it the way an ordinary function constructor's
// instance is built (the constructor's stable synthetic class id plus a
// class-default link to its `prototype`), not via `Object.create`, which
// mints a fresh synthetic class per call. Without this arm the instance
// had no `on`/`emit` and was not `instanceof Stream`.
if method == "Stream" {
let scope = crate::gc::RuntimeHandleScope::new();
let ctor = scope.root_nanbox_f64(crate::object::bound_native_callable_export_value(
"stream", "Stream",
));
let cid = synthetic_class_id_for_function(ctor.get_nanbox_f64());
let instance = scope.root_raw_mut_ptr(js_object_alloc(cid, 0));
let proto = crate::closure::closure_get_dynamic_prop(
(ctor.get_nanbox_u64() & crate::value::POINTER_MASK) as usize,
"prototype",
);
if crate::value::JSValue::from_bits(proto.to_bits()).is_pointer() {
instance.with_mut_ptr::<ObjectHeader, _>(|obj| {
super::super::prototype_chain::object_link_class_default_prototype(
obj as usize,
proto.to_bits(),
)
});
}
return Some(
instance
.with_mut_ptr::<ObjectHeader, _>(|obj| crate::value::js_nanbox_pointer(obj as i64)),
);
}
None
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,12 @@ pub extern "C" fn js_register_class_parent_dynamic(class_id: u32, mut parent_val
register_class(class_id, parent);
}
}
if module == "events" {
if module == "events" || (module == "stream" && method == "Stream") {
// #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" => 0xFFFF0076,
"EventEmitter" | "Stream" => 0xFFFF0076,
"EventEmitterAsyncResource" => 0xFFFF0077,
_ => 0,
};
Expand Down
9 changes: 7 additions & 2 deletions crates/perry-runtime/src/object/native_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,10 @@ unsafe fn nm_ee_dynamic_super(
args_len: usize,
) -> Option<f64> {
let (module, method) = bound_native_callable_module_and_method(func_value)?;
if module.trim_start_matches("node:") == "events"
&& (method == "EventEmitter" || method == "EventEmitterAsyncResource")
let module = module.trim_start_matches("node:");
// #10430: legacy `Stream` is `function Stream(opts) { EE.call(this, opts) }`.
if (module == "events" && (method == "EventEmitter" || method == "EventEmitterAsyncResource"))
|| (module == "stream" && method == "Stream")
{
let this_val = super::js_implicit_this_get();
if crate::value::JSValue::from_bits(this_val.to_bits()).is_pointer() {
Expand Down Expand Up @@ -691,6 +693,9 @@ pub(crate) fn cjs_default_export_value(module_name: &str) -> Option<f64> {
match module_name {
"assert" | "assert/strict" => Some(callable_exports::assert_cjs_export_value(module_name)),
"events" => Some(bound_native_callable_export_value("events", "EventEmitter")),
// #10431: `stream`'s `module.exports` IS the legacy `Stream` constructor
// (exports hang off it: `attach_stream_legacy_prototype`).
"stream" => Some(bound_native_callable_export_value("stream", "Stream")),
// #3687: `node:cluster` default import is a distinct EventEmitter-shaped
// `cluster.default` namespace (its `on`/`emit`/… reads diverge from the
// bare `import * as` namespace).
Expand Down
Loading
Loading