fix(compile): #5918 — import_function_prefixes collides on exported name across unrelated imports#5919
Merged
proggeramlug merged 1 commit intoJul 4, 2026
Conversation
…orted name across unrelated imports import_function_prefixes (imported name -> source module prefix, consumed by ExternFuncRef codegen to form perry_fn_<prefix>__<name>) was populated by inserting BOTH the exported/origin name and the local alias for every renamed Named import. Per the PerryTS#35/PerryTS#321 precedent in the same function, a renamed import's ExternFuncRef in the HIR always carries the LOCAL name (unique per import site) -- the exported_name insert only ever mattered in the no-rename case (local == exported). Running it unconditionally meant: when two DIFFERENT import statements in the same file rename to different locals from modules whose ORIGIN export names happen to collide (common with short, minifier-style names -- "a"/"b"/"c" -- which is exactly what esbuild's chunk-splitting produces), the second insert silently overwrote the first's entry. The overwritten entry could be a completely unrelated LOCAL alias from an earlier import, repointing that alias's ExternFuncRef at the wrong module entirely. Confirmed via a real-world source compile of sst/opencode (compilePackages: ["*"]): remeda's real dist/chunk-*.js build output has this exact shape, and nearly every remeda function transitively imports the four affected chunks, so this single bug blocked the entire package. Fix: only insert under exported_name in the local_name == exported_name case; the aliased case inserts under local_name only, keeping every key in the map unique per file (local identifiers can't collide with each other within one file; export names from different origin modules can and do). Added a 4-file regression test reproducing the exact collision shape, lifted from remeda's real build output. Verified test_gap_renamed_ class_export_namespace.ts, test_issue_836_zod_class_reexports.ts, and test_issue_678_reexport_default.ts (the existing tests covering this same code path) are unaffected.
📝 WalkthroughWalkthroughModifies the ChangesImport prefix collision fix
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry/src/commands/compile/run_pipeline.rs`:
- Around line 2671-2697: The import-prefix mapping in the
`ModuleKind::Interpreted` import loop is still using the exported/origin name in
the aliased case, which can overwrite another local alias in the shared map.
Update the `import_function_prefixes` insertion logic in `run_pipeline` so
aliased imports are keyed only by `local_name`, while the no-rename path keeps
using `exported_name`; do not add or retain the extra `imported`/exported key
for aliased bindings. Keep this consistent with `import_function_origin_names`,
which already handles origin-name rewriting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f8a9fc3-1b0c-4f6f-bb40-28d5e220f9b3
📒 Files selected for processing (6)
crates/perry/src/commands/compile/run_pipeline.rstest-files/fixtures/issue_5918_pkg/chunk_anxbdsui.tstest-files/fixtures/issue_5918_pkg/chunk_d6fck2ga.tstest-files/fixtures/issue_5918_pkg/chunk_wimgwyzl.tstest-files/fixtures/issue_5918_pkg/chunk_wmcgp7py.tstest-files/test_issue_5918_import_prefix_name_collision.ts
Comment on lines
+2671
to
2697
| // Issue #<TBD>: only key by `exported_name` in the | ||
| // no-rename case (`local_name == exported_name`), where | ||
| // the HIR's `ExternFuncRef` genuinely carries that | ||
| // string. In the aliased case (#35/#321 above: | ||
| // `ExternFuncRef` carries the LOCAL name, unique per | ||
| // import site), inserting under `exported_name` too is | ||
| // not just redundant — `exported_name` is whatever the | ||
| // ORIGIN module happens to call it, so it can collide | ||
| // with an unrelated LOCAL alias elsewhere in the same | ||
| // file. Concrete repro: `import { a as n, c as a } from | ||
| // "./x"; import { a as t } from "./y"` — the second | ||
| // specifier's exported name "a" overwrote the first | ||
| // import's *local* alias "a" (from `c as a`), silently | ||
| // repointing `ExternFuncRef { name: "a" }` at module y | ||
| // instead of x. Only inserting the ALIASED case under | ||
| // `local_name` (never also under `exported_name`) keeps | ||
| // every key in this map unique per file — local names | ||
| // can't collide with each other (each `let`/import | ||
| // binding needs a distinct identifier), but exported | ||
| // names from different source modules can and do. | ||
| if local_name == exported_name { | ||
| import_function_prefixes | ||
| .insert(exported_name.clone(), effective_prefix.clone()); | ||
| } else { | ||
| import_function_prefixes | ||
| .insert(local_name.clone(), effective_prefix.clone()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether codegen ever looks up import_function_prefixes by the
# EXPORTED/origin name for a V8/Interpreted-backed aliased import (vs.
# only by local name), to determine if the extra `imported` insert at
# L3160-L3163 is load-bearing or a leftover of the same bug pattern
# fixed for NativeCompiled imports at L2671-L2697.
rg -n 'import_function_prefixes' crates/perry-codegen/src -C3
rg -n 'import_function_v8_specifiers' crates/perry-codegen/src -C3Repository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the V8/Interpreted import seeding loop and the places that consume
# import_function_origin_names / import_function_v8_specifiers.
sed -n '3128,3188p' crates/perry/src/commands/compile/run_pipeline.rs
printf '\n--- origin-name lookups ---\n'
rg -n 'import_function_origin_names' crates/perry-codegen/src -C2
printf '\n--- v8 specifier lookups ---\n'
rg -n 'import_function_v8_specifiers' crates/perry-codegen/src -C2
printf '\n--- prefix lookups using imported/local names in lowering ---\n'
rg -n 'import_function_prefixes\.(get|contains_key)\(' crates/perry-codegen/src -C2Repository: PerryTS/perry
Length of output: 33609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '3128,3188p' crates/perry/src/commands/compile/run_pipeline.rs
printf '\n--- origin-name lookups ---\n'
rg -n 'import_function_origin_names' crates/perry-codegen/src -C2
printf '\n--- v8 specifier lookups ---\n'
rg -n 'import_function_v8_specifiers' crates/perry-codegen/src -C2
printf '\n--- prefix lookups using imported/local names in lowering ---\n'
rg -n 'import_function_prefixes\.(get|contains_key)\(' crates/perry-codegen/src -C2Repository: PerryTS/perry
Length of output: 33609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact V8/Interpreted import loop and the consumers of the seeded maps.
sed -n '3148,3178p' crates/perry/src/commands/compile/run_pipeline.rs
printf '\n--- origin-name lookups in codegen ---\n'
rg -n 'import_function_origin_names' crates/perry-codegen/src -C1 | head -n 80
printf '\n--- v8 specifier lookups in codegen ---\n'
rg -n 'import_function_v8_specifiers' crates/perry-codegen/src -C1 | head -n 80Repository: PerryTS/perry
Length of output: 16944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '3148,3178p' crates/perry/src/commands/compile/run_pipeline.rs
printf '\n--- origin-name lookups in codegen ---\n'
rg -n 'import_function_origin_names' crates/perry-codegen/src -C1 | head -n 80
printf '\n--- v8 specifier lookups in codegen ---\n'
rg -n 'import_function_v8_specifiers' crates/perry-codegen/src -C1 | head -n 80Repository: PerryTS/perry
Length of output: 16927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- origin-name lookups ---\n'
rg -n 'import_function_origin_names\.(get|contains_key|insert)\(' crates/perry-codegen/src -C2
printf '\n--- v8-specifier lookups ---\n'
rg -n 'import_function_v8_specifiers\.(get|contains_key|insert)\(' crates/perry-codegen/src -C2
printf '\n--- named-import loop around the reviewed code ---\n'
sed -n '2660,2705p' crates/perry/src/commands/compile/run_pipeline.rsRepository: PerryTS/perry
Length of output: 242
Drop the extra imported key in the ModuleKind::Interpreted import loop below. The rest of codegen resolves these imports by the local binding, with import_function_origin_names handling the origin-name rewrite; keeping imported here can still overwrite an unrelated alias in the shared map.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry/src/commands/compile/run_pipeline.rs` around lines 2671 - 2697,
The import-prefix mapping in the `ModuleKind::Interpreted` import loop is still
using the exported/origin name in the aliased case, which can overwrite another
local alias in the shared map. Update the `import_function_prefixes` insertion
logic in `run_pipeline` so aliased imports are keyed only by `local_name`, while
the no-rename path keeps using `exported_name`; do not add or retain the extra
`imported`/exported key for aliased bindings. Keep this consistent with
`import_function_origin_names`, which already handles origin-name rewriting.
This was referenced Jul 3, 2026
Closed
Merged
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Jul 4, 2026
… collision
`import { Context } from "effect"` (named import of a value the source
module re-exports as a namespace via `export * as Context from
"./Context.js"`) registered every member of the target namespace into the
flat `import_function_prefixes` map, keyed only by the member's bare name.
When two such namespace-reexport targets imported into the same file
export a member with the same bare name (e.g. `Context.a` and `Option.a`,
both reached through effect's barrel), the second registration silently
overwrote the first.
perry already has the fix for this exact class of collision (PerryTS#680):
`namespace_member_prefixes`, keyed by `(namespace_local, member_name)`.
The plain `import * as X` branch already populates it; the
named-import-of-namespace-reexport branch did not. Fixed by registering
there too, and by teaching `expr/static_method.rs`'s
`namespace_imports.contains(class_name)` branch (the actual codegen
consumer for uppercase-receiver forms like `Context.a()`) to consult
`namespace_member_prefixes` before falling back to the flat map — mirroring
the pattern `namespace_call.rs` and `property_get.rs` already use for
lowercase-receiver call/read forms.
Found via a real-world source compile of `sst/opencode`, whose
`provider.ts` does `import { Effect, Layer, Context, Schema, Types } from
"effect"` — the last remaining pair of undefined symbols after PerryTS#5918/PerryTS#5919.
proggeramlug
added a commit
that referenced
this pull request
Jul 4, 2026
…esolution collisions (#5923) * fix(compile,codegen): #5922 — namespace-reexport member prefix collision `import { Context } from "effect"` (named import of a value the source module re-exports as a namespace via `export * as Context from "./Context.js"`) registered every member of the target namespace into the flat `import_function_prefixes` map, keyed only by the member's bare name. When two such namespace-reexport targets imported into the same file export a member with the same bare name (e.g. `Context.a` and `Option.a`, both reached through effect's barrel), the second registration silently overwrote the first. perry already has the fix for this exact class of collision (#680): `namespace_member_prefixes`, keyed by `(namespace_local, member_name)`. The plain `import * as X` branch already populates it; the named-import-of-namespace-reexport branch did not. Fixed by registering there too, and by teaching `expr/static_method.rs`'s `namespace_imports.contains(class_name)` branch (the actual codegen consumer for uppercase-receiver forms like `Context.a()`) to consult `namespace_member_prefixes` before falling back to the flat map — mirroring the pattern `namespace_call.rs` and `property_get.rs` already use for lowercase-receiver call/read forms. Found via a real-world source compile of `sst/opencode`, whose `provider.ts` does `import { Effect, Layer, Context, Schema, Types } from "effect"` — the last remaining pair of undefined symbols after #5918/#5919. * fix(compile,codegen): #5924 — namespace-reexport origin-name collision Follow-up to #5922/#5923: fixing `import_function_prefixes` alone wasn't enough for a real-world `sst/opencode` compile. `import_function_origin_names` (issue #678's symbol-suffix-override map for re-export renames) has the same flat, bare-name-keyed structure and independently broke the same real code. `import { Effect, Layer, Context, Schema, Types } from "effect"` processes five namespace-reexport targets into one file's shared maps. `Effect` re-exports something named `Service` under a rename, so its turn inserts `import_function_origin_names["Service"] = "<effect's internal origin name>"`. `Context` directly exports its own `Service` (no rename needed), so its turn inserts nothing — the earlier (wrong) entry survives. Codegen then resolves `Context.Service(...)` (a real call in opencode's `provider.ts`: `class Service extends Context.Service<...>()(...)`) via the contaminated flat entry, producing an undefined `perry_fn_..._Context_ts__<wrong-suffix>` symbol at link time. Fixed by adding `namespace_member_origin_names`, a per-namespace-scoped companion map mirroring #680's `namespace_member_prefixes` — but unconditionally populated for every namespace member (not just renamed ones), since a sparse map would still fall through to the contaminated flat map for unrenamed members. Consulted by every codegen site that builds a `perry_fn_<src>__<suffix>` symbol for a namespace-member access (`expr/static_method.rs`, `lower_call/namespace_call.rs`, `expr/property_get.rs`) via a new `import_origin_suffix_ns` helper. * fix(compile): #5927 — plain-import-vs-namespace-member flat map collision Third companion to #5918/#5922/#5924. A PLAIN named import's bare name and a NAMESPACE MEMBER's bare name share the same flat `import_function_prefixes`/`import_function_origin_names` maps. A plain import has no other resolution path (a bare call has no namespace to scope against), so it must always win — but namespace processing's unconditional `.insert()` could silently overwrite it depending on import-statement order. Found via a real-world `sst/opencode` compile: `provider.ts` does `import { omit } from "remeda"` (plain) and `import { Context } from "effect"` (namespace-reexport), and effect's real `Context.ts` also exports a member literally named `omit`. `Context` is imported after `omit` in source order, so Context's registration silently overwrote remeda's — the bare `omit(...)` call then resolved against Context.ts's prefix combined with remeda's origin-name rename, producing an undefined symbol request. Fixed by using `.entry().or_insert()` instead of `.insert()` for BOTH namespace-processing branches' flat-map writes (plain `import * as X` and the named-import-of-namespace-reexport branch). Plain imports keep their unconditional `.insert()`, so they always win regardless of processing order: if a namespace claims an empty slot first, the plain import's later unconditional insert still overwrites it; if the plain import claims it first, the namespace's `or_insert` sees the slot taken and skips. Namespace-member resolution is unaffected since the three real consumers (`expr/static_method.rs`, `lower_call/namespace_call.rs`, `expr/property_get.rs`) already prefer the namespace-scoped maps from #5922/#5924 and never need the flat-map fallback for genuine namespace members. Verified both import orderings against Node with new regression fixtures. * style: rustfmt property_get.rs import list CI's `cargo fmt --all -- --check` flagged the import_origin_suffix_ns addition from the #5922/#5924 fix — rustfmt wraps differently than the manual edit did. * fix(link): #5928 — fixed-point dedup for shared deps across well-known libs Companion to #5920/#5921. `strip_bundled_runtime_from_well_known_lib` only targets `perry_runtime-*` codegen-unit members. Programs linking multiple well-known libraries that each independently bundle a full "shared tokio" HTTP-client stack (e.g. both `http` and `fastify` need tokio/hyper_util/h2/ rustls/reqwest/ring) still collide on every OTHER shared transitive dependency those libraries have in common with `libperry_stdlib.a` — even `std`/`core`/`alloc` themselves. macOS's current linker has no `-multiply_defined suppress` / `-ld_classic` escape hatch anymore (verified obsolete on current toolchains via direct testing), so these surface as hard `ld: duplicate symbol` failures. Adds `strip_bundled_shared_deps_from_well_known_lib`, applying the same two safety rules as the runtime-specific function (stdlib bundles the identical codegen unit; no other kept member depends on a symbol only the duplicate-candidate provides) to every member, not just `perry_runtime-` ones. A naive one-shot widening is NOT safe — candidates can depend on each other (e.g. hyper_util's object referencing a symbol only tokio's object defines, both bundled in the same well-known lib and both initially flagged removable) — so this is a fixed-point iteration: each round recomputes undefined-symbol references from the shrinking kept-set and protects any still-needed candidate, repeating until stable. Verified against `issue_5920_wrapper_bundled_runtime_async_starvation` (passes) and a real-world `sst/opencode` compile: fully eliminates duplicate symbols for simpler well-known libs (ioredis/net/ws went from hundreds of duplicates to zero); substantially reduces them for the largest, most interconnected pair (http+fastify, both needing the full HTTP-client stack) though Rule 2 conservatively protects more members as a single archive's internal dependency graph grows, so this specific combination is improved but not fully zero yet. Note: `strip_bundled_runtime_from_well_known_lib`/this function require `llvm-objcopy`/`llvm-nm`/`llvm-ar` (via `PERRY_LLVM_OBJCOPY`/`PERRY_LLVM_NM`/ `PERRY_LLVM_AR` env vars or `PATH`) to actually run — without them they silently no-op (each wrapped in `.unwrap_or_else` with a "(non-fatal)" log line), producing a much later, confusing "N duplicate symbols" `ld` failure with no indication dedup was skipped. This cost significant debugging time this session before being traced to a missing local LLVM toolchain install — worth a louder diagnostic in a follow-up. --------- Co-authored-by: Ralph <ralph@skelpo.com>
proggeramlug
pushed a commit
to proggeramlug/perry
that referenced
this pull request
Jul 4, 2026
… collision
`import { Context } from "effect"` (named import of a value the source
module re-exports as a namespace via `export * as Context from
"./Context.js"`) registered every member of the target namespace into the
flat `import_function_prefixes` map, keyed only by the member's bare name.
When two such namespace-reexport targets imported into the same file
export a member with the same bare name (e.g. `Context.a` and `Option.a`,
both reached through effect's barrel), the second registration silently
overwrote the first.
perry already has the fix for this exact class of collision (PerryTS#680):
`namespace_member_prefixes`, keyed by `(namespace_local, member_name)`.
The plain `import * as X` branch already populates it; the
named-import-of-namespace-reexport branch did not. Fixed by registering
there too, and by teaching `expr/static_method.rs`'s
`namespace_imports.contains(class_name)` branch (the actual codegen
consumer for uppercase-receiver forms like `Context.a()`) to consult
`namespace_member_prefixes` before falling back to the flat map — mirroring
the pattern `namespace_call.rs` and `property_get.rs` already use for
lowercase-receiver call/read forms.
Found via a real-world source compile of `sst/opencode`, whose
`provider.ts` does `import { Effect, Layer, Context, Schema, Types } from
"effect"` — the last remaining pair of undefined symbols after PerryTS#5918/PerryTS#5919.
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
1 participant
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.
Fixes #5918.
Problem
import_function_prefixes(imported name → source module prefix, consumed byExternFuncRefcodegen to formperry_fn_<prefix>__<name>) was populated by inserting both the exported/origin name and the local alias for every renamedNamedimport:Per the #35/#321 precedent right above this in the same function, a renamed import's
Expr::ExternFuncRefin the HIR always carries the local name (unique per import site) — theexported_nameinsert only ever mattered in the no-rename case (local == exported).Running it unconditionally meant: when two different import statements in the same file rename to different locals from modules whose origin export names happen to collide (very common with short, minifier-style names — exactly what esbuild's chunk-splitting produces), the second insert silently overwrote the first's entry, even repointing a completely unrelated local alias at the wrong module.
Real-world impact
Found via a real-world source compile of
sst/opencodewithperry.compilePackages: ["*"].remeda's real, unmodifieddist/chunk-*.jsbuild output has this exact 4-chunk collision shape, and nearly every remeda function transitively imports the affected chunks — this single bug blocked the entire package from linking.Fix
Only insert under
exported_namewhenlocal_name == exported_name; the aliased case inserts underlocal_nameonly. This keeps every key in the map unique per file — local identifiers can't collide with each other within one file, but export names from different origin modules can and do.Testing
test_issue_5918_import_prefix_name_collision.ts+ a 4-file fixture reproducing the exact collision shape (lifted directly from remeda's real build output). Output verified byte-for-byte againstnode --experimental-strip-types.test_gap_renamed_class_export_namespace.ts,test_issue_836_zod_class_reexports.ts,test_issue_678_reexport_default.ts.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests