diff --git a/changelog.d/10673-namespace-export-equals-fallback.md b/changelog.d/10673-namespace-export-equals-fallback.md new file mode 100644 index 0000000000..ae9f869667 --- /dev/null +++ b/changelog.d/10673-namespace-export-equals-fallback.md @@ -0,0 +1,17 @@ +Fixed `axios` (and any `perry.compilePackages` target with a similar shape) +throwing `TypeError: Class extends value is not a constructor` at +module-init time. The blocker was in the `https-proxy-agent` -> `agent-base` +dependency chain: `agent-base`'s TypeScript source merges `namespace +createAgent { export class Agent extends EventEmitter { ... } }` onto a +same-named `function createAgent()` and exports it with `export =` — +Perry's HIR doesn't correctly attach the namespace's exported members to +the same runtime value `export =` ends up exporting, so +`require("agent-base").Agent` read back as `undefined` and the downstream +`class HttpsProxyAgent extends agent_base_1.Agent` threw. Perry's +`compilePackages` module resolution now detects this TS +namespace/function-merge + `export =` shape and falls back to the +package's compiled JS emit instead of its raw `.ts` source — the same file +Node itself runs, since `--experimental-strip-types` can't execute raw +`namespace`/`export =` syntax either. Extends the existing #6586 +ESM+CJS-epilogue fallback in `is_hybrid_cjs_emit_input` with a second, +narrowly-scoped trigger; not keyed on the `agent-base` package name. diff --git a/crates/perry/src/commands/compile/cjs_wrap/detect.rs b/crates/perry/src/commands/compile/cjs_wrap/detect.rs index 76b979e912..6faa700197 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/detect.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/detect.rs @@ -458,6 +458,58 @@ pub(in crate::commands::compile) fn has_top_level_module_exports_assignment(sour false } +/// Returns true if `source` (expected to already be comment/string-stripped +/// via [`strip_comments_and_strings`]) contains a top-level TypeScript +/// `namespace X { … }` / legacy `module X { … }` declaration with a REAL +/// (non-ambient) body — i.e. NOT `declare namespace X { … }`, which is +/// type-only and never emits runtime code, so it can't be the cause of a +/// namespace/function declaration-merge going missing at runtime. +/// +/// Line-anchored rather than depth-tracked, unlike [`has_top_level_esm`]: a +/// `namespace`/`module` block is hand-authored (or `tsc`-emitted) TypeScript +/// source, never a minified bundle, so it is always written starting its own +/// line. Requiring the `{` to follow the (possibly dotted) namespace name on +/// the SAME statement, with only whitespace/dots in between, keeps this from +/// matching ordinary CommonJS `module.exports = { … }` — there `module` is +/// followed immediately by `.`, never by whitespace then an identifier. +/// +/// Used together with [`has_top_level_export_equals`] (#10662): a package +/// like `agent-base` merges `namespace createAgent { export class Agent +/// extends EventEmitter { … } }` onto a same-named `function createAgent()` +/// and exports the merged value via `export = createAgent`. Perry's HIR +/// lowers the namespace's exported members as static-field-set init +/// statements against a synthetic class entity that does not end up being +/// the SAME runtime object `export =` exports — so a downstream `class X +/// extends pkg.Agent` sees `pkg.Agent` as `undefined` and throws "Class +/// extends value is not a constructor" (axios's `https-proxy-agent` → +/// `agent-base` dependency chain). Node can't run this non-erasable TS +/// syntax directly either (`--experimental-strip-types` rejects `namespace`/ +/// `export =`), so a package built this way is NEVER executed from its raw +/// `.ts` source in practice — only via its compiled emit. Detecting the +/// shape and falling back to that emit (see `is_hybrid_cjs_emit_input` in +/// `resolve.rs`) matches what Node actually runs, instead of attempting to +/// correctly implement namespace/function declaration merging. +pub(in crate::commands::compile) fn has_top_level_namespace_or_module_block(source: &str) -> bool { + let re = perry_perex::tooling::Regex::new( + r"(?m)^[ \t]*(declare\s+)?(?:export\s+)?(?:namespace|module)\s+[A-Za-z_$][\w$]*(?:\s*\.\s*[A-Za-z_$][\w$]*)*\s*\{", + ) + .expect("valid namespace/module regex"); + re.captures_iter(source).any(|cap| cap.get(1).is_none()) +} + +/// Returns true if `source` (comment/string-stripped) contains a top-level +/// TypeScript `export = ;` statement — the CJS-interop export form a +/// namespace-merged package like `agent-base` uses instead of `module.exports +/// = …` (see [`has_top_level_namespace_or_module_block`], #10662). The +/// trailing character class excludes `export ==`/`export =>`; neither is +/// valid syntax here, but the exclusion costs nothing and avoids relying on +/// lookahead, which the `regex` crate doesn't support. +pub(in crate::commands::compile) fn has_top_level_export_equals(source: &str) -> bool { + let re = perry_perex::tooling::Regex::new(r"(?m)^[ \t]*export\s*=[\s\w$(\[{]") + .expect("valid export= regex"); + re.is_match(source) +} + /// Returns true if `line` starts with `keyword` followed by a character /// that can legally begin an `import`/`export` statement's continuation: /// space, `{`, `*` (export only), `"`, `'`, or `(` (dynamic import). We diff --git a/crates/perry/src/commands/compile/cjs_wrap/issue_10662_tests.rs b/crates/perry/src/commands/compile/cjs_wrap/issue_10662_tests.rs new file mode 100644 index 0000000000..aed104f1b0 --- /dev/null +++ b/crates/perry/src/commands/compile/cjs_wrap/issue_10662_tests.rs @@ -0,0 +1,101 @@ +//! Regression tests for #10662: `agent-base`'s TypeScript `namespace +//! createAgent { export class Agent extends EventEmitter { … } } export = +//! createAgent;` — a `function createAgent()` merged with a namespace of the +//! same name, exported via TS's `export =` form. Perry's HIR lowers the +//! namespace's exported `Agent` class as a static-field-set against a +//! synthetic class entity distinct from the runtime function value +//! `export =` actually exports, so a downstream `https-proxy-agent extends +//! agent_base_1.Agent` (an `axios` transitive dependency) sees `.Agent` as +//! `undefined` and throws "Class extends value is not a constructor". +//! +//! `has_top_level_namespace_or_module_block` / +//! `has_top_level_export_equals` detect this shape so +//! `is_hybrid_cjs_emit_input` (`resolve.rs`) can fall back to the package's +//! compiled JS emit — the same emit Node itself runs, since +//! `--experimental-strip-types` can't execute raw `namespace`/`export =` +//! syntax either. + +use super::detect::{ + has_top_level_export_equals, has_top_level_namespace_or_module_block, + strip_comments_and_strings, +}; + +#[test] +fn namespace_block_detects_the_agent_base_shape() { + let src = strip_comments_and_strings( + "function createAgent(opts) {\n return new createAgent.Agent(opts);\n}\n\nnamespace createAgent {\n export class Agent extends EventEmitter {}\n}\n\nexport = createAgent;\n", + ); + assert!(has_top_level_namespace_or_module_block(&src)); + assert!(has_top_level_export_equals(&src)); +} + +#[test] +fn namespace_block_accepts_legacy_module_keyword_and_dotted_names() { + let src = strip_comments_and_strings("module Foo.Bar {\n export const x = 1;\n}\n"); + assert!(has_top_level_namespace_or_module_block(&src)); +} + +#[test] +fn ambient_declare_namespace_is_not_flagged() { + // `declare namespace X { … }` is type-only — it never emits runtime + // code, so it cannot be the cause of a namespace/function merge going + // missing at runtime, and must not trigger the JS-emit fallback. + let src = strip_comments_and_strings( + "declare namespace createAgent {\n export class Agent {}\n}\nexport = createAgent;\n", + ); + assert!(!has_top_level_namespace_or_module_block(&src)); +} + +#[test] +fn ordinary_cjs_module_exports_object_literal_is_not_flagged() { + // `module.exports = { … }` is the single most common CommonJS shape — + // `module` is followed by `.`, never by whitespace then an identifier, + // so it must never be mistaken for a `namespace`/`module X {` block. + let src = strip_comments_and_strings( + "function build() { return 1; }\nmodule.exports = { build: build, value: 42 };\n", + ); + assert!(!has_top_level_namespace_or_module_block(&src)); +} + +#[test] +fn export_equals_matches_the_export_equals_form_only() { + assert!(has_top_level_export_equals(&strip_comments_and_strings( + "export = createAgent;\n" + ))); + assert!(has_top_level_export_equals(&strip_comments_and_strings( + "export=createAgent;\n" + ))); + + // Ordinary ESM export forms must not match — none of these are the + // CJS-interop `export =` shape. + assert!(!has_top_level_export_equals(&strip_comments_and_strings( + "export const x = 1;\n" + ))); + assert!(!has_top_level_export_equals(&strip_comments_and_strings( + "export class Foo {}\n" + ))); + assert!(!has_top_level_export_equals(&strip_comments_and_strings( + "export default Foo;\n" + ))); + assert!(!has_top_level_export_equals(&strip_comments_and_strings( + "export { Foo };\n" + ))); +} + +#[test] +fn plain_esm_or_cjs_source_without_the_merge_shape_is_unaffected() { + // A normal ESM file with a class extending an imported native builtin — + // the overwhelmingly common case — must not be flagged: no namespace + // block, no `export =`. + let esm = strip_comments_and_strings( + "import { EventEmitter } from 'events';\nexport class Agent extends EventEmitter {}\n", + ); + assert!(!has_top_level_namespace_or_module_block(&esm)); + assert!(!has_top_level_export_equals(&esm)); + + // A normal CJS file. + let cjs = + strip_comments_and_strings("'use strict';\nclass Agent {}\nmodule.exports = { Agent };\n"); + assert!(!has_top_level_namespace_or_module_block(&cjs)); + assert!(!has_top_level_export_equals(&cjs)); +} diff --git a/crates/perry/src/commands/compile/cjs_wrap/mod.rs b/crates/perry/src/commands/compile/cjs_wrap/mod.rs index 815617bb51..83079a1345 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/mod.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/mod.rs @@ -43,6 +43,8 @@ mod extract_requires; mod hoist_classes; mod wrap; +#[cfg(test)] +mod issue_10662_tests; #[cfg(test)] mod issue_6585_tests; #[cfg(test)] diff --git a/crates/perry/src/commands/compile/resolve.rs b/crates/perry/src/commands/compile/resolve.rs index b77abcb267..54d2d205c0 100644 --- a/crates/perry/src/commands/compile/resolve.rs +++ b/crates/perry/src/commands/compile/resolve.rs @@ -767,13 +767,29 @@ fn original_source_via_map(entry: &Path) -> Option { original_source_from_map_file(&append_map_extension(entry)) } -/// A published CommonJS package can ship the TypeScript input to its CJS emit. -/// Some such inputs are intentionally hybrid: normal ESM declarations for -/// TypeScript plus a top-level `module.exports = ...` interop epilogue. The -/// source is not a directly executable module in Perry: ESM classification -/// leaves `module` unbound, while CJS wrapping would move its `export` -/// declarations inside an IIFE. Node loads the emitted JS entry, so keep that -/// entry instead of following its source map for this narrow shape (#6586). +/// A published CommonJS package can ship a TypeScript input that is not +/// directly executable as a Perry module, in which case Perry should keep +/// the package on its compiled JS emit instead of the raw source (matching +/// what Node actually runs) rather than following a source map / `src/` +/// convention to that source. Two known shapes trigger this, both narrow and +/// evidence-driven rather than a general "prefer JS" default: +/// +/// - **ESM-plus-CJS-epilogue hybrid** (#6586): normal ESM declarations for +/// TypeScript plus a top-level `module.exports = ...` interop epilogue. +/// ESM classification leaves `module` unbound, while CJS wrapping would +/// move its `export` declarations inside an IIFE — neither executes. +/// - **Namespace/function declaration merging via `export =`** (#10662): +/// `namespace X { export class Y extends Z {} }` merged onto a same-named +/// `function X() {}` and exported with `export = X` — the shape +/// `agent-base` (an `axios` → `https-proxy-agent` transitive dependency) +/// uses. Perry's HIR lowers the namespace's exported members as static +/// fields against a synthetic class entity that is not the SAME runtime +/// value `export =` ends up exporting, so e.g. `pkg.Agent` reads back as +/// `undefined` and a downstream `class X extends pkg.Agent` throws "Class +/// extends value is not a constructor". Node can't run this non-erasable +/// TS syntax directly either (`--experimental-strip-types` rejects +/// `namespace`/`export =`), so such a package is never executed from its +/// raw `.ts` source in practice — only via its compiled emit. fn is_hybrid_cjs_emit_input(path: &Path) -> bool { static CACHE: OnceLock>> = OnceLock::new(); @@ -789,8 +805,10 @@ fn is_hybrid_cjs_emit_input(path: &Path) -> bool { return false; }; let stripped = super::cjs_wrap::detect::strip_comments_and_strings(&source); - let hybrid = super::cjs_wrap::detect::has_top_level_esm(&stripped) - && super::cjs_wrap::detect::has_top_level_module_exports_assignment(&stripped); + let hybrid = (super::cjs_wrap::detect::has_top_level_esm(&stripped) + && super::cjs_wrap::detect::has_top_level_module_exports_assignment(&stripped)) + || (super::cjs_wrap::detect::has_top_level_namespace_or_module_block(&stripped) + && super::cjs_wrap::detect::has_top_level_export_equals(&stripped)); cache .lock() .expect("hybrid source cache") diff --git a/crates/perry/tests/issue_10662_namespace_export_equals_fallback.rs b/crates/perry/tests/issue_10662_namespace_export_equals_fallback.rs new file mode 100644 index 0000000000..72336c2540 --- /dev/null +++ b/crates/perry/tests/issue_10662_namespace_export_equals_fallback.rs @@ -0,0 +1,187 @@ +//! Regression test for #10662: `axios` throws `TypeError: Class extends +//! value is not a constructor` at module-init time because its transitive +//! dependency chain `https-proxy-agent` -> `agent-base` hits a TypeScript +//! declaration-merging shape Perry's HIR does not lower correctly. +//! +//! `agent-base`'s real source (`src/index.ts`) is: +//! +//! ```ts +//! function createAgent(opts) { return new createAgent.Agent(opts); } +//! namespace createAgent { +//! export class Agent extends EventEmitter { ... } +//! } +//! export = createAgent; +//! ``` +//! +//! `perry.compilePackages` prefers compiling a package's raw TypeScript +//! source over its published JS emit (`resolve_package_source_entry`), and +//! picks `src/index.ts` here since `agent-base` ships both. Perry's HIR +//! lowers the namespace's exported `Agent` class as a `StaticFieldSet` +//! against a synthetic class entity that is NOT the same runtime object +//! `export =` ends up exporting: `require("agent-base").Agent` reads back +//! as `undefined`, and `https-proxy-agent`'s `class HttpsProxyAgent extends +//! agent_base_1.Agent` throws. +//! +//! The fix (`is_hybrid_cjs_emit_input` in `resolve.rs`, alongside its +//! existing #6586 ESM+CJS-epilogue trigger) detects the namespace-block + +//! `export =` shape and falls back to the package's compiled JS emit +//! instead — the same file Node itself runs (raw `namespace`/`export =` +//! isn't valid under `--experimental-strip-types` either, so a package +//! built this way is never executed from its `.ts` source in practice). +//! +//! This fixture mirrors the real shape exactly enough to reproduce the bug +//! (namespace-merged-with-function class extending a native `EventEmitter`, +//! consumed by a downstream CJS `class X extends pkg.Agent`) without +//! depending on the actual npm packages. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +#[test] +fn namespace_merged_function_export_equals_falls_back_to_js_emit() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + + std::fs::write( + root.join("package.json"), + r#"{ + "name": "issue-10662-consumer", + "private": true, + "perry": { + "compilePackages": ["agent-base-like"], + "allow": { "compilePackages": ["agent-base-like"] } + } +}"#, + ) + .expect("write consumer package.json"); + + // `agent-base`'s exact shape: a package.json "main" pointing at the + // compiled JS, PLUS a `src/index.ts` Perry would otherwise prefer. + let pkg = root.join("node_modules").join("agent-base-like"); + std::fs::create_dir_all(pkg.join("src")).expect("mkdir src"); + std::fs::create_dir_all(pkg.join("dist").join("src")).expect("mkdir dist/src"); + std::fs::write( + pkg.join("package.json"), + r#"{ "name": "agent-base-like", "version": "1.0.0", "main": "dist/src/index", "typings": "dist/src/index" }"#, + ) + .expect("write agent-base-like package.json"); + + // The raw TS source: `namespace createAgent { export class Agent + // extends EventEmitter { ... } }` merged onto `function createAgent()`, + // exported via `export =`. Perry cannot correctly lower this shape + // today (#10662) — the JS-emit fallback is what makes it work. + std::fs::write( + pkg.join("src").join("index.ts"), + r#"import { EventEmitter } from 'events'; + +function createAgent(opts?: any) { + return new createAgent.Agent(opts); +} + +namespace createAgent { + export class Agent extends EventEmitter { + public tag: string; + constructor(opts?: any) { + super(); + this.tag = "agent-tag"; + } + } +} + +export = createAgent; +"#, + ) + .expect("write agent-base-like src/index.ts"); + + // The compiled emit `tsc` would actually publish — plain CJS, no + // namespace-merge complexity, `require()`d by Node in practice. + std::fs::write( + pkg.join("dist").join("src").join("index.js"), + r#""use strict"; +const events_1 = require("events"); +function createAgent(opts) { + return new createAgent.Agent(opts); +} +(function (createAgent) { + class Agent extends events_1.EventEmitter { + constructor(opts) { + super(); + this.tag = "agent-tag"; + } + } + createAgent.Agent = Agent; +})(createAgent || (createAgent = {})); +module.exports = createAgent; +"#, + ) + .expect("write agent-base-like dist/src/index.js"); + + // The `https-proxy-agent` half: a downstream CJS file (already + // "compiled" — no namespace complexity of its own) whose class extends + // the namespace-merged package's exported member. + let entry = root.join("main.ts"); + std::fs::write( + &entry, + r#"import "./downstream.cjs"; +"#, + ) + .expect("write entry"); + std::fs::write( + root.join("downstream.cjs"), + r#"'use strict'; +const pkg = require("agent-base-like"); +if (typeof pkg !== "function") { + throw new Error("expected agent-base-like's export = value to be callable, got " + typeof pkg); +} +if (typeof pkg.Agent !== "function") { + throw new Error("expected pkg.Agent to be a constructor, got " + typeof pkg.Agent); +} +class Downstream extends pkg.Agent { + constructor() { + super(); + this.extra = "downstream"; + } +} +const d = new Downstream(); +let seen = 0; +d.on("x", () => { seen++; }); +d.emit("x"); +console.log("tag:", d.tag, "extra:", d.extra, "events:", seen); +"#, + ) + .expect("write downstream.cjs"); + + let output = root.join("main_bin"); + let compile = Command::new(perry_bin()) + .current_dir(root) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed (namespace-merge JS-emit fallback regressed?)\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled binary"); + let stdout = String::from_utf8_lossy(&run.stdout); + assert!( + run.status.success(), + "compiled binary failed (agent-base-like's namespace-merged Agent should have resolved via the dist/ fallback)\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + stdout, + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + stdout, "tag: agent-tag extra: downstream events: 1\n", + "downstream class extending a namespace-merged native-base subclass must construct and behave correctly" + ); +}