Skip to content

fix: record local alias for dynamic import() destructuring renames #1970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
carlos-alm wants to merge 1 commit into
base: fix/issue-1823-barrel-re-export-with-rename-export-x-as-y
Choose a base branch
from
Open

fix: record local alias for dynamic import() destructuring renames #1970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 104 additions & 16 deletions crates/codegraph-core/src/extractors/javascript.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,13 @@ fn handle_var_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
if let Some(str_arg) = find_child(&args, "string") {
let mod_path = node_text(&str_arg, source)
.replace(&['\'', '"'][..], "");
let names = collect_object_pattern_names(&name_n, source);
// CJS require bindings never populate renamed_imports —
// resolve_call_targets deliberately ignores the original
// name for these (empty target_file forces a same-file
// fallback match, matching WASM's importedNamesMap
// exclusion, #1678) — so the rename pairs collected here
// are discarded.
let names = collect_object_pattern_names(&name_n, source, &mut Vec::new());
if !names.is_empty() {
let mut imp = Import::new(
mod_path, names, start_line(node),
Expand Down Expand Up @@ -1672,9 +1678,13 @@ fn handle_dynamic_import(node: &Node, _fn_node: &Node, source: &[u8], symbols: &
if let Some(str_node) = str_node {
let mod_path = node_text(&str_node, source)
.replace(&['\'', '"', '`'][..], "");
let names = extract_dynamic_import_names(node, source);
let mut renamed_imports = Vec::new();
let names = extract_dynamic_import_names(node, source, &mut renamed_imports);
let mut imp = Import::new(mod_path, names, start_line(node));
imp.dynamic_import = Some(true);
if !renamed_imports.is_empty() {
imp.renamed_imports = Some(renamed_imports);
}
symbols.imports.push(imp);
}
}
Expand Down Expand Up @@ -3086,9 +3096,19 @@ const DYNAMIC_IMPORT_WRAPPER_KINDS: &[&str] =

/// Extract named bindings from a dynamic `import()` call expression.
/// Handles: `const { a, b } = await import(...)`, `const mod = await import(...)`,
/// and casts/parens wrapping the awaited call, e.g.
/// `const { a } = (await import(...)) as { a: Fn }`.
fn extract_dynamic_import_names(call_node: &Node, source: &[u8]) -> Vec<String> {
/// casts/parens wrapping the awaited call, e.g.
/// `const { a } = (await import(...)) as { a: Fn }`, and destructuring
/// renames, e.g. `const { a: b } = await import(...)`.
///
/// `renamed_out` is populated with `{ local, imported }` pairs for every
/// `{ imported: local }` specifier — mirrors `extract_import_names_with_renames`'s
/// static-import convention (#1730) so call-edge resolution can recover the
/// original exported name when a call site uses the local alias (#1824).
fn extract_dynamic_import_names(
call_node: &Node,
source: &[u8],
renamed_out: &mut Vec<RenamedImport>,
) -> Vec<String> {
// Walk up through any combination/nesting of await/parenthesized/as-cast
// wrappers to reach the variable_declarator.
let mut current = call_node.parent();
Expand All @@ -3107,15 +3127,19 @@ fn extract_dynamic_import_names(call_node: &Node, source: &[u8]) -> Vec<String>
return Vec::new();
};
match name_node.kind() {
"object_pattern" => collect_object_pattern_names(&name_node, source),
"object_pattern" => collect_object_pattern_names(&name_node, source, renamed_out),
"identifier" => vec![node_text(&name_node, source).to_string()],
"array_pattern" => collect_array_pattern_names(&name_node, source),
_ => Vec::new(),
}
}

/// Collect names from `const { a, b } = await import(...)`
fn collect_object_pattern_names(pattern: &Node, source: &[u8]) -> Vec<String> {
fn collect_object_pattern_names(
pattern: &Node,
source: &[u8],
renamed_out: &mut Vec<RenamedImport>,
) -> Vec<String> {
let mut names = Vec::new();
for i in 0..pattern.child_count() {
let Some(child) = pattern.child(i) else { continue };
Expand All @@ -3124,9 +3148,46 @@ fn collect_object_pattern_names(pattern: &Node, source: &[u8]) -> Vec<String> {
names.push(node_text(&child, source).to_string());
}
"pair_pattern" | "pair" => {
// { exportName: localAlias } → extract the key (export name)
if let Some(key) = child.child_by_field_name("key") {
names.push(node_text(&key, source).to_string());
// { imported: local } → the local binding (`value`) is what
// call sites actually reference; `key` is the name exported
// by the target module. Preferring `key` unconditionally (as
// this branch used to) silently dropped the local alias for
// every renamed destructure — the same class of bug fixed for
// static `import { X as Y }` specifiers in #1730 (#1824).
let key = child.child_by_field_name("key");
let value = child.child_by_field_name("value");
let local_node = match value.map(|v| (v.kind(), v)) {
Some(("identifier", v)) | Some(("shorthand_property_identifier_pattern", v)) => {
Some(v)
}
Some(("assignment_pattern", v)) => {
// { imported: local = defaultValue } — the local
// binding is the assignment_pattern's left identifier.
v.child_by_field_name("left")
.filter(|left| left.kind() == "identifier")
}
_ => None,
};
match (local_node, key) {
(Some(local_node), Some(key)) => {
let local_text = node_text(&local_node, source).to_string();
let key_text = node_text(&key, source).to_string();

@greptile-apps greptile-apps Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Raw Literal Keys Miss Exports

When the native extractor sees a dynamic import destructure such as const { 'foo-bar': local } = await import('./mod.js'), node_text(&key, source) includes the quotes or computed-key syntax. renamed_imports.imported then no longer matches the target export name, so the alias call edge is not created and the exported symbol can be reported as unused.

Fix in Claude Code

if local_text != key_text {
renamed_out.push(RenamedImport {
local: local_text.clone(),
imported: key_text,
});
}
names.push(local_text);
}
(None, Some(key)) => {
// Nested pattern (`{ foo: { nested } }`) or other
// unsupported value shape — no single local binding
// to extract; fall back to the key so the specifier
// isn't dropped entirely.
names.push(node_text(&key, source).to_string());
}
_ => {}
}
}
"object_assignment_pattern" => {
Expand Down Expand Up @@ -4318,12 +4379,25 @@ mod tests {

#[test]
fn finds_dynamic_import_with_aliased_destructuring() {
// #1824: the local binding actually referenced by call sites
// (`fromBarrel`) must be recorded in `names`, not the name exported by
// the target module (`buildGraph`) — mirrors the static
// `import { X as Y }` fix from #1730. `renamed_imports` carries the
// local → original mapping so call-edge resolution can still find
// `buildGraph` in the target file.
let s = parse_js("const { buildGraph: fromBarrel } = await import('./builder.js');");
let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect();
assert_eq!(dyn_imports.len(), 1);
assert_eq!(dyn_imports[0].source, "./builder.js");
assert!(dyn_imports[0].names.contains(&"buildGraph".to_string()));
assert!(!dyn_imports[0].names.contains(&"fromBarrel".to_string()));
assert!(dyn_imports[0].names.contains(&"fromBarrel".to_string()));
assert!(!dyn_imports[0].names.contains(&"buildGraph".to_string()));
let renamed = dyn_imports[0]
.renamed_imports
.as_ref()
.expect("renamed_imports should be populated for a renamed destructure");
assert_eq!(renamed.len(), 1);
assert_eq!(renamed[0].local, "fromBarrel");
assert_eq!(renamed[0].imported, "buildGraph");
}

#[test]
Expand All @@ -4333,18 +4407,32 @@ mod tests {
assert_eq!(dyn_imports.len(), 1);
assert_eq!(dyn_imports[0].source, "./mod.js");
assert!(dyn_imports[0].names.contains(&"a".to_string()));
assert!(dyn_imports[0].names.contains(&"buildGraph".to_string()));
assert!(dyn_imports[0].names.contains(&"fromBarrel".to_string()));
assert!(dyn_imports[0].names.contains(&"c".to_string()));
assert!(!dyn_imports[0].names.contains(&"fromBarrel".to_string()));
assert!(!dyn_imports[0].names.contains(&"buildGraph".to_string()));
let renamed = dyn_imports[0]
.renamed_imports
.as_ref()
.expect("renamed_imports should be populated for a renamed destructure");
assert_eq!(renamed.len(), 1);
assert_eq!(renamed[0].local, "fromBarrel");
assert_eq!(renamed[0].imported, "buildGraph");
}

#[test]
fn finds_dynamic_import_with_aliased_default_destructuring() {
let s = parse_js("const { buildGraph: local = null } = await import('./builder.js');");
let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect();
assert_eq!(dyn_imports.len(), 1);
assert!(dyn_imports[0].names.contains(&"buildGraph".to_string()));
assert!(!dyn_imports[0].names.contains(&"local".to_string()));
assert!(dyn_imports[0].names.contains(&"local".to_string()));
assert!(!dyn_imports[0].names.contains(&"buildGraph".to_string()));
let renamed = dyn_imports[0]
.renamed_imports
.as_ref()
.expect("renamed_imports should be populated for a renamed destructure");
assert_eq!(renamed.len(), 1);
assert_eq!(renamed[0].local, "local");
assert_eq!(renamed[0].imported, "buildGraph");
}

#[test]
Expand Down
59 changes: 52 additions & 7 deletions src/extractors/javascript.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,12 +810,14 @@ function collectDynamicImport(node: TreeSitterNode, imports: Import[]): boolean
const strArg = findChild(args, 'string');
if (strArg) {
const modPath = strArg.text.replace(/['"]/g, '');
const names = extractDynamicImportNames(node);
const renamedImports: Array<{ local: string; imported: string }> = [];
const names = extractDynamicImportNames(node, renamedImports);
imports.push({
source: modPath,
names,
line: nodeStartLine(node),
dynamicImport: true,
...(renamedImports.length > 0 ? { renamedImports } : {}),
});
} else {
debug(
Expand Down Expand Up @@ -1555,8 +1557,15 @@ function handleDynamicImportCall(node: TreeSitterNode, imports: Import[]): void
const strArg = findChild(args, 'string');
if (strArg) {
const modPath = strArg.text.replace(/['"]/g, '');
const names = extractDynamicImportNames(node);
imports.push({ source: modPath, names, line: nodeStartLine(node), dynamicImport: true });
const renamedImports: Array<{ local: string; imported: string }> = [];
const names = extractDynamicImportNames(node, renamedImports);
imports.push({
source: modPath,
names,
line: nodeStartLine(node),
dynamicImport: true,
...(renamedImports.length > 0 ? { renamedImports } : {}),
});
} else {
debug(
`Skipping non-static dynamic import() at line ${nodeStartLine(node)} (template literal or variable)`,
Expand Down Expand Up @@ -4177,13 +4186,22 @@ const DYNAMIC_IMPORT_WRAPPER_TYPES = new Set([
* const { a, b } = await import('./foo.js') → ['a', 'b']
* const mod = await import('./foo.js') → ['mod']
* const { a } = (await import('./foo.js')) as { a: Fn } → ['a']
* const { a: b } = await import('./foo.js') → ['b']
* import('./foo.js') → [] (no names extractable)
*
* Walks up the AST from the call_expression — through any nesting of
* await/parenthesized/as-cast wrappers — to find the enclosing
* variable_declarator and reads the name/object_pattern.
*
* `renamedOut`, when supplied, is populated with `{ local, imported }` pairs
* for every `{ imported: local }` specifier — mirrors `extractImportNames`'s
* static-import convention (#1730) so call-edge resolution can recover the
* original exported name when a call site uses the local alias (#1824).
*/
function extractDynamicImportNames(callNode: TreeSitterNode): string[] {
function extractDynamicImportNames(
callNode: TreeSitterNode,
renamedOut?: Array<{ local: string; imported: string }>,
): string[] {
// Walk up through await_expression / parenthesized_expression / as_expression
// wrappers, in any combination or order, to reach the variable_declarator.
let current = callNode.parent;
Expand All @@ -4204,10 +4222,37 @@ function extractDynamicImportNames(callNode: TreeSitterNode): string[] {
if (child.type === 'shorthand_property_identifier_pattern') {
names.push(child.text);
} else if (child.type === 'pair_pattern') {
// { a: localName } → use localName (the alias) for the local binding,
// but use the key (original name) for import resolution
// { imported: local } → the local binding (`value`) is what call
// sites actually reference; `key` is the name exported by the target
// module. Preferring `key` unconditionally (as this branch used to)
// silently dropped the local alias for every renamed destructure,
// the same class of bug fixed for static `import { X as Y }`
// specifiers in #1730 (#1824).
const key = child.childForFieldName('key');
if (key) names.push(key.text);
const value = child.childForFieldName('value');
let localNode: TreeSitterNode | undefined;
if (
value?.type === 'identifier' ||
value?.type === 'shorthand_property_identifier_pattern'
) {
localNode = value;
} else if (value?.type === 'assignment_pattern') {
// { imported: local = defaultValue } — the local binding is the

@greptile-apps greptile-apps Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Raw Literal Keys Miss Exports

When a dynamic import uses a valid quoted or computed destructuring key like const { 'foo-bar': local } = await import('./mod.js'), this stores the raw key text as imported. The resolver then looks for an export named 'foo-bar' or ['foo-bar'] instead of foo-bar, so calls through local() stay unresolved and the real export can still be marked dead.

Fix in Claude Code

// assignment_pattern's left-hand identifier.
const left = value.childForFieldName('left');
if (left?.type === 'identifier') localNode = left;
}
if (localNode && key) {
names.push(localNode.text);
if (localNode.text !== key.text) {
renamedOut?.push({ local: localNode.text, imported: key.text });
}
} else if (key) {
// Nested pattern (`{ foo: { nested } }`) or other unsupported
// value shape — no single local binding to extract; fall back to
// the key so the specifier isn't dropped entirely.
names.push(key.text);
}
}
}
return names;
Expand Down
4 changes: 4 additions & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,10 @@ export interface Import {
* `extractImportNames`), so `resolveBarrelExport` uses this map to translate
* a consumer's requested external name back to X before matching against
* `names`/looking up the underlying definition (#1823).
*
* Also populated for dynamic `import()` destructuring renames
* (`const { X: Y } = await import(...)`): same local/original split as the
* static case, produced by `extractDynamicImportNames` (#1824).
*/
renamedImports?: Array<{ local: string; imported: string }>;
/**
Expand Down
Loading
Loading