fix: preserve barrel-renamed declared name for CJS require() imports (#2071) - #2296
Merged
carlos-alm merged 1 commit intoAug 6, 2026
Merged
fix: preserve barrel-renamed declared name for CJS require() imports (#2071)#2296carlos-alm merged 1 commit into
carlos-alm merged 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
…2071) buildImportArtifactNames's local barrel-tracing helper kept only the resolved file and dropped the resolved declared name, asymmetric with buildImportedNamesMap's ESM path. Extract the shared traceBarrelTarget helper in resolve-imports.ts so both paths trace barrel renames identically and can no longer silently diverge. No user-facing behavior change (docs check acknowledged) — internal resolver-internals fix with no new languages, commands, or architecture. Impact: 7 functions changed, 15 affected
Copy link
Copy Markdown
Contributor
Greptile SummaryThis PR centralizes barrel-target tracing and preserves the underlying declared name for CommonJS bindings passing through renamed barrel exports.
Confidence Score: 5/5The PR appears safe to merge, with no concrete behavioral, build, or security regression identified. The shared helper preserves prior ESM semantics, correctly carries renamed CJS declarations, and the richer map values remain unobserved by the sole consumer, which only checks key presence. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Import["ESM import or CJS require binding"] --> Resolve["Resolve imported path"]
Resolve --> Trace["traceBarrelTarget(path, name)"]
Trace --> Check{"Barrel file?"}
Check -->|No| Original["Return original file and name"]
Check -->|Yes| Barrel["resolveBarrelExportCached"]
Barrel --> Found{"Export resolved?"}
Found -->|Yes| Target["Return declaring file and translated name"]
Found -->|No| Original
Target --> Map["Store import-artifact resolution"]
Original --> Map
Map --> Classify["resolveReceiverEdge checks key presence"]
Reviews (1): Last reviewed commit: "fix: preserve barrel-renamed declared na..." | Re-trigger Greptile |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Codegraph Impact Analysis7 functions changed → 15 callers affected across 3 files
|
Sorry, something went wrong.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
buildImportArtifactNames's local barrel-tracing helper (traceBarrel) resolved through barrel chains via resolveBarrelExportCached but kept only resolved.file, discarding resolved.name — the declared name after a barrel-rename translation (export { realName as friendlyName } from './underlying'). This was asymmetric with the ESM import path's own traceBarrel in buildImportedNamesMap (same file), which correctly threads .name through.
No consumer currently reads the dropped value — resolveReceiverEdge, the sole consumer of importArtifactNames, only calls .has() for import-artifact classification, never .get() — so this was latent, not an observed bug. But if a future consumer needs the correct declared name (mirroring how the ESM path already works), the CJS-through-a-renamed-barrel case would have silently carried the wrong name with no test coverage catching it.
Fix
Extracted a single shared traceBarrelTarget(ctx, resolvedPath, name) helper into resolve-imports.ts — the module that already owns isBarrelFile/resolveBarrelExportCached/BarrelExportResolution. Both buildImportedNamesMap (ESM) and buildImportArtifactNames (CJS require()) now call this one implementation instead of maintaining separate private copies, so they can no longer silently diverge the way they did here.
buildImportArtifactNames's return type widened from Map<string,string> (name -> file) to ReadonlyMap<string, BarrelExportResolution> (name -> {file, name}), matching the fix sketch from the issue. resolveReceiverEdge's importedNames parameter type widened to ReadonlyMap<string, unknown> since it only ever checks .has() and is fed either the plain ESM map or the richer artifact map.
No native-engine changes — the issue confirms there's no native equivalent of buildImportArtifactNames to diverge from.
Verification
Filed #2295 for a related but out-of-scope cleanup: three other inline barrel-trace call sites in build-edges.ts duplicate the same fallback pattern but are already correct, so consolidating them onto traceBarrelTarget too was left for a follow-up.
Closes #2071