From f315ad65076f1a52b23bd4619d76972553db37ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 10 Jun 2026 09:42:03 +0200 Subject: [PATCH] fix(compile): resolve barrel/type-only default-import link wall (nestjs) (#4872) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four coordinated fixes for the nestjs-hello native-link wall: 1. compile.rs: a DEFAULT import of a compiled module that has no `default` export (ESM barrel with only named exports, or a type-only interface surface with no exports at all) now routes through the namespace-import machinery instead of registering a phantom callable. Member reads resolve per-export to origin symbols and whole-value reads materialize the namespace object — Node `require(esm)` semantics. Pre-fix the consumer emitted `perry_fn___default` / `__perry_wrap_perry_fn___default` references that no object file defines. 2. cjs_wrap: `__exportStar(require("./x"), exports)` — tsc's CJS lowering of `export * from "./x"` — now also emits the real ESM `export * from './x'` at module scope, so compile.rs's transitive re-export propagation resolves named imports through multi-level barrels to the defining module (`@nestjs/common` → `decorators` → `core` → `controller.decorator.js`). The specs covered by the star re-export are skipped by the recursive named-export pull so the static origin binding isn't shadowed by an `export const X = _cjs.X` runtime read. 3. cjs_wrap detect: modules whose only CJS marker is `Object.defineProperty(exports, "__esModule", { value: true });` (nestjs dist `*.interface.js`) are now detected as CommonJS. Pre-fix they compiled as zero-export ES modules and threw `ReferenceError: exports is not defined` at init. 4. compile.rs: `export *` propagation no longer re-exports `default` across hops (ESM spec), which keeps the has-default probe in (1) sound for barrels whose star sources have default exports. Drive-by: class_validation.rs counted TypeScript constructor overload SIGNATURES (body-less `constructor(...)` declarations) as real constructors, rejecting rxjs's `Notification` (3 signatures + 1 implementation) with "may only have one constructor". Only the implementation counts now. The nestjs-hello fixture now compiles and links end-to-end (`Wrote executable`, ~41 MB). It still SKIPs at runtime on the next wall — `.prototype` of a capturing class expression (`ClassExprFresh`) reads undefined, so tslib's `__decorate` throws during `@nestjs/common/services/logger.service.js` init. WALLS.md documents the minimal repro for that follow-up. Validation: new e2e test issue_4872_barrel_default_reexports.rs covers all three shapes; full `cargo test -p perry` green (incl. the #4841 namespace-CJS classifier test); hono-basic + node-http-serve release fixtures PASS; axios-get / ws-echo failures reproduce identically with the pre-change baseline binary (pre-existing). --- .../src/lower_decl/class_validation.rs | 12 +- crates/perry/src/commands/compile.rs | 43 ++++- .../src/commands/compile/cjs_wrap/detect.rs | 7 + .../compile/cjs_wrap/extract_requires.rs | 25 +++ .../src/commands/compile/cjs_wrap/mod.rs | 4 +- .../src/commands/compile/cjs_wrap/wrap.rs | 30 ++++ .../issue_4872_barrel_default_reexports.rs | 163 ++++++++++++++++++ tests/release/packages/nestjs-hello/WALLS.md | 65 +++++-- 8 files changed, 327 insertions(+), 22 deletions(-) create mode 100644 crates/perry/tests/issue_4872_barrel_default_reexports.rs diff --git a/crates/perry-hir/src/lower_decl/class_validation.rs b/crates/perry-hir/src/lower_decl/class_validation.rs index ba57d7ade3..bd82b6c108 100644 --- a/crates/perry-hir/src/lower_decl/class_validation.rs +++ b/crates/perry-hir/src/lower_decl/class_validation.rs @@ -86,8 +86,16 @@ pub fn validate_class_element_early_errors(class: &ast::Class, class_name: &str) for member in &class.body { match member { // `constructor(){}` (the ordinary one) parses as a dedicated - // Constructor node; count them to catch duplicates. - ast::ClassMember::Constructor(_) => constructor_count += 1, + // Constructor node; count them to catch duplicates. TypeScript + // overload SIGNATURES (`constructor(kind: 'N');` with no body) + // are type-only declarations, not constructors — only the + // implementation (the one with a body) counts (#4872, rxjs's + // `Notification` has 3 overload signatures + 1 implementation). + ast::ClassMember::Constructor(c) => { + if c.body.is_some() { + constructor_count += 1; + } + } ast::ClassMember::Method(m) => { let Some(name) = static_prop_name(&m.key) else { continue; diff --git a/crates/perry/src/commands/compile.rs b/crates/perry/src/commands/compile.rs index 52dc41e7e3..40871f099d 100644 --- a/crates/perry/src/commands/compile.rs +++ b/crates/perry/src/commands/compile.rs @@ -1139,6 +1139,17 @@ pub fn run_with_parse_cache( if let Some(source_exports) = all_module_exports.get(&source_path_str) { let current_exports = all_module_exports.get(&path_str); for (name, origin) in source_exports { + // ESM semantics: `export * from "src"` + // re-exports every named export EXCEPT + // `default`. Leaking it made barrels + // claim a default binding they never + // define, which breaks the #4872 + // has-default probe that decides whether + // a default import can bind to + // `perry_fn___default`. + if name == "default" { + continue; + } let already_exists = current_exports .map(|e| e.contains_key(name)) .unwrap_or(false); @@ -2423,8 +2434,36 @@ pub fn run_with_parse_cache( .find(|nl| nl.module == import.source); for spec in &import.specifiers { - // Handle namespace imports (import * as X) - if let perry_hir::ImportSpecifier::Namespace { local } = spec { + // Handle namespace imports (import * as X). + // + // Issue #4872: a DEFAULT import of a compiled module that + // has NO `default` export gets the same treatment. The + // CJS wrap lowers every `require('X')` to `import _req_N + // from 'X'`; when X resolves to an ESM barrel with only + // named exports (rxjs's src/index.ts, uid's index.mjs) or + // to a type-only interface surface with no exports at all + // (nestjs dist `*.interface.js`), there is no + // `perry_fn___default` symbol for the consumer to + // bind — the old fall-through registered the local as a + // callable function import and the link died on + // `__perry_wrap_perry_fn___default`. Node's + // `require(esm)` semantics hand back the module namespace + // object, so route the local through the namespace + // machinery: member reads resolve per-export to origin + // symbols, and a whole-value read materializes the + // namespace object (empty for zero-export modules). + let namespace_like_local: Option<&String> = match spec { + perry_hir::ImportSpecifier::Namespace { local } => Some(local), + perry_hir::ImportSpecifier::Default { local } + if !all_module_exports + .get(&resolved_path_str) + .is_some_and(|exports| exports.contains_key("default")) => + { + Some(local) + } + _ => None, + }; + if let Some(local) = namespace_like_local { namespace_imports.push(local.clone()); // Register all exports from the source module if let Some(exports) = all_module_exports.get(&resolved_path_str) { diff --git a/crates/perry/src/commands/compile/cjs_wrap/detect.rs b/crates/perry/src/commands/compile/cjs_wrap/detect.rs index 2b0579813e..81bd4a3ba6 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/detect.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/detect.rs @@ -28,6 +28,13 @@ pub(in crate::commands::compile) fn is_commonjs(source: &str) -> bool { } source.contains("module.exports") || source.contains("exports.") + // Issue #4872: tsc-compiled type-only modules (nestjs dist + // `*.interface.js`) contain ONLY the interop marker + // `Object.defineProperty(exports, "__esModule", { value: true });` + // — no `exports.X =`, no `require(`. Without this arm they fall + // through to the ESM pipeline, where the bare `exports` identifier + // throws a ReferenceError at module init. + || source.contains("defineProperty(exports,") || (source.contains("require(") && !source.contains("import ")) } diff --git a/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs b/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs index 4f633dea91..e1b9114dcb 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs @@ -21,6 +21,31 @@ pub fn extract_require_specifiers(source: &str) -> Vec { specs } +/// Issue #4872: extract `__exportStar(require('SPEC'), exports)` re-export +/// calls — the tsc-emitted CJS lowering of `export * from 'SPEC'`. Matches +/// the bare inline-helper form (`__exportStar(require("./x"), exports)`), +/// the tslib member form (`tslib_1.__exportStar(require("./x"), exports)`), +/// and the comma-sequenced form (`(0, tslib_1.__exportStar)(require("./x"), +/// exports)`). The helper *definition* (`var __exportStar = (this && ...)`) +/// never matches because the pattern requires a `require('...')` literal as +/// the first argument. Order preserved, deduped. +pub fn extract_export_star_specs(source: &str) -> Vec { + let re = regex::Regex::new( + r#"(?:[A-Za-z_$][A-Za-z0-9_$]*\s*\.\s*)?__exportStar\s*\)?\s*\(\s*require\s*\(\s*['"]([^'"]+)['"]\s*\)\s*,\s*exports\s*\)"#, + ) + .unwrap(); + let mut specs = Vec::new(); + for cap in re.captures_iter(source) { + if let Some(m) = cap.get(1) { + let s = m.as_str().to_string(); + if !specs.contains(&s) { + specs.push(s); + } + } + } + specs +} + /// Refs #488 drizzle-sqlite: extract `var = require("");` /// declarations from the source as `(alias_name, spec, (start_byte, /// end_byte))`. The byte range covers the whole matched statement so diff --git a/crates/perry/src/commands/compile/cjs_wrap/mod.rs b/crates/perry/src/commands/compile/cjs_wrap/mod.rs index 636ba3d240..432e027cb9 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/mod.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/mod.rs @@ -48,7 +48,9 @@ pub(self) use extract_exports::{ extract_exports_from_source, extract_named_exports_from_require, extract_object_literal_exports_from_require, extract_single_module_exports_assignment, }; -pub(self) use extract_requires::{extract_require_aliases_with_ranges, extract_require_specifiers}; +pub(self) use extract_requires::{ + extract_export_star_specs, extract_require_aliases_with_ranges, extract_require_specifiers, +}; pub(self) use hoist_classes::{ extract_top_level_class_decls, rewrite_module_exports_class_expression, }; diff --git a/crates/perry/src/commands/compile/cjs_wrap/wrap.rs b/crates/perry/src/commands/compile/cjs_wrap/wrap.rs index ad2cb744ed..ede87cff50 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/wrap.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/wrap.rs @@ -177,6 +177,18 @@ pub(in crate::commands::compile) fn wrap_commonjs_for_target( let mut named_exports = extract_exports_from_source(source); + // Issue #4872: `__exportStar(require('X'), exports)` is tsc's CJS + // lowering of `export * from 'X'` — emit exactly that as a real ESM + // re-export at module scope. The static `export *` lets compile.rs's + // transitive re-export propagation resolve names through multi-level + // barrels to their defining module (nestjs's `@nestjs/common/index.js` + // → `decorators/index.js` → `core/index.js` → `controller.decorator.js`), + // so a consumer's `import { Controller } from '@nestjs/common'` binds + // the origin's symbol instead of link-failing on + // `perry_fn___Controller`. The runtime copy inside the + // IIFE still runs, so `_cjs.X` property reads keep working too. + let export_star_specs = extract_export_star_specs(source); + // For trivial re-export wrappers (`module.exports = require('./X')`), // recursively pull in the target's named exports. Without this, // react/index.js — which has zero `exports.X =` patterns of its own — @@ -186,6 +198,15 @@ pub(in crate::commands::compile) fn wrap_commonjs_for_target( if !spec.starts_with("./") && !spec.starts_with("../") { continue; } + // #4872: specs re-exported via `__exportStar` surface through the + // static `export * from` emitted below — resolving to the ORIGIN + // module's symbols. Pulling the target's textual exports here would + // emit explicit `export const X = _cjs.X;` bindings that shadow the + // star re-export (ESM precedence) and degrade those names back to + // runtime property reads. + if export_star_specs.contains(spec) { + continue; + } let Some(target) = super::super::resolve::resolve_relative_import_path(spec, source_path) else { continue; @@ -368,6 +389,14 @@ pub(in crate::commands::compile) fn wrap_commonjs_for_target( None => "export default _cjs;".to_string(), }; + // #4872: ESM `export * from` declarations for every `__exportStar` + // call detected above. + let export_star_decls = export_star_specs + .iter() + .map(|spec| format!("export * from '{}';", spec)) + .collect::>() + .join("\n"); + let wrapped = format!( r#"{imports} {import_aliases} @@ -478,6 +507,7 @@ const _cjs = (function() {{ {direct_class_exports} {direct_named_reexports} {named_export_decls} +{export_star_decls} "# ); if std::env::var("PERRY_DEBUG_CJS_WRAP").is_ok() { diff --git a/crates/perry/tests/issue_4872_barrel_default_reexports.rs b/crates/perry/tests/issue_4872_barrel_default_reexports.rs new file mode 100644 index 0000000000..ce8543e460 --- /dev/null +++ b/crates/perry/tests/issue_4872_barrel_default_reexports.rs @@ -0,0 +1,163 @@ +//! Regression test for #4872: undefined default-wrapper symbols for +//! re-exported barrel modules (the nestjs link wall). +//! +//! Three shapes, all taken from the `tests/release/packages/nestjs-hello` +//! fixture's failure: +//! +//! 1. A tsc-emitted TYPE-ONLY module whose entire body is +//! `Object.defineProperty(exports, "__esModule", { value: true });` +//! (nestjs dist `*.interface.js`). Pre-fix it wasn't detected as CJS, so +//! it compiled as a zero-export ES module; the consumer's synthetic +//! `require()` still value-read its default import and the link died on +//! `__perry_wrap_perry_fn___default` (and, once that was fixed, the +//! unwrapped module threw `ReferenceError: exports is not defined` at +//! init). +//! +//! 2. `__exportStar(require("./x"), exports)` barrels (tsc's CJS lowering of +//! `export * from "./x"`), nested two levels deep +//! (`@nestjs/common/index.js` → `decorators/index.js` → leaf). Pre-fix +//! the wrap surfaced no static re-export, so the consumer's named import +//! bound `perry_fn___Controller` — which no object file defines. +//! +//! 3. A default import (synthesized by the CJS wrap from `require(...)`) of +//! an ES module that has ONLY named exports (rxjs `src/index.ts`, uid +//! `dist/index.mjs`). There is no `default` binding to call, so the local +//! now binds the module NAMESPACE (Node `require(esm)` semantics) and +//! member calls resolve per-export to origin symbols. +//! +//! Fixes (see PR for #4872): cjs_wrap emits `export * from ''` for +//! `__exportStar` calls; `is_commonjs` detects `defineProperty(exports,`; +//! compile.rs routes default imports of no-default modules through the +//! namespace machinery; `export *` propagation no longer leaks `default`. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +#[test] +fn barrel_default_imports_and_export_star_chains_link_and_run() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + + std::fs::write( + root.join("package.json"), + r#"{ + "name": "barrel-default-reexports", + "type": "module", + "perry": { + "compilePackages": ["fakepkg"], + "allow": { "compilePackages": ["fakepkg"] } + } +}"#, + ) + .expect("write consumer package.json"); + + let pkg = root.join("node_modules").join("fakepkg"); + std::fs::create_dir_all(&pkg).expect("mkdir fakepkg"); + std::fs::write( + pkg.join("package.json"), + r#"{ "name": "fakepkg", "version": "1.0.0", "main": "index.js" }"#, + ) + .expect("write fakepkg package.json"); + + // Shape 1: type-only interface surface — the nestjs `*.interface.js` + // dist output. No exports, no require, just the interop marker. + std::fs::write( + pkg.join("iface.js"), + "\"use strict\";\nObject.defineProperty(exports, \"__esModule\", { value: true });\n", + ) + .expect("write iface.js"); + + // Shape 3: ES module with ONLY named exports — no default binding. + std::fs::write( + pkg.join("esm-barrel.mjs"), + "export const uid = (n) => \"U:\" + n;\n", + ) + .expect("write esm-barrel.mjs"); + + // Shape 2: two-level `__exportStar` chain down to a concrete leaf. + std::fs::write( + pkg.join("leaf.js"), + r#""use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.Controller = void 0; +function Controller() { return "CTRL"; } +exports.Controller = Controller; +"#, + ) + .expect("write leaf.js"); + std::fs::write( + pkg.join("mid.js"), + r#""use strict"; +var __exportStar = (this && this.__exportStar) || function(m, exports) { + for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) exports[p] = m[p]; +}; +Object.defineProperty(exports, "__esModule", { value: true }); +__exportStar(require("./leaf"), exports); +"#, + ) + .expect("write mid.js"); + + // The package barrel: star re-exports the type-only surface AND the + // mid-level barrel, plus a member call on the no-default ES module. + std::fs::write( + pkg.join("index.js"), + r#""use strict"; +var __exportStar = (this && this.__exportStar) || function(m, exports) { + for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) exports[p] = m[p]; +}; +Object.defineProperty(exports, "__esModule", { value: true }); +__exportStar(require("./iface"), exports); +__exportStar(require("./mid"), exports); +const esm_1 = require("./esm-barrel.mjs"); +exports.greet = function greet() { return esm_1.uid(7); }; +"#, + ) + .expect("write index.js"); + + let entry = root.join("main.ts"); + std::fs::write( + &entry, + r#" +import { greet, Controller } from "fakepkg"; +// Shape 3: member call on a default-import of a no-default ES module. +console.log(greet()); +// Shape 2: named import resolved through the two-level __exportStar chain. +console.log(Controller()); +"#, + ) + .expect("write entry"); + + 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 (link wall 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"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + let stdout = String::from_utf8_lossy(&run.stdout); + assert_eq!( + stdout, "U:7\nCTRL\n", + "barrel re-exports must resolve to concrete origin bindings" + ); +} diff --git a/tests/release/packages/nestjs-hello/WALLS.md b/tests/release/packages/nestjs-hello/WALLS.md index 071b9058e9..ec30c8c75e 100644 --- a/tests/release/packages/nestjs-hello/WALLS.md +++ b/tests/release/packages/nestjs-hello/WALLS.md @@ -31,33 +31,64 @@ npm install - Legacy CommonJS inheritance patterns that call `Stream.call(this)` or `EventEmitter.call(this)` now lower to the same receiver initialization shape this fixture needs from Express/readable-stream. +- **Wall 1 (resolved, #4872)** — undefined default-wrapper symbols for + re-exported barrel modules (`__perry_wrap_perry_fn_..._rxjs_src_index_ts__default`, + the nestjs `*.interface.js` family, `uid_dist_index_mjs__default`, + `perry_fn_..._common_index_js__Controller`, …). Four coordinated fixes: + a default import of a compiled module with no `default` export now binds + the module namespace (Node `require(esm)` semantics) instead of a phantom + callable; `__exportStar(require("./x"), exports)` in CJS-wrapped sources + now also emits a real `export * from './x'` so multi-level tsc barrels + resolve named imports to the defining module; `export *` propagation no + longer leaks `default` across hops; and tsc-emitted type-only modules + whose only statement is `Object.defineProperty(exports, "__esModule", …)` + are now detected as CJS (previously they compiled as zero-export ESM and + threw `ReferenceError: exports is not defined` at init). A TS constructor + overload-signature miscount (rxjs `Notification` rejected with "may only + have one constructor") was fixed alongside. The fixture now **links**: + `Wrote executable` (~41 MB). ## Open -### Wall 1 - default-wrapper symbols for re-exported modules +### Wall 2 - `.prototype` of a capturing class expression is undefined (tslib `__decorate`) -After the resolved walls above, the fixture reaches native linking and then -fails with undefined default-wrapper symbols. Current representative symbols: +The binary now links but the server dies during module init with +`TypeError: Cannot convert undefined or null to object`. Root cause, bisected +to `@nestjs/common/services/logger.service.js`: tsc's class-decorator output -```text -undefined reference to `__perry_wrap_perry_fn_node_modules_rxjs_src_index_ts__default' -undefined reference to `perry_fn_node_modules_rxjs_src_index_ts__default' -undefined reference to `perry_fn_node_modules_rxjs_src_operators_index_ts__default' -undefined reference to `perry_fn_node_modules_uid_dist_index_mjs__default' -undefined reference to `__perry_wrap_perry_fn_node_modules__nestjs_core_router_interfaces_routes_interface_js__default' -undefined reference to `__perry_wrap_perry_fn_node_modules__nestjs_platform_express_interfaces_nest_express_application_interface_js__default' +```js +let Logger = Logger_1 = class Logger { ... }; +tslib_1.__decorate([Logger.WrapBuffer, ...], Logger.prototype, "error", null); ``` -These modules are import barrels or type/interface re-export surfaces rather -than concrete default function definitions. Call sites still emit default-call -and default-wrapper references for them, but no object file defines the matching -symbols. The next focused fix should make default-import/default-call lowering -follow re-exported module surfaces back to a concrete binding, or avoid emitting -callable default references for type-only/interface barrels. +calls `Object.getOwnPropertyDescriptor(Logger.prototype, "error")`, and +`Logger.prototype` reads as `undefined` (while `typeof Logger` is +`"function"`). Minimal repro — no CJS wrap needed; the trigger is a class +EXPRESSION inside a closure whose **getter captures an outer variable**, +which routes the class through the `ClassExprFresh` lowering +(`captured_args: [LocalGet(..)]` in HIR), and `.prototype` on a +`ClassExprFresh` value is not wired: + +```ts +const r = (function() { + var L_1: any; + let L: any = L_1 = class L { + get gi() { return L_1.staticRef; } // getter capture → ClassExprFresh + e(m: any) { return m; } + }; + return [typeof L, typeof L.prototype]; +})(); +// Perry: ["function", "undefined"] — node: ["function", "object"] +``` + +Without the capturing getter the same shape keeps a real `.prototype`. The +next focused fix should give `ClassExprFresh` values a live `.prototype` +object (tslib `__decorate` then mutates it via `defineProperty`, so instances +must observe the decorated methods). ## When this fixture flips to PASS -Once the open linker wall is gone and `fixture.sh` succeeds end-to-end, delete +Once the open runtime wall is gone and `fixture.sh` succeeds end-to-end, delete this `WALLS.md`. The fixture driver treats `WALLS.md` as the marker that turns compile/startup failures into SKIP; removing it converts those into hard FAILs so regressions past this baseline are visible.