From 00696f0269faf010701f6706ddbbfcb5bbb2e277 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Tue, 5 May 2026 18:47:28 +0200 Subject: [PATCH 1/4] Add test --- .../turbopack/remove-unused-imports/member/input/a.js | 9 +++++++++ .../turbopack/remove-unused-imports/member/input/b.js | 1 + .../remove-unused-imports/member/input/index.js | 5 +++++ .../turbopack/remove-unused-imports/member/options.json | 3 +++ 4 files changed, 18 insertions(+) create mode 100644 turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/a.js create mode 100644 turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/b.js create mode 100644 turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/index.js create mode 100644 turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/options.json diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/a.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/a.js new file mode 100644 index 000000000000..816958923b48 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/a.js @@ -0,0 +1,9 @@ +import { b } from './b.js' + +export function a() { + return sub.call() +} + +function sub() { + return b +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/b.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/b.js new file mode 100644 index 000000000000..57e2a26314d4 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/b.js @@ -0,0 +1 @@ +export const b = 123 diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/index.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/index.js new file mode 100644 index 000000000000..d8a24b3a359e --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/index.js @@ -0,0 +1,5 @@ +import { a } from './a.js' + +it('should work', () => { + expect(a()).toBe(123) +}) diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/options.json b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/options.json new file mode 100644 index 000000000000..bf30b867b721 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/options.json @@ -0,0 +1,3 @@ +{ + "scopeHoisting": false +} From 4c0efb7d623cc3294629d85c05c32dedebdd7dfd Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Tue, 5 May 2026 18:56:45 +0200 Subject: [PATCH 2/4] Fix --- .../src/analyzer/imports.rs | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs index 0ec518ffa86a..6065c2f12f01 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs @@ -775,6 +775,7 @@ mod analyzer_state { pub(super) struct AnalyzerState { is_in_fn: bool, cur_top_level_decl_name: Option, + is_in_static_member_expr_obj: bool, } impl AnalyzerState { @@ -815,6 +816,20 @@ mod analyzer_state { self.state.is_in_fn = old_is_in_fn; result } + + pub(super) fn set_is_in_static_member_expr_obj(&mut self) { + // Doesn't use visitor, because this propery is only valid for the current node, not the + // whole subtree. + self.state.is_in_static_member_expr_obj = true; + } + + pub(super) fn take_is_in_static_member_expr_obj(&mut self) -> bool { + // Doesn't use visitor, because this propery is only valid for the current node, not the + // whole subtree. + let v = self.state.is_in_static_member_expr_obj; + self.state.is_in_static_member_expr_obj = false; + v + } } } @@ -1223,17 +1238,26 @@ impl Visit for Analyzer<'_> { } fn visit_member_expr(&mut self, node: &MemberExpr) { - if let MemberProp::Ident(..) | MemberProp::PrivateName(..) = &node.prop - && node.obj.is_ident() + if matches!( + &node.prop, + MemberProp::Ident(..) | MemberProp::PrivateName(..) + ) && matches!(&*node.obj, Expr::Ident(_)) { - // Skip traversing if obj is a Expr::Ident, so that it doesn't get added to - // full_star_imports below in visit_expr. + // So that it doesn't get added to full_star_imports below in visit_expr. + self.set_is_in_static_member_expr_obj(); + } + node.visit_children_with(self); + } - // TODO this currently doesn't properly mark the import in self.program_decl_usage, see - // todo in - // turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/ - // import-star/input/index.js - return; + fn visit_expr(&mut self, node: &Expr) { + // Must always be called unconditionally to reset the flag + let is_in_static_member_expr_obj = self.take_is_in_static_member_expr_obj(); + + if !is_in_static_member_expr_obj + && let Expr::Ident(i) = node + && let Some(module_path) = self.namespace_imports_to_specifier.get(&i.to_id()) + { + self.data.full_star_imports.insert(module_path.clone()); } node.visit_children_with(self); } @@ -1258,15 +1282,6 @@ impl Visit for Analyzer<'_> { node.visit_children_with(self); } - fn visit_expr(&mut self, node: &Expr) { - if let Expr::Ident(i) = node - && let Some(module_path) = self.namespace_imports_to_specifier.get(&i.to_id()) - { - self.data.full_star_imports.insert(module_path.clone()); - } - node.visit_children_with(self); - } - fn visit_ident(&mut self, node: &Ident) { let id = node.to_id(); if let Some((esm_reference_index, _)) = self.data.get_binding(&id) { From 998911d1422c0147194b06cbc502ad21904a316e Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Tue, 5 May 2026 19:38:07 +0200 Subject: [PATCH 3/4] rename test --- .../remove-unused-imports/{member => member-call}/input/a.js | 0 .../remove-unused-imports/{member => member-call}/input/b.js | 0 .../remove-unused-imports/{member => member-call}/input/index.js | 0 .../remove-unused-imports/{member => member-call}/options.json | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/{member => member-call}/input/a.js (100%) rename turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/{member => member-call}/input/b.js (100%) rename turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/{member => member-call}/input/index.js (100%) rename turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/{member => member-call}/options.json (100%) diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/a.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/input/a.js similarity index 100% rename from turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/a.js rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/input/a.js diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/b.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/input/b.js similarity index 100% rename from turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/b.js rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/input/b.js diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/index.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/input/index.js similarity index 100% rename from turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/input/index.js rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/input/index.js diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/options.json b/turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/options.json similarity index 100% rename from turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member/options.json rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/remove-unused-imports/member-call/options.json From 6acffb615c93a6d5b0218848900b0677c08c39a2 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Tue, 5 May 2026 19:39:55 +0200 Subject: [PATCH 4/4] better fix --- .../src/analyzer/imports.rs | 33 +++++-------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs index 6065c2f12f01..c1c22519eda1 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs @@ -775,7 +775,6 @@ mod analyzer_state { pub(super) struct AnalyzerState { is_in_fn: bool, cur_top_level_decl_name: Option, - is_in_static_member_expr_obj: bool, } impl AnalyzerState { @@ -816,20 +815,6 @@ mod analyzer_state { self.state.is_in_fn = old_is_in_fn; result } - - pub(super) fn set_is_in_static_member_expr_obj(&mut self) { - // Doesn't use visitor, because this propery is only valid for the current node, not the - // whole subtree. - self.state.is_in_static_member_expr_obj = true; - } - - pub(super) fn take_is_in_static_member_expr_obj(&mut self) -> bool { - // Doesn't use visitor, because this propery is only valid for the current node, not the - // whole subtree. - let v = self.state.is_in_static_member_expr_obj; - self.state.is_in_static_member_expr_obj = false; - v - } } } @@ -1241,20 +1226,20 @@ impl Visit for Analyzer<'_> { if matches!( &node.prop, MemberProp::Ident(..) | MemberProp::PrivateName(..) - ) && matches!(&*node.obj, Expr::Ident(_)) + ) && let Expr::Ident(ident) = &*node.obj { - // So that it doesn't get added to full_star_imports below in visit_expr. - self.set_is_in_static_member_expr_obj(); + // Intentionally skipping over visit_expr(node.obj) here so that it doesn't get added to + // full_star_imports below in visit_expr. + ident.visit_with(self); + } else { + node.visit_children_with(self); } - node.visit_children_with(self); } fn visit_expr(&mut self, node: &Expr) { - // Must always be called unconditionally to reset the flag - let is_in_static_member_expr_obj = self.take_is_in_static_member_expr_obj(); - - if !is_in_static_member_expr_obj - && let Expr::Ident(i) = node + // Careful about adding anything here, visit_member_expr might skip over this method for + // some Expr::Ident-s. + if let Expr::Ident(i) = node && let Some(module_path) = self.namespace_imports_to_specifier.get(&i.to_id()) { self.data.full_star_imports.insert(module_path.clone());