diff --git a/changelog.d/9689-aliased-native-class-heritage.md b/changelog.d/9689-aliased-native-class-heritage.md new file mode 100644 index 0000000000..1fe49bd370 --- /dev/null +++ b/changelog.d/9689-aliased-native-class-heritage.md @@ -0,0 +1,6 @@ +**Native stream subclasses keep their derived methods when constructor imports +are aliased or minified.** Class heritage now resolves native import bindings to +their original exports before selecting the subclass initializer. This keeps +readdirp's `_read` implementation and the complete EventEmitter method surface +intact, allowing Claude Code's chokidar watcher to initialize without the +`once is not a function` startup errors reported in #9680. diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 7c755d7702..2561f9fedd 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -1884,6 +1884,50 @@ fn bun_transpiler_and_build_lower_to_native_dispatch() { ); } +/// #9680: native constructors retain their exported identity when a bundler +/// minifies the local import name used by a class declaration or expression. +#[test] +fn aliased_native_imports_canonicalize_class_heritage() { + let source = r#" + import { Readable as ut } from "node:stream"; + import { EventEmitter as jt } from "node:events"; + + class ReaddirpStream extends ut { + _read() {} + } + const Watcher = class extends jt {}; + "#; + let module = perry_parser::parse_typescript(source, "watcher.js").expect("source parses"); + let hir = super::lower_module(&module, "watcher", "watcher.js").expect("source lowers"); + + let stream = hir + .classes + .iter() + .find(|class| class.name == "ReaddirpStream") + .expect("stream class is lowered"); + assert_eq!(stream.extends_name.as_deref(), Some("Readable")); + assert_eq!( + stream.native_extends, + Some(("node_stream".to_string(), "Readable".to_string())) + ); + assert!( + stream.extends_expr.is_none(), + "aliased native heritage must not use replacement-object construction" + ); + + let watcher = hir + .classes + .iter() + .find(|class| class.name == "Watcher") + .expect("inferred-name class expression is lowered"); + assert_eq!(watcher.extends_name.as_deref(), Some("EventEmitter")); + assert_eq!( + watcher.native_extends, + Some(("events".to_string(), "EventEmitter".to_string())) + ); + assert!(watcher.extends_expr.is_none()); +} + /// #8882: a module-level class constructing a sibling class that is declared /// inside a function body lowered LATER. This is the shape the CJS wrap /// produces for Next's `server/lib/lru-cache.js`: `LRUCache` is hoisted out of diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index d7d0080aa8..e824e5ee00 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -7,37 +7,35 @@ use crate::lower::{lower_expr, LoweringContext}; use crate::lower_patterns::*; use crate::lower_types::*; -/// The four classic node:stream base-class names (`Readable`/`Writable`/ -/// `Duplex`/`Transform`). When a class extends a parent with one of these -/// textual names, perry routes `super()` to the native `js_node_stream_*` -/// shim (which installs the native stream surface but never sets the -/// `_readableState`/`_writableState`/`_transformState` objects). That is only -/// correct when the name actually resolves to the `node:stream` builtin — -/// i.e. it was imported from `stream`/`node:stream`, in which case the import -/// machinery registered it as the `stream` native module -/// (`register_native_module(binding, "stream", Some(name))`, see -/// `var_decl_sources::register_destructured_stream_ctors` and the import -/// arms in `lower/module_decl.rs`). -/// -/// The same textual name can instead be a userland binding from a -/// stream-shim npm package — `const { Transform } = -/// require('readable-stream')` in winston's `logger.js`, where -/// `class Logger extends Transform` then reads `this._readableState.pipes` -/// directly. Such a binding never registers as the `stream` native module -/// (readable-stream is not a node builtin), so the package's real constructor -/// must run instead. Returning `false` here lets the unknown-Ident arm -/// capture the parent as `extends_expr` and route `super()` through the -/// dynamic-parent path (`js_register_class_parent_dynamic` + the -/// `js_fetch_or_value_super` dispatch), which runs the real `Transform` -/// function body on the subclass instance. +/// Recover the exported constructor name behind a minified native import used +/// as class heritage (`import { Readable as ut }; class R extends ut`). Native +/// 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(("events", Some(class @ ("EventEmitter" | "EventEmitterAsyncResource")))) + | Some(("async_hooks", Some(class @ ("AsyncLocalStorage" | "AsyncResource")))) + | Some(("ws", Some(class @ "WebSocketServer"))) + | Some(( + "stream/web" | "node:stream/web", + Some(class @ ("ReadableStream" | "WritableStream" | "TransformStream")), + )) => Some(class), + _ => None, + } +} + +/// Only genuine `node:stream` bindings use Perry's native subclass shims. A +/// userland binding from `readable-stream` must keep the dynamic parent path so +/// its real constructor runs. Inspecting the preserved export also supports a +/// minified local binding such as `Readable as ut`. fn is_genuine_node_stream_parent(ctx: &LoweringContext, name: &str) -> bool { - if !matches!(name, "Readable" | "Writable" | "Duplex" | "Transform") { - return false; + match ctx.lookup_native_module(name) { + Some(("stream", Some("Readable" | "Writable" | "Duplex" | "Transform"))) => 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"), + _ => false, } - // Only the genuine `node:stream` builtin import registers these names as - // the `stream` native module. Anything else (a userland require/import of - // a stream-shim package, or an unbound reference) is not the builtin. - matches!(ctx.lookup_native_module(name), Some(("stream", _))) } mod class_heritage; @@ -331,8 +329,11 @@ pub fn lower_class_decl( ) } else if let ast::Expr::Ident(ident) = super_class.as_ref() { let parent_name = ident.sym.to_string(); + let canonical_parent_name = canonical_native_parent_name(ctx, &parent_name) + .unwrap_or(&parent_name) + .to_string(); // First check if it's a native module class - let native_parent = match parent_name.as_str() { + let native_parent = match canonical_parent_name.as_str() { "EventEmitter" => Some(("events".to_string(), "EventEmitter".to_string())), "EventEmitterAsyncResource" => Some(( "events".to_string(), @@ -379,7 +380,7 @@ pub fn lower_class_decl( "Readable" | "Writable" | "Duplex" | "Transform" if is_genuine_node_stream_parent(ctx, &parent_name) => { - Some(("node_stream".to_string(), parent_name.clone())) + Some(("node_stream".to_string(), canonical_parent_name.clone())) } _ => None, }; @@ -399,7 +400,7 @@ pub fn lower_class_decl( // dispatch resolves through the existing extends_name // path while the native_extends carries the (module, // class) tag for the runtime shim). - (None, Some(parent_name), native_parent, None) + (None, Some(canonical_parent_name), native_parent, None) } else if locally_shadowed { // Lexical local shadow → dynamic parent via `extends_expr` (the // in-scope local value), invoked by `super()` through @@ -1385,7 +1386,10 @@ pub fn lower_class_from_ast( ) } else if let ast::Expr::Ident(ident) = super_class.as_ref() { let parent_name = ident.sym.to_string(); - let native_parent = match parent_name.as_str() { + let canonical_parent_name = canonical_native_parent_name(ctx, &parent_name) + .unwrap_or(&parent_name) + .to_string(); + let native_parent = match canonical_parent_name.as_str() { "EventEmitter" => Some(("events".to_string(), "EventEmitter".to_string())), "EventEmitterAsyncResource" => Some(( "events".to_string(), @@ -1416,7 +1420,7 @@ pub fn lower_class_from_ast( "Readable" | "Writable" | "Duplex" | "Transform" if is_genuine_node_stream_parent(ctx, &parent_name) => { - Some(("node_stream".to_string(), parent_name.clone())) + Some(("node_stream".to_string(), canonical_parent_name.clone())) } _ => None, }; @@ -1430,7 +1434,7 @@ pub fn lower_class_from_ast( let locally_shadowed = !ctx.class_renames.contains_key(&parent_name) && ctx.locals.lookup(&parent_name).is_some(); if native_parent.is_some() && !locally_shadowed { - (None, Some(parent_name), native_parent, None) + (None, Some(canonical_parent_name), native_parent, None) } else if locally_shadowed { // #5437 (Next.js p-queue `PQueue` inside a minified bundle): a // class EXPRESSION whose parent Ident is an IN-SCOPE LOCAL diff --git a/test-files/test_issue_9680_readdirp_stream_emitter_surface.ts b/test-files/test_issue_9680_readdirp_stream_emitter_surface.ts new file mode 100644 index 0000000000..585f41ffdc --- /dev/null +++ b/test-files/test_issue_9680_readdirp_stream_emitter_surface.ts @@ -0,0 +1,79 @@ +// Regression for #9680: the bundled readdirp source aliases its node:stream +// import, subclasses it, and hands the instance to chokidar. Chokidar chains +// on(...).on(...).once(...), so the stream must retain the complete inherited +// EventEmitter surface even though the native base's local name was minified. + +import { EventEmitter as jt } from "node:events"; +import { Readable as ut } from "node:stream"; + +class ReaddirpStyleStream extends ut { + readCalls = 0; + + constructor() { + super({ objectMode: true, autoDestroy: true, highWaterMark: 4096 }); + } + + _read(): void { + this.readCalls++; + } +} + +const Watcher = class extends jt { + marker(): string { + return "watcher-subclass"; + } +}; + +function readdirpStyle(): any { + return new ReaddirpStyleStream(); +} + +const stream = readdirpStyle(); +const emitterMethods = [ + "on", + "once", + "off", + "removeListener", + "emit", + "prependListener", + "prependOnceListener", + "listenerCount", + "eventNames", +]; + +const expectedNames = new Set(emitterMethods); +const surfaceNames = new Set(); +let prototypeCursor: any = stream; +while (prototypeCursor !== null) { + for (const name of Object.getOwnPropertyNames(prototypeCursor)) { + if (expectedNames.has(name)) surfaceNames.add(name); + } + prototypeCursor = Object.getPrototypeOf(prototypeCursor); +} + +stream._read(); +console.log("read-calls:" + stream.readCalls); +console.log("surface:" + Array.from(surfaceNames).sort().join(",")); +console.log(emitterMethods.map((name) => `${name}:${typeof stream[name]}`).join(",")); + +const calls: string[] = []; +const watcher: any = new Watcher(); +watcher.once("ready", (): void => calls.push("watcher-ready")); +watcher.emit("ready"); +console.log("watcher:" + watcher.marker()); + +const persistent = (value: string): void => calls.push(`on:${value}`); +stream + .on("entry", persistent) + .on("unused", (): void => calls.push("unused")) + .once("entry", (value: string): void => calls.push(`once:${value}`)); +stream.prependListener("entry", (value: string): void => calls.push(`prepend:${value}`)); +stream.prependOnceListener("entry", (value: string): void => calls.push(`prepend-once:${value}`)); + +console.log("events-before:" + stream.eventNames().join(",")); +console.log("listeners-before:" + stream.listenerCount("entry")); +console.log("emit-1:" + stream.emit("entry", "a")); +console.log("emit-2:" + stream.emit("entry", "b")); +stream.off("entry", persistent); +console.log("listeners-after:" + stream.listenerCount("entry")); +console.log("calls:" + calls.join(","));