fix(stream): #3663 — wire on/write/pipe + data pump on all stream construction paths - #4076
Merged
Conversation
added 5 commits
June 2, 2026 09:14
new Readable()/Writable()/Duplex()/Transform()/PassThrough() only produced a
fully-methoded object on the ESM named-import path (lowered to Expr::New ->
lower_builtin_new). The CJS-destructure form (const { Readable } =
require('stream')) and namespace-member form (new stream.Readable()) lower to
NewDynamic, which fell through to an empty-object placeholder, so .on()/.write()/
.pipe() threw 'is not a function' — the bulk of issue #3663's 65 parity failures.
Two routing fixes:
- codegen new_dynamic.rs: NewDynamic { callee: PropertyGet { NativeModuleRef(
"stream"), Ctor } } routes to lower_builtin_new (import * as stream).
- runtime js_new_function_construct: when the callee resolves to the stream.<Ctor>
bound-method closure, dispatch to js_node_stream_*_new. Covers every aliasing
shape (require destructure, require-into-namespace, const alias).
…stances
The classic-stream method routing (.pipe/.on/.write -> NativeMethodCall real
pump) fires during lowering only when the receiver local is tagged as a
('stream', Ctor) native instance. That tagging keys off lookup_native_module,
which the ESM named-import path populates but CJS require-destructure does not —
so const { Readable } = require('stream') left Readable a plain local, the
instance was never tagged, and methods dispatched to no-op stub closures.
Register destructured stream constructors (and the new stream.Ctor() member
form) as native-module aliases so the existing instance-registration +
method-routing machinery treats them identically to import { Readable }.
…e tagging
is_require_builtin_module's allowlist is only fs/path/crypto, so it didn't
recognize require('stream'); add require_literal_specifier which returns the
specifier verbatim and let the caller match 'stream'. This makes the direct
const { Readable } = require('stream') form (what Node's own test files use)
tag the stream instance so .pipe/.on/.write route to the real pump.
…e construction
Exercises the NewDynamic -> stream-constructor path (import * as stream;
new stream.Readable(...)) end-to-end: methods present AND data pumps + events
fire, byte-identical to node --experimental-strip-types. (CJS require('stream')
forms can't be node-compared under the ESM-only strip-types harness, but are
verified manually and share the same runtime/lowering path.)
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 #3663 — classic
node:streamobjects (Readable/Writable/Duplex/Transform/PassThrough) missingon/write/pipeon some construction paths.The methods only worked on the direct ESM named-import path (
import { Readable } from "stream"; new Readable(...)). Every other construction form failed:import { Readable } from "stream"const { Readable } = require("stream")TypeError: on is not a functionimport * as s; new s.Readable()TypeErrorconst s = require("stream"); const { Readable } = sTypeErrorThe reported 65 parity failures concentrate here because Node's own stream test files use
const { Readable } = require("stream").Root cause
Two independent gaps, both keyed off how the binding is introduced:
Construction. A named ESM import lowers
new Readable()toExpr::New { class_name: "Readable" }→ the existinglower_builtin_newstream handler builds a fully-methoded object. CJS-destructured / namespace-member bindings instead lower toExpr::NewDynamic, which fell through to an empty-object placeholder — no EventEmitter/Writable methods.Data pump. The classic-stream method routing (
.pipe/.on/.write→ realNativeMethodCallpump) fires during lowering only when the receiver local is tagged as a("stream", Ctor)native instance. That tagging keys offlookup_native_module, which ESM named imports populate but CJS require-destructure does not — so even once methods existed they were no-op stubs.Fix (4 files)
runtime/object/class_registry.rs—js_new_function_constructalready resolves a callee value to(module, method)and dispatchestty/fs/tlsstream-likes; add astreamarm so any aliasing shape that reaches it builds the real object viajs_node_stream_*_new.codegen/expr/new_dynamic.rs— routeNewDynamic { callee: PropertyGet { NativeModuleRef("stream"), Ctor } }(theimport * as streammember form) straight tolower_builtin_new.hir/destructuring/var_decl.rs— register destructured / namespaced stream constructors as native-module member aliases (mirroringimport { Readable }), so the existing instance-tagging + method-routing machinery treats them identically.is_require_builtin_module's allowlist is onlyfs/path/crypto, so a dedicatedrequire_literal_specifierrecognizesrequire("stream").test-parity/node-suite/stream/construct-paths/namespace-member-new.ts— parity fixture for thenew stream.Readable()construct-and-pump path.Verification
node --experimental-strip-typesfor a full pipe/push/'end'/'finish'pump, plusReadable.from,r.constructor.name, and Transform/Duplex/PassThrough method presence.pipe/write/push/flow/events/duplex/transform): 172/175 match Node. The 3 mismatches are all ESM-path fixtures untouched by this change (pre-existing fidelity gaps: repipe double-pump, default-_transformthrow, sync-throw stack trace).buffer/byte-lengthgaps unrelated to this change.module == "stream"+ the five constructor names, so non-stream destructures are unaffected.PERRY_NO_AUTO_OPTIMIZE).cargo fmt --all --checkclean;perry-hirtests pass.Known follow-ups (out of scope, pre-existing)
Data-pump fidelity gaps that also affect the ESM path: repipe-after-unpipe double-pump,
new Transform()with no_transformshould throwERR_METHOD_NOT_IMPLEMENTED, and sync-throw stack-trace depth.