From a871399f11a69b3dc512ab13bab5d7036f19bd42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 12 Jul 2026 17:03:15 +0200 Subject: [PATCH] fix(codegen): follow re-export hops when resolving an export's origin (#6304, #5916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs with the same shape: Perry resolved an exported name to the module that *mentions* it rather than the module that *defines* it, and so never followed the re-export hop to the real binding. #6304 — dynamic import() namespace of a pure re-export chunk was empty. esbuild/bun emit a shared chunk under `--splitting` as a two-statement re-export file: // agent-VP4LHHJR.js import { run } from "./chunk-XXXX.js"; export { run }; `flatten_exports` treated `export { run }` as a LOCAL export of the agent module. The driver then searched the agent's own HIR for a function/class/ global named `run`, found nothing (it is an IMPORT binding, not a definition), and fell back to a `ForeignVar` getter on `perry_fn___run` — a symbol the #461 stub loop claims with an undefined-returning stub. Net effect: the whole namespace of a re-export-only chunk came out `undefined`, and an unguarded `ns.run()` printed `undefined` instead of throwing — a SILENT wrong answer that quietly broke every code-split bundle. A chunk containing real code was unaffected, which is why this hid for so long. Fix: `flatten_exports` now resolves an `Export::Named` whose local is an import binding to the module that actually defines it, following chains of import/re-export hops to the ultimate owner. Native imports and sources with no HIR in the graph are left alone (the resolver returns `None` and callers keep their prior behaviour), so this can only move an entry CLOSER to a real definition, never invent one. #5916 — barrel re-export of a namespace value failed to LINK. // selfns.ts export * as Token from "./selfns" // reexport_selfns.ts export { Token, estimate } from "./selfns" // main.ts import { Token } from "./reexport_selfns"; Token.estimate("hi") Undefined symbols: ___perry_wrap_perry_fn_reexport_selfns_ts__Token The consumer-side namespace detection only inspected the imported-from module's OWN exports for a `NamespaceReExport`. Across a `export { X } from` hop it saw a plain `ReExport`, fell through to the ordinary value path, and `Token.estimate(...)` lowered to a `StaticMethodCall` against a closure wrapper no module emits. Fix: walk the re-export chain first, so the scan runs against the module that actually DECLARES the namespace, under the name it declares it with. When no hop applies (the common case) the scan target is unchanged. Note: the issue's stated hypothesis — that the `Export::Named`-only destructuring in artifacts.rs's wrapper loops is the cause — does not hold. A `ReExport` of an ordinary function/const across the same hop already links and runs correctly today; it is specifically a NAMESPACE-valued binding that needs the namespace routing. The two issues are therefore different roots in the same family, and are fixed independently here. Verification: * #6304 repro built with real `esbuild --bundle --splitting --format=esm`: perry output is now byte-identical to `node out/index.js`. * #5916 repro from the issue links and prints `3 / 3`, matching node. * New gap fixtures (parity vs `node --experimental-strip-types`): test_gap_dynamic_import_reexport_chunk.ts (re-export-only chunk in a dynamic-import namespace, incl. renamed re-export + a real-code control chunk) and test_gap_namespace_reexport_barrel.ts (#5916 link case). * 6 new flatten_exports unit tests (rename, multi-hop chain, local-definition precedence, native-import exclusion, cycle termination); perry-hir lib suite green (229 passed). * Static-import barrels through both shapes still work (no regression), and all 10 pre-existing dynamic-import gap tests still pass. --- crates/perry-hir/src/dynamic_import.rs | 234 +++++++++++++++++- crates/perry-hir/src/dynamic_import/tests.rs | 196 +++++++++++++++ .../src/commands/compile/run_pipeline.rs | 105 +++++++- test-files/dynamic_import_chunk_agent.ts | 16 ++ test-files/dynamic_import_chunk_shared.ts | 9 + test-files/dynamic_import_chunk_worker.ts | 11 + test-files/namespace_reexport_barrel.ts | 14 ++ test-files/namespace_reexport_selfns.ts | 12 + .../test_gap_dynamic_import_reexport_chunk.ts | 27 ++ .../test_gap_namespace_reexport_barrel.ts | 21 ++ 10 files changed, 630 insertions(+), 15 deletions(-) create mode 100644 test-files/dynamic_import_chunk_agent.ts create mode 100644 test-files/dynamic_import_chunk_shared.ts create mode 100644 test-files/dynamic_import_chunk_worker.ts create mode 100644 test-files/namespace_reexport_barrel.ts create mode 100644 test-files/namespace_reexport_selfns.ts create mode 100644 test-files/test_gap_dynamic_import_reexport_chunk.ts create mode 100644 test-files/test_gap_namespace_reexport_barrel.ts diff --git a/crates/perry-hir/src/dynamic_import.rs b/crates/perry-hir/src/dynamic_import.rs index 19eab27ea6..9fe5ac376e 100644 --- a/crates/perry-hir/src/dynamic_import.rs +++ b/crates/perry-hir/src/dynamic_import.rs @@ -144,11 +144,40 @@ fn flatten_into<'a, F>( for export in &module.exports { match export { Export::Named { local, exported } => { + // #6304: `export { run }` does NOT imply `run` is defined here. + // When `run` is an *import* binding of this module — + // import { run } from "./chunk-XXXX.js"; + // export { run }; + // — the value lives in `./chunk-XXXX.js`, not here. That two- + // statement file is exactly the shape esbuild/bun emit for a + // shared chunk under `--splitting`, so it is the common case, + // not an edge case. + // + // Pre-fix this pushed `source_module = `, and the + // driver's namespace-entry classifier then searched THIS + // module's HIR for a function/class/global named `run`, found + // nothing (it is an import, not a definition), and fell back to + // a `ForeignVar` getter call on `perry_fn___run` — + // which the #461 stub loop claims with an undefined-returning + // stub. Net effect: the whole namespace of a re-export-only + // chunk came out `undefined`, silently. + // + // Resolving the binding to its defining module instead lets the + // classifier see the real function/class/global and emit the + // proper closure/global reference. + let origin = + resolve_binding_origin(module_name, local, lookup).unwrap_or_else(|| { + BindingOrigin { + source_module: module_name.to_string(), + source_local: local.clone(), + namespace_of: None, + } + }); out.push(FlatExport { name: exported.clone(), - source_module: module_name.to_string(), - source_local: local.clone(), - nested_namespace_of: None, + source_module: origin.source_module, + source_local: origin.source_local, + nested_namespace_of: origin.namespace_of, }); } Export::ReExport { @@ -156,17 +185,27 @@ fn flatten_into<'a, F>( imported, exported, } => { - // The value lives in `source`; if `source` re-exports it - // again, we want to follow that chain so codegen reaches - // the ULTIMATE owner. But for the MVP we surface one hop - // — the cross-module access pattern works regardless of - // how many hops away the value's defining module is, as - // long as we name the directly-importing source. + // The value lives in `source` — but `source` may itself + // re-export it (a barrel chain, or a bundler emitting one + // chunk that re-exports another). Follow the chain to the + // ULTIMATE owner so the classifier sees a real definition + // rather than another forwarding stub. When nothing can be + // followed, `resolve_binding_origin` returns `None` and we + // fall back to naming the directly-importing source — the + // long-standing one-hop behaviour. + let origin = + resolve_binding_origin(source, imported, lookup).unwrap_or_else(|| { + BindingOrigin { + source_module: source.clone(), + source_local: imported.clone(), + namespace_of: None, + } + }); out.push(FlatExport { name: exported.clone(), - source_module: source.clone(), - source_local: imported.clone(), - nested_namespace_of: None, + source_module: origin.source_module, + source_local: origin.source_local, + nested_namespace_of: origin.namespace_of, }); } Export::ExportAll { source } => { @@ -188,6 +227,177 @@ fn flatten_into<'a, F>( } } +/// #6304: where an exported name's value actually lives, after following +/// import bindings and re-export hops through the module graph. +struct BindingOrigin { + /// Module that owns the binding. + source_module: String, + /// The name the binding has *in* `source_module`. + source_local: String, + /// `Some(m)` when the binding is the module namespace of `m` rather than + /// a plain value (`import * as X` / `export * as X`). + namespace_of: Option, +} + +/// True when `module` actually *defines* `name` (as opposed to merely +/// importing or re-exporting it). A definition stops origin resolution. +fn defines_local_binding(module: &Module, name: &str) -> bool { + module.functions.iter().any(|f| f.name == name) + || module.classes.iter().any(|c| c.name == name) + || module.globals.iter().any(|g| g.name == name) + || module.enums.iter().any(|e| e.name == name) +} + +/// The import binding (if any) that `name` refers to in `module`. +/// +/// Native / builtin imports (`import { readFile } from "fs"`) are deliberately +/// excluded: their source is not a compiled module in the graph, so redirecting +/// an export to them would name a module that has no HIR and no +/// `perry_fn_*` symbols. Those keep the pre-existing local-lookup behaviour. +fn find_import_binding(module: &Module, name: &str) -> Option<(String, ImportBindingKind)> { + for import in &module.imports { + if import.type_only || import.is_native { + continue; + } + for spec in &import.specifiers { + match spec { + crate::ir::ImportSpecifier::Named { imported, local } if local == name => { + return Some(( + import.source.clone(), + ImportBindingKind::Value(imported.clone()), + )); + } + crate::ir::ImportSpecifier::Default { local } if local == name => { + return Some(( + import.source.clone(), + ImportBindingKind::Value("default".to_string()), + )); + } + crate::ir::ImportSpecifier::Namespace { local } if local == name => { + return Some((import.source.clone(), ImportBindingKind::Namespace)); + } + _ => {} + } + } + } + None +} + +enum ImportBindingKind { + /// A plain value binding; the payload is the name in the source module. + Value(String), + /// The whole module namespace of the import's source. + Namespace, +} + +/// #6304: resolve `(module_name, local)` to the module that actually defines +/// the binding, following import bindings and `ReExport` / `NamespaceReExport` +/// hops. +/// +/// Returns `None` when nothing could be followed — either `module_name` already +/// defines the binding, or the chain leaves the compiled-module graph (a native +/// import, or a source we have no HIR for). Callers then keep their pre-existing +/// default, so this is strictly a refinement: it can only move an entry CLOSER +/// to a real definition, never invent one. +/// +/// Cycle-safe: a `(module, name)` pair already visited terminates the walk, so a +/// self-referential barrel (`export * as Token from "./selfns"` inside +/// `selfns.ts`) cannot loop forever. +fn resolve_binding_origin<'a, F>( + start_module: &str, + start_local: &str, + lookup: &F, +) -> Option +where + F: Fn(&str) -> Option<&'a Module>, +{ + let mut module_name = start_module.to_string(); + let mut local = start_local.to_string(); + let mut seen: HashSet<(String, String)> = HashSet::new(); + // Only report an origin once we've actually moved somewhere new; otherwise + // the caller's existing default already names the right module. + let mut moved = false; + + loop { + if !seen.insert((module_name.clone(), local.clone())) { + break; + } + let Some(module) = lookup(&module_name) else { + break; + }; + // A real definition here — this is the owner. + if defines_local_binding(module, &local) { + break; + } + + // `import { x } from "src"; export { x }` — hop to `src`. + if let Some((source, kind)) = find_import_binding(module, &local) { + if lookup(&source).is_none() { + break; + } + match kind { + ImportBindingKind::Value(imported) => { + module_name = source; + local = imported; + moved = true; + continue; + } + ImportBindingKind::Namespace => { + return Some(BindingOrigin { + source_module: source.clone(), + source_local: String::new(), + namespace_of: Some(source), + }); + } + } + } + + // `export { x } from "src"` / `export * as X from "src"` — hop through + // the re-export. Lets a chain of barrels (or bundler chunks that + // re-export one another) reach the ultimate owner. + let mut hopped = false; + for export in &module.exports { + match export { + Export::ReExport { + source, + imported, + exported, + } if *exported == local => { + if lookup(source).is_none() { + break; + } + module_name = source.clone(); + local = imported.clone(); + moved = true; + hopped = true; + break; + } + Export::NamespaceReExport { source, name } if *name == local => { + if lookup(source).is_none() { + break; + } + return Some(BindingOrigin { + source_module: source.clone(), + source_local: String::new(), + namespace_of: Some(source.clone()), + }); + } + _ => {} + } + } + if hopped { + continue; + } + break; + } + + moved.then(|| BindingOrigin { + source_module: module_name, + source_local: local, + namespace_of: None, + }) +} + /// Issue #100 / #1725 / #1674: collect every `Stmt::Let { init: Some(_), .. }` /// reachable in the module into a `local_id → init_expr` map — the module-init /// body, every function / method / constructor body, and (descending) nested diff --git a/crates/perry-hir/src/dynamic_import/tests.rs b/crates/perry-hir/src/dynamic_import/tests.rs index 47a7cc2c6c..1e150f49d5 100644 --- a/crates/perry-hir/src/dynamic_import/tests.rs +++ b/crates/perry-hir/src/dynamic_import/tests.rs @@ -941,6 +941,202 @@ fn flatten_export_all_cycle_safe() { assert!(names.contains(&"fromB".to_string())); } +/// #6304 test scaffolding: a module that DEFINES `name` as an exported +/// function, so origin resolution has something real to stop on. +fn module_defining_fn(module: &str, name: &str) -> Module { + let mut m = Module::new(module); + m.functions.push(crate::ir::Function { + id: 0, + name: name.into(), + type_params: vec![], + params: vec![], + return_type: Type::Any, + body: vec![], + is_async: false, + is_generator: false, + is_strict: false, + is_exported: true, + captures: vec![], + decorators: vec![], + was_plain_async: false, + was_unrolled: false, + }); + m.exports.push(Export::Named { + local: name.into(), + exported: name.into(), + }); + m +} + +/// #6304: a plain non-native named import of `imported` from `source`. +fn named_import(source: &str, imported: &str, local: &str) -> crate::ir::Import { + crate::ir::Import { + source: source.into(), + specifiers: vec![crate::ir::ImportSpecifier::Named { + imported: imported.into(), + local: local.into(), + }], + is_native: false, + module_kind: crate::ir::ModuleKind::NativeCompiled, + resolved_path: None, + type_only: false, + is_dynamic: false, + is_dynamic_target: false, + is_deferred_require: false, + is_adopted_require: false, + } +} + +#[test] +fn flatten_reexport_only_chunk_resolves_to_defining_module() { + // #6304 — the canonical esbuild/bun `--splitting` shared-chunk shape: + // + // // agent-VP4LHHJR.js + // import { run } from "./chunk-XXXX.js"; + // export { run }; + // + // `run` is an IMPORT binding, not a definition. Pre-fix this flattened to + // `source_module = agent`, the driver found no local `run` there, and the + // namespace entry degraded to an undefined-returning stub — so `ns.run` + // came out `undefined` for the whole chunk. + let chunk = module_defining_fn("chunk", "run"); + let mut agent = Module::new("agent"); + agent.imports.push(named_import("chunk", "run", "run")); + agent.exports.push(Export::Named { + local: "run".into(), + exported: "run".into(), + }); + let map = std::collections::HashMap::from([ + ("chunk".to_string(), chunk), + ("agent".to_string(), agent), + ]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("agent", &lookup); + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].name, "run"); + // The value lives in `chunk`, NOT in `agent`. + assert_eq!(flat[0].source_module, "chunk"); + assert_eq!(flat[0].source_local, "run"); + assert_eq!(flat[0].nested_namespace_of, None); +} + +#[test] +fn flatten_reexport_only_chunk_honours_import_rename() { + // `import { run as go } from "./chunk"; export { go as run }` — the export + // key is the consumer-visible `run`, but the binding in `chunk` is `run` + // (the import's ORIGINAL name), not the local alias `go`. + let chunk = module_defining_fn("chunk", "run"); + let mut agent = Module::new("agent"); + agent.imports.push(named_import("chunk", "run", "go")); + agent.exports.push(Export::Named { + local: "go".into(), + exported: "run".into(), + }); + let map = std::collections::HashMap::from([ + ("chunk".to_string(), chunk), + ("agent".to_string(), agent), + ]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("agent", &lookup); + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].name, "run"); + assert_eq!(flat[0].source_module, "chunk"); + assert_eq!(flat[0].source_local, "run"); +} + +#[test] +fn flatten_reexport_chain_reaches_ultimate_owner() { + // A chain of forwarding chunks must land on the module that actually + // defines the binding, not on the first hop (which would itself only + // forward, yielding another undefined stub). + // outer -> mid (import+export) -> impl (definition) + let impl_mod = module_defining_fn("impl", "run"); + let mut mid = Module::new("mid"); + mid.imports.push(named_import("impl", "run", "run")); + mid.exports.push(Export::Named { + local: "run".into(), + exported: "run".into(), + }); + let mut outer = Module::new("outer"); + outer.imports.push(named_import("mid", "run", "run")); + outer.exports.push(Export::Named { + local: "run".into(), + exported: "run".into(), + }); + let map = std::collections::HashMap::from([ + ("impl".to_string(), impl_mod), + ("mid".to_string(), mid), + ("outer".to_string(), outer), + ]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("outer", &lookup); + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].source_module, "impl"); + assert_eq!(flat[0].source_local, "run"); +} + +#[test] +fn flatten_local_definition_still_wins_over_same_named_import() { + // A module that DEFINES the name it exports must keep pointing at itself — + // the redirect only fires for bindings this module does not define. + let mut m = module_defining_fn("m", "run"); + // A same-named import would be invalid TS, but assert the definition wins + // regardless so the redirect can never hijack a real local body. + m.imports.push(named_import("other", "run", "run")); + let other = module_defining_fn("other", "run"); + let map = std::collections::HashMap::from([("m".to_string(), m), ("other".to_string(), other)]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("m", &lookup); + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].source_module, "m"); + assert_eq!(flat[0].source_local, "run"); +} + +#[test] +fn flatten_reexport_of_native_import_is_not_redirected() { + // `import { readFile } from "fs"; export { readFile }` — "fs" is a native + // module with no HIR and no `perry_fn_*` symbols. Redirecting there would + // name a module that does not exist in the graph, so the native import is + // skipped and the entry keeps its pre-existing local shape. + let mut m = Module::new("m"); + let mut imp = named_import("fs", "readFile", "readFile"); + imp.is_native = true; + m.imports.push(imp); + m.exports.push(Export::Named { + local: "readFile".into(), + exported: "readFile".into(), + }); + let map = std::collections::HashMap::from([("m".to_string(), m)]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("m", &lookup); + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].source_module, "m"); + assert_eq!(flat[0].source_local, "readFile"); +} + +#[test] +fn flatten_reexport_binding_cycle_terminates() { + // Two chunks that forward the same name to each other — must not loop. + let mut a = Module::new("a"); + a.imports.push(named_import("b", "v", "v")); + a.exports.push(Export::Named { + local: "v".into(), + exported: "v".into(), + }); + let mut b = Module::new("b"); + b.imports.push(named_import("a", "v", "v")); + b.exports.push(Export::Named { + local: "v".into(), + exported: "v".into(), + }); + let map = std::collections::HashMap::from([("a".to_string(), a), ("b".to_string(), b)]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("a", &lookup); + // Terminates (no stack overflow / hang) and still yields the name. + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].name, "v"); +} + #[test] fn flatten_namespace_re_export() { let mut m = Module::new("m"); diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index d49330190e..6e4ac60ae0 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -17,6 +17,12 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use crate::OutputFormat; +/// #5916: how many `export { X } from "src"` hops to follow when resolving an +/// imported name back to the module that actually declares it. Barrel chains in +/// the wild are a handful of levels deep at most; the cap is a belt-and-braces +/// stop so a pathological (or cyclic) re-export graph can never spin forever. +const MAX_REEXPORT_HOPS: usize = 16; + /// Builds the complete codegen view of a foreign HIR class. /// /// Callers choose only the import binding (when the route introduces one) and @@ -1737,6 +1743,30 @@ pub fn run_with_parse_cache( perry_hir::Export::Named { .. } => {} } } + // #6304: `flatten_exports` also has to resolve `export { run }` where + // `run` is an IMPORT binding (`import { run } from "./chunk.js"` — the + // shape esbuild/bun emit for a shared chunk under `--splitting`). That + // walk reads `Import::source`, which likewise holds the raw specifier, + // so normalize it to `Module::name` on the same terms as the export + // sources above. Unresolvable / native sources are left verbatim; the + // resolver's `lookup` then misses and it falls back to the local + // behaviour instead of naming a module that has no HIR. + for import in rewritten.imports.iter_mut() { + if import.is_native { + continue; + } + if let Some((resolved_path, _)) = resolve_import( + &import.source, + path, + &ctx.project_root, + &ctx.compile_packages, + &ctx.compile_package_dirs, + ) { + if let Some(name) = path_to_module_name.get(&resolved_path) { + import.source = name.clone(); + } + } + } module_name_to_module.insert(hir_module.name.clone(), rewritten); } // Set of native-module paths that are dynamic-import targets. We @@ -2579,6 +2609,75 @@ pub fn run_with_parse_cache( } } + // Issue #5916: the `NamespaceReExport` we are about to scan + // for may not sit in the module we import from — it can be + // one or more `export { X } from "src"` hops upstream. A + // barrel that does + // export { Token } from "./selfns" // ReExport + // where `selfns.ts` declares + // export * as Token from "./selfns" // NamespaceReExport + // exposes `Token` as a NAMESPACE, but the scan below only + // looked at the barrel's own exports, saw a plain `ReExport`, + // and fell through to the ordinary value path. `Token.estimate(…)` + // then lowered to a `StaticMethodCall` referencing + // `__perry_wrap_perry_fn___Token` — a closure wrapper + // no module emits, so the link failed outright. (A ReExport of + // an ordinary function/const across the same hop links fine; + // it is specifically a NAMESPACE-valued binding that needs the + // namespace routing.) + // + // Walk the re-export chain first so the scan runs against the + // module that actually DECLARES the namespace, under the name + // it declares it with. When no hop applies (the overwhelmingly + // common case) this leaves the scan target exactly where it was, + // so behaviour is unchanged. + let mut ns_scan_hir = source_module; + let mut ns_scan_path = resolved_path_str.clone(); + let mut ns_scan_name = exported_name.clone(); + for _ in 0..MAX_REEXPORT_HOPS { + let Some(hir) = ns_scan_hir else { break }; + // Already the declaring module — nothing to follow. + if hir.exports.iter().any(|e| { + matches!( + e, + perry_hir::Export::NamespaceReExport { name, .. } + if *name == ns_scan_name + ) + }) { + break; + } + // Follow `export { } from ""`. + let Some((hop_src, hop_imported)) = + hir.exports.iter().find_map(|e| match e { + perry_hir::Export::ReExport { + source, + imported, + exported, + } if *exported == ns_scan_name => { + Some((source.clone(), imported.clone())) + } + _ => None, + }) + else { + break; + }; + let Some((hop_path, _)) = resolve_import( + &hop_src, + std::path::Path::new(&ns_scan_path), + &ctx.project_root, + &ctx.compile_packages, + &ctx.compile_package_dirs, + ) else { + break; + }; + let Some(hop_hir) = ctx.native_modules.get(&hop_path) else { + break; + }; + ns_scan_path = hop_path.to_string_lossy().to_string(); + ns_scan_hir = Some(hop_hir); + ns_scan_name = hop_imported; + } + // Issue #310: when the source module re-exports the // imported name as a namespace (`export * as Foo from // "./Foo"`), the local binding behaves identically to @@ -2589,17 +2688,17 @@ pub fn run_with_parse_cache( // name, then route the local through `namespace_imports` // + register the namespace target's full export surface. let mut handled_as_namespace_reexport = false; - if let Some(src_hir) = source_module { + if let Some(src_hir) = ns_scan_hir { for export in &src_hir.exports { if let perry_hir::Export::NamespaceReExport { source: ns_src, name, } = export { - if name != &exported_name { + if name != &ns_scan_name { continue; } - let importer = std::path::Path::new(&resolved_path_str); + let importer = std::path::Path::new(&ns_scan_path); let Some((ns_target, _)) = resolve_import( ns_src, importer, diff --git a/test-files/dynamic_import_chunk_agent.ts b/test-files/dynamic_import_chunk_agent.ts new file mode 100644 index 0000000000..af65cf3c72 --- /dev/null +++ b/test-files/dynamic_import_chunk_agent.ts @@ -0,0 +1,16 @@ +// Helper for test_gap_dynamic_import_reexport_chunk.ts — issue #6304. +// +// A PURE RE-EXPORT chunk: the exact two-statement shape esbuild/bun emit for +// a shared chunk under `--splitting` (`import { … } from "./chunk-XXXX.js"; +// export { … };`). Nothing is defined here — every exported name is an IMPORT +// binding whose value lives in the shared chunk. +// +// Pre-fix, `flatten_exports` treated `export { run }` as a LOCAL export of this +// module, the driver found no local `run` to bind, and the namespace entry +// degraded to an undefined-returning stub — so `(await import(...)).run` was +// `undefined` and calling it returned `undefined` instead of throwing. +import { run, VERSION } from "./dynamic_import_chunk_shared.ts"; + +// Also exercise the import-rename shape: the binding in the shared chunk is +// `VERSION`, re-exported here under a different consumer-visible key. +export { run, VERSION as version }; diff --git a/test-files/dynamic_import_chunk_shared.ts b/test-files/dynamic_import_chunk_shared.ts new file mode 100644 index 0000000000..32503bde83 --- /dev/null +++ b/test-files/dynamic_import_chunk_shared.ts @@ -0,0 +1,9 @@ +// Helper for test_gap_dynamic_import_reexport_chunk.ts — stands in for the +// `chunk-XXXX.js` esbuild/bun hoist the shared code into under `--splitting`. +// This module holds the REAL definitions; the sibling agent/worker chunks +// only forward to it. +export function run(tag: string): string { + return "[shared-util] " + tag; +} + +export const VERSION = "1.0.0"; diff --git a/test-files/dynamic_import_chunk_worker.ts b/test-files/dynamic_import_chunk_worker.ts new file mode 100644 index 0000000000..398c66f8ff --- /dev/null +++ b/test-files/dynamic_import_chunk_worker.ts @@ -0,0 +1,11 @@ +// Helper for test_gap_dynamic_import_reexport_chunk.ts — the CONTROL chunk. +// +// Unlike the agent chunk, this one contains real code (a local function) on top +// of its imports from the shared chunk. This shape already worked before #6304; +// keeping it in the fixture guards against a fix that trades the re-export-only +// case for the ordinary one. +import { run, VERSION } from "./dynamic_import_chunk_shared.ts"; + +export function work(): string { + return run("worker") + " v" + VERSION; +} diff --git a/test-files/namespace_reexport_barrel.ts b/test-files/namespace_reexport_barrel.ts new file mode 100644 index 0000000000..357a195536 --- /dev/null +++ b/test-files/namespace_reexport_barrel.ts @@ -0,0 +1,14 @@ +// Helper for test_gap_namespace_reexport_barrel.ts — issue #5916. +// +// A barrel that forwards BOTH an ordinary value (`estimate`) and a +// NAMESPACE-valued binding (`Token`) across one `export { … } from` hop. +// +// Pre-fix, the consumer-side namespace detection only inspected THIS module's +// own exports for a `NamespaceReExport`. It found a plain `ReExport`, fell +// through to the ordinary value path, and `Token.estimate(…)` in the consumer +// lowered to a `StaticMethodCall` against +// `__perry_wrap_perry_fn___Token` — a wrapper symbol nobody emits, +// so the whole program failed to LINK. The ordinary `estimate` re-export across +// the same hop always linked fine; it is specifically the namespace-valued +// binding that needs the namespace routing. +export { Token, estimate } from "./namespace_reexport_selfns.ts"; diff --git a/test-files/namespace_reexport_selfns.ts b/test-files/namespace_reexport_selfns.ts new file mode 100644 index 0000000000..b46befe55d --- /dev/null +++ b/test-files/namespace_reexport_selfns.ts @@ -0,0 +1,12 @@ +// Helper for test_gap_namespace_reexport_barrel.ts — issue #5916. +// +// Declares a namespace-valued export by re-exporting ITSELF under a name +// (`export * as Token from "./this-file"`). This is the shape Effect / zod +// barrels use pervasively, and the value of `Token` is a module NAMESPACE, not +// an ordinary function or const. +export * as Token from "./namespace_reexport_selfns.ts"; + +const CHARS_PER_TOKEN = 4; + +export const estimate = (input: string): number => + Math.max(0, Math.round(input.length / CHARS_PER_TOKEN)); diff --git a/test-files/test_gap_dynamic_import_reexport_chunk.ts b/test-files/test_gap_dynamic_import_reexport_chunk.ts new file mode 100644 index 0000000000..ca298bfbff --- /dev/null +++ b/test-files/test_gap_dynamic_import_reexport_chunk.ts @@ -0,0 +1,27 @@ +// Issue #6304: the namespace object of a dynamically-imported PURE RE-EXPORT +// chunk must expose the re-exported bindings. +// +// `dynamic_import_chunk_agent.ts` is the two-statement re-export file esbuild +// and bun emit for a shared chunk under `--splitting` (`import { run } from +// "./chunk-XXXX.js"; export { run };`). Pre-fix its namespace came out empty: +// `typeof ns.run` was `undefined`, and an unguarded `ns.run()` printed +// `undefined` rather than throwing — a SILENT wrong answer that quietly broke +// every code-split bundle. +// +// The worker chunk is the control: a chunk that contains real code (not just +// re-exports) already worked and must keep working. + +async function main(): Promise { + const ns = await import("./dynamic_import_chunk_agent.ts"); + console.log("typeof ns:", typeof ns); + console.log("typeof ns.run:", typeof ns.run); + console.log("ns.run():", ns.run("agent")); + // Re-export under a renamed key resolves to the shared chunk's binding. + console.log("ns.version:", ns.version); + + const w = await import("./dynamic_import_chunk_worker.ts"); + console.log("typeof w.work:", typeof w.work); + console.log("w.work():", w.work()); +} + +main(); diff --git a/test-files/test_gap_namespace_reexport_barrel.ts b/test-files/test_gap_namespace_reexport_barrel.ts new file mode 100644 index 0000000000..564cf5101f --- /dev/null +++ b/test-files/test_gap_namespace_reexport_barrel.ts @@ -0,0 +1,21 @@ +// Issue #5916: re-exporting a NAMESPACE-valued binding through a barrel +// (`export { Token } from "./mod"`, where `mod` declares `export * as Token`) +// must resolve to the namespace, not to a bare function symbol. +// +// Pre-fix this did not even link: +// Undefined symbols for architecture arm64: +// "___perry_wrap_perry_fn___Token", referenced from: _main +// because the consumer-side namespace detection only looked at the barrel's own +// exports and never followed the `export { … } from` hop to the module that +// actually declares the namespace. +// +// Importing the same names DIRECTLY from the declaring module (no barrel hop) +// always worked — the bug is specifically about surviving the re-export hop. + +import { Token, estimate } from "./namespace_reexport_barrel.ts"; + +// Namespace-valued re-export: member access must dispatch through the namespace. +console.log(Token.estimate("hello world")); + +// Ordinary value re-export across the same hop (this already worked — guard it). +console.log(estimate("hello world"));