-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(es/minifier): Fix merge_imports to preserve import order #11258
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
9c03e47
d184185
3dba728
8906e54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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::*; | ||
|
|
||
|
|
@@ -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<ImportKey, Vec<ImportDecl>> = FxHashMap::default(); | ||
| // Group imports by source and metadata, also track the first occurrence index | ||
| let mut import_groups: FxHashMap<ImportKey, (usize, Vec<ImportDecl>)> = 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,65 @@ 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<usize, Vec<ImportDecl>> = 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 = FxHashSet::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) { | ||
| // This is the first occurrence - insert merged imports here | ||
| 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()))); | ||
| } | ||
| } | ||
| // 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()); | ||
| } | ||
| } | ||
|
Comment on lines
+169
to
204
|
||
|
|
||
| module.body = new_body; | ||
| } | ||
|
|
||
| /// Merge multiple ImportDecl nodes. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| import { v1 } from 'a'; | ||
| import { v2, v3 } from "b"; | ||
| import { v4 } from 'c'; | ||
| console.log(v1, v2, v3, v4); |
Uh oh!
There was an error while loading. Please reload this page.