From 9c03e471dccbcefa65b44196fe33a4792d5c0f10 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Mon, 10 Nov 2025 13:05:33 +0000 Subject: [PATCH 1/4] fix(es/minifier): Fix merge_imports to preserve import order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes the `merge_imports` option in the compression transform to preserve the original import order and prevent imports from being placed after their usage. **Problem:** When `merge_imports` was enabled, imports from the same module were being merged correctly, but the merged imports were being added at the end of the module body, causing them to appear after their usage statements. This broke the execution order and made the output not aesthetically pleasing. **Solution:** - Track the index of the first occurrence of each import group - Insert merged imports at the position of the first occurrence - Maintain the original import order by processing items sequentially **Test:** Added test case for issue #11257 that verifies imports are merged and placed in the correct position. Fixes #11257 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../src/pass/postcompress.rs | 67 ++++++++++++------- .../tests/fixture/issues/11257/input.js | 6 ++ .../tests/fixture/issues/11257/output.js | 4 ++ 3 files changed, 54 insertions(+), 23 deletions(-) create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/11257/input.js create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/11257/output.js diff --git a/crates/swc_ecma_minifier/src/pass/postcompress.rs b/crates/swc_ecma_minifier/src/pass/postcompress.rs index ea9491e37e8a..fd59d69322a9 100644 --- a/crates/swc_ecma_minifier/src/pass/postcompress.rs +++ b/crates/swc_ecma_minifier/src/pass/postcompress.rs @@ -132,10 +132,10 @@ impl SpecifierKey { /// This optimization reduces bundle size by combining multiple imports from /// the same source into a single import declaration. fn merge_imports_in_module(module: &mut Module) { - // Group imports by source and metadata - let mut import_groups: FxHashMap> = FxHashMap::default(); + // Group imports by source and metadata, also track the first occurrence index + let mut import_groups: FxHashMap)> = FxHashMap::default(); - for item in module.body.iter() { + for (idx, item) in module.body.iter().enumerate() { if let ModuleItem::ModuleDecl(ModuleDecl::Import(import_decl)) = item { // Skip side-effect only imports (no specifiers) if import_decl.specifiers.is_empty() { @@ -145,41 +145,62 @@ fn merge_imports_in_module(module: &mut Module) { let key = ImportKey::from_import_decl(import_decl); import_groups .entry(key) - .or_default() + .or_insert_with(|| (idx, Vec::new())) + .1 .push(import_decl.clone()); } } + // Build a map of indices where merged imports should be inserted + let mut inserts_at: FxHashMap> = FxHashMap::default(); + + for (key, (first_idx, import_decls)) in import_groups.iter() { + if import_decls.len() > 1 { + let merged_imports = merge_import_decls(import_decls, key); + inserts_at.insert(*first_idx, merged_imports); + } + } + // Remove all imports that will be merged (except side-effect imports) - module.body.retain(|item| { + // and insert merged imports at the position of the first occurrence + let mut new_body = Vec::new(); + let mut processed_indices = FxHashMap::default(); + + for (idx, item) in module.body.iter().enumerate() { if let ModuleItem::ModuleDecl(ModuleDecl::Import(import_decl)) = item { // Keep side-effect imports if import_decl.specifiers.is_empty() { - return true; + new_body.push(item.clone()); + continue; } let key = ImportKey::from_import_decl(import_decl); - // Only keep if there's just one import for this key (no merging needed) - import_groups.get(&key).map_or(true, |v| v.len() <= 1) - } else { - true - } - }); - // Create merged imports and add them back - for (key, import_decls) in import_groups.iter() { - if import_decls.len() <= 1 { - // No merging needed, already retained above - continue; - } + // Check if this import is part of a merge group + if let Some((first_idx, decls)) = import_groups.get(&key) { + if decls.len() > 1 { + // This import needs to be merged + if idx == *first_idx && processed_indices.insert(*first_idx, ()).is_none() { + // This is the first occurrence - insert merged imports here + for merged in inserts_at.get(first_idx).unwrap() { + new_body + .push(ModuleItem::ModuleDecl(ModuleDecl::Import(merged.clone()))); + } + } + // Skip this individual import (it's been merged) + continue; + } + } - let merged_imports = merge_import_decls(import_decls, key); - for merged in merged_imports { - module - .body - .push(ModuleItem::ModuleDecl(ModuleDecl::Import(merged))); + // Keep imports that don't need merging + new_body.push(item.clone()); + } else { + // Keep all non-import items + new_body.push(item.clone()); } } + + module.body = new_body; } /// Merge multiple ImportDecl nodes. diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/11257/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/11257/input.js new file mode 100644 index 000000000000..9f80405f2fc2 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/11257/input.js @@ -0,0 +1,6 @@ +import { v1 } from 'a'; +import { v2 } from 'b'; +import { v3 } from 'b'; +import { v4 } from 'c'; + +console.log(v1, v2, v3, v4); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/11257/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/11257/output.js new file mode 100644 index 000000000000..0b62ab27e6c9 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/11257/output.js @@ -0,0 +1,4 @@ +import { v1 } from 'a'; +import { v2, v3 } from "b"; +import { v4 } from 'c'; +console.log(v1, v2, v3, v4); From d1841853147fbfae6a34c6a2f3cf2fbab4578197 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Mon, 10 Nov 2025 13:49:01 +0000 Subject: [PATCH 2/4] test(es/minifier): Update test output to reflect correct import ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update the expected output for test 11133 to match the corrected behavior from the merge_imports fix. The merged imports now correctly appear at the position of their first occurrence, preserving the original import order and keeping imports before their usage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../tests/fixture/issues/11133/output.js | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/11133/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/11133/output.js index d64658503a70..fa7de9a78f74 100644 --- a/crates/swc_ecma_minifier/tests/fixture/issues/11133/output.js +++ b/crates/swc_ecma_minifier/tests/fixture/issues/11133/output.js @@ -1,23 +1,23 @@ // Test case 1: Basic duplicate named imports +import { add, subtract, multiply } from "math"; +// Test case 2: Same export imported with different local names (should preserve both) +import { add as a, add as b } from "calculator"; +// Test case 3: Mix of default and named imports +import defaultExport, { namedExport } from "module1"; +// Test case 4: Namespace import with named imports (CANNOT be merged - incompatible) +import * as utils from "utils"; +import { helper } from "utils"; +// Test case 4b: Default with namespace (CAN be merged) +import defUtils, * as utils2 from "utils2"; // Test case 5: Side-effect import (should not be merged) import 'polyfill'; import 'polyfill'; // Test case 6: Different sources (should not be merged) import { foo } from 'lib1'; import { foo } from 'lib2'; -// Use all imports to avoid dead code elimination -console.log(add, subtract, multiply), console.log(a, b), console.log(defaultExport, namedExport), console.log(utils, helper), console.log(defUtils, utils2), console.log(foo), console.log(duplicate), console.log(thing, renamedThing, otherThing); -// Test case 4: Namespace import with named imports (CANNOT be merged - incompatible) -import * as utils from "utils"; -import { helper } from "utils"; -// Test case 8: Mix of named imports with and without aliases -import { thing, thing as renamedThing, otherThing } from "things"; -// Test case 2: Same export imported with different local names (should preserve both) -import { add as a, add as b } from "calculator"; -// Test case 4b: Default with namespace (CAN be merged) -import defUtils, * as utils2 from "utils2"; -// Test case 3: Mix of default and named imports -import defaultExport, { namedExport } from "module1"; -import { add, subtract, multiply } from "math"; // Test case 7: Duplicate named imports (exact same specifier) import { duplicate } from "dups"; +// Test case 8: Mix of named imports with and without aliases +import { thing, thing as renamedThing, otherThing } from "things"; +// Use all imports to avoid dead code elimination +console.log(add, subtract, multiply), console.log(a, b), console.log(defaultExport, namedExport), console.log(utils, helper), console.log(defUtils, utils2), console.log(foo), console.log(duplicate), console.log(thing, renamedThing, otherThing); From 3dba7284a64fff4d6a0fa61e0d7e7f7f843fdeea Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Mon, 10 Nov 2025 14:06:23 +0000 Subject: [PATCH 3/4] test(es/minifier): Update test expectations for correct import ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After fixing the merge_imports feature to preserve import order correctly, these test expectations needed to be updated. Imports now properly appear before exports and other statements, as they should. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../tsc-references/es6modulekindWithES5Target9.2.minified.js | 4 ++-- .../esnextmodulekindWithES5Target9.2.minified.js | 4 ++-- .../swc/tests/tsc-references/exportNamespace12.2.minified.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/swc/tests/tsc-references/es6modulekindWithES5Target9.2.minified.js b/crates/swc/tests/tsc-references/es6modulekindWithES5Target9.2.minified.js index 0718b59bb1b4..892c1c4500f7 100644 --- a/crates/swc/tests/tsc-references/es6modulekindWithES5Target9.2.minified.js +++ b/crates/swc/tests/tsc-references/es6modulekindWithES5Target9.2.minified.js @@ -1,7 +1,7 @@ //// [es6modulekindWithES5Target9.ts] +import d, { a } from "mod"; +import * as M from "mod"; export * from "mod"; export { b } from "mod"; export default d; -import d, { a } from "mod"; -import * as M from "mod"; export { a, M, d }; diff --git a/crates/swc/tests/tsc-references/esnextmodulekindWithES5Target9.2.minified.js b/crates/swc/tests/tsc-references/esnextmodulekindWithES5Target9.2.minified.js index cae21b6a9918..c58abcb045ef 100644 --- a/crates/swc/tests/tsc-references/esnextmodulekindWithES5Target9.2.minified.js +++ b/crates/swc/tests/tsc-references/esnextmodulekindWithES5Target9.2.minified.js @@ -1,7 +1,7 @@ //// [esnextmodulekindWithES5Target9.ts] +import d, { a } from "mod"; +import * as M from "mod"; export * from "mod"; export { b } from "mod"; export default d; -import d, { a } from "mod"; -import * as M from "mod"; export { a, M, d }; diff --git a/crates/swc/tests/tsc-references/exportNamespace12.2.minified.js b/crates/swc/tests/tsc-references/exportNamespace12.2.minified.js index 8bb0418b1cf9..ad1dc21b6c41 100644 --- a/crates/swc/tests/tsc-references/exportNamespace12.2.minified.js +++ b/crates/swc/tests/tsc-references/exportNamespace12.2.minified.js @@ -1,7 +1,7 @@ //// [main.ts] -console.log(c), console.log(types.c); import * as types from "./types"; import { c } from "./types"; +console.log(c), console.log(types.c); //// [types.ts] export { }; //// [values.ts] From 8906e54e642a739af381281a6b08cf29c55ff26a Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Tue, 11 Nov 2025 10:23:38 +0000 Subject: [PATCH 4/4] refactor(es/minifier): Address PR review feedback for import merging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace FxHashMap with FxHashSet for processed_indices tracking - Replace unwrap() with expect() with descriptive error message 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- crates/swc_ecma_minifier/src/pass/postcompress.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/swc_ecma_minifier/src/pass/postcompress.rs b/crates/swc_ecma_minifier/src/pass/postcompress.rs index fd59d69322a9..6c7e890733d4 100644 --- a/crates/swc_ecma_minifier/src/pass/postcompress.rs +++ b/crates/swc_ecma_minifier/src/pass/postcompress.rs @@ -1,4 +1,4 @@ -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use swc_common::{util::take::Take, DUMMY_SP}; use swc_ecma_ast::*; @@ -164,7 +164,7 @@ fn merge_imports_in_module(module: &mut Module) { // Remove all imports that will be merged (except side-effect imports) // and insert merged imports at the position of the first occurrence let mut new_body = Vec::new(); - let mut processed_indices = FxHashMap::default(); + let mut processed_indices = FxHashSet::default(); for (idx, item) in module.body.iter().enumerate() { if let ModuleItem::ModuleDecl(ModuleDecl::Import(import_decl)) = item { @@ -180,9 +180,12 @@ fn merge_imports_in_module(module: &mut Module) { if let Some((first_idx, decls)) = import_groups.get(&key) { if decls.len() > 1 { // This import needs to be merged - if idx == *first_idx && processed_indices.insert(*first_idx, ()).is_none() { + if idx == *first_idx && processed_indices.insert(*first_idx) { // This is the first occurrence - insert merged imports here - for merged in inserts_at.get(first_idx).unwrap() { + for merged in inserts_at.get(first_idx).expect( + "Invariant violated: first_idx should always be present in inserts_at \ + due to import group construction", + ) { new_body .push(ModuleItem::ModuleDecl(ModuleDecl::Import(merged.clone()))); }