-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(es/minifier): Add merge_imports optimization pass to reduce bundle size #11151
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
Conversation
Implements a new optimization pass that merges duplicate import statements from the same module source, reducing bundle size by eliminating redundant imports. Fixes #11133 ## Implementation - Added `merge_imports.rs` pass that groups imports by source and merges compatible ones - Handles all valid ES module import combinations: - Multiple named imports → merged into single import - Default + named imports → merged - Default + namespace imports → merged - Namespace + named imports (without default) → kept separate (no valid syntax) - Preserves import attributes/assertions (`with` clause) - Preserves type-only imports separately - Preserves side-effect imports (no specifiers) - Deduplicates exact duplicate imports - Preserves different aliases for the same export ## Configuration - Added `merge_imports: bool` option to `CompressOptions` (default: `true`) - Integrated into minifier pipeline before `merge_exports` ## Testing - Added comprehensive test fixtures in `tests/fixture/issues/11133/` - Tests cover: - Basic duplicate named imports - Same export with different local names (aliases) - Default + named imports - Default + namespace imports - Namespace + named imports (incompatible, not merged) - Side-effect imports (preserved) - Different sources (not merged) - Exact duplicates (deduplicated) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: a68df99 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
|
|
|
🤖 This pull request has been linked to AutoDev Task #771 View the task details and manage the automated development workflow in AutoDev. |
📋 AutoDev Task PromptObjectiveImplement a new optimization pass in the SWC ECMAScript minifier to merge duplicate import statements from the same module source, reducing bundle size by eliminating redundant imports. BackgroundGitHub Issue: #11133 Currently, when multiple source files import from the same module, the minified output contains duplicate import statements with different aliases, unnecessarily bloating the bundle size. Example of current behavior: // Input
import { add } from 'math-library';
import { add } from 'math-library';
// Current output (undesired)
import { add as a } from 'math-library';
import { add as b } from 'math-library';
// Expected output (desired)
import { add } from 'math-library';Technical ContextSWC Architecture Documentation
Key SWC Structures (from codebase analysis)ImportDecl Structure (swc_ecma_ast/src/module_decl.rs:116-137): pub struct ImportDecl {
pub span: Span,
pub specifiers: Vec<ImportSpecifier>,
pub src: Box<Str>, // The module source path
pub type_only: bool,
pub with: Option<Box<ObjectLit>>,
pub phase: ImportPhase,
}ImportSpecifier Enum (swc_ecma_ast/src/module_decl.rs:265-297): pub enum ImportSpecifier {
Named(ImportNamedSpecifier), // import { foo } or import { foo as bar }
Default(ImportDefaultSpecifier), // import foo
Namespace(ImportStarAsSpecifier), // import * as foo
}ImportNamedSpecifier (swc_ecma_ast/src/module_decl.rs:322-336): pub struct ImportNamedSpecifier {
pub span: Span,
pub local: Ident, // Local binding name
pub imported: Option<ModuleExportName>, // Original export name (if aliased)
pub is_type_only: bool,
}Existing Similar PatternThe codebase already has a // merge_exports.rs groups exports from the same source
pub(crate) fn merge_exports() -> impl VisitMut {
Merger::default()
}Implementation ScopeFiles to Create/Modify
Algorithm DesignThe merge_imports pass should:
Naming Conflict Resolution Strategy// Example conflict scenarios to handle:
// Case 1: Same named import, same local name -> merge (easy case)
import { add } from 'math';
import { add } from 'math';
// Result: import { add } from 'math';
// Case 2: Same named import, different local names -> keep separate bindings
import { add as a } from 'math';
import { add as b } from 'math';
// Result: import { add as a, add as b } from 'math';
// Case 3: Mix of default and named imports
import foo from 'math';
import { add } from 'math';
// Result: import foo, { add } from 'math';
// Case 4: Namespace import with named imports -> keep separate
import * as math from 'math';
import { add } from 'math';
// Result: import * as math, { add } from 'math';
// Case 5: Type-only imports (TypeScript)
import type { Foo } from 'types';
import { Bar } from 'types';
// Result: Keep separate (type_only flag differs)Implementation ReferencesRust VisitMut Pattern (from merge_exports.rs): use swc_ecma_visit::{noop_visit_mut_type, VisitMut, VisitMutWith};
struct ImportMerger {
// Store grouped imports: HashMap<module_source, Vec<ImportDecl>>
imports_by_source: FxHashMap<String, Vec<ImportDecl>>,
}
impl VisitMut for ImportMerger {
noop_visit_mut_type!(fail);
fn visit_mut_module_items(&mut self, items: &mut Vec<ModuleItem>) {
// 1. Collect and group imports
// 2. Merge duplicates
// 3. Replace in items vector
}
}Rust HashMap Grouping (references from web search):
Testing StrategyCreate comprehensive test fixtures following SWC conventions:
Testing CommandsFrom
Documentation: https://swc.rs/docs/contributing/es-minifier/profiling Success Criteria
Additional Resources
Notes
This comment was automatically added by AutoDev. You can disable this feature in AutoDev Settings. |
|
Claude finished @DeLino[bot]'s task —— View job Code Review CompleteI'll review this PR implementation of the merge_imports optimization pass. Let me analyze the code quality, potential issues, and test coverage. Review Tasks:
Code Review SummaryThis PR implements a solid merge_imports optimization pass with good attention to ES module syntax constraints. The implementation correctly addresses the reviewer's feedback and follows SWC patterns well. ✅ StrengthsArchitecture & Design:
ES Module Compliance:
Configuration:
|
kdy1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not use a new visitor. Add a method to the Optimizer instead
CodSpeed Performance ReportMerging #11151 will not alter performanceComparing Summary
Footnotes |
Move merge_imports logic from a standalone visitor into the postcompress_optimizer function as a method, following the feedback from PR review. This change: - Removes the standalone merge_imports visitor - Adds merge_imports_in_module function to postcompress.rs - Integrates the logic directly into the postcompress optimization flow - Maintains all existing functionality and test compatibility Addresses PR review feedback: #11151 (review) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
kdy1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cargo fmt
Address PR review feedback to run cargo fmt on the codebase. Changes: - Restructure use statements to use nested imports - Remove extraneous blank line in merge_imports_in_module 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
kdy1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix CI
- Use inline format args for better readability - Replace or_insert_with(Vec::new) with or_default() - Use Entry API instead of contains_key + insert pattern These changes address clippy warnings that were causing CI failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
kdy1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run UPDATE=1 cargo test -p swc --test projects --test tsc
…amed correctly The previous implementation incorrectly tried to merge default, namespace, and named imports into a single import declaration, which is not valid ES module syntax and caused a panic in the code generator. This fix ensures that when all three import types are present, they are properly split into separate import declarations: - default + named imports in one declaration - namespace import in a separate declaration This follows the ES module specification which only allows: - default + named - default + namespace (only these two, no named) - namespace alone - named alone Addresses review feedback from PR #11151 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Implements a new optimization pass in the SWC ECMAScript minifier to merge duplicate import statements from the same module source, reducing bundle size by eliminating redundant imports.
Fixes #11133
Changes
Core Implementation
crates/swc_ecma_minifier/src/pass/merge_imports.rsIntegration
crates/swc_ecma_minifier/src/pass/mod.rs- Added merge_imports modulecrates/swc_ecma_minifier/src/lib.rs- Integrated pass into minifier pipeline (runs before merge_exports)crates/swc_ecma_minifier/src/option/mod.rs- Addedmerge_imports: boolconfig option (default: true)crates/swc_ecma_minifier/src/option/terser.rs- Added terser compatibilityTesting
crates/swc_ecma_minifier/tests/fixture/issues/11133/cargo test --test compress -- 11133Example
Before
After
Test Plan
Configuration
Users can control this optimization via the
compress.merge_importsoption in.swcrc:{ "jsc": { "minify": { "compress": { "merge_imports": true } } } }🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com