feat(minifier): add invalid_import_side_effects option#17300
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #17300 will not alter performanceComparing Summary
Footnotes
|
494bd15 to
732dd69
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a new invalid_import_side_effects option to the minifier to control whether unused import removal optimization should be performed. The option allows users to disable the optimization introduced in #16797, as it requires additional assumptions about import behavior. When set to true (default), imports are preserved because accessing non-existing imports or unresolved import statements can throw errors. When set to false, unused imports can be safely removed.
Key Changes:
- Added
invalid_import_side_effectsoption with default value oftrueto preserve backward compatibility - Integrated the option into the import removal logic to gate the optimization
- Updated tests to explicitly set the option when testing import removal behavior
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_minifier/src/options.rs | Added invalid_import_side_effects field to TreeShakeOptions struct with default value of true |
| crates/oxc_minifier/src/peephole/remove_unused_declaration.rs | Integrated the new option into import removal logic; updated tests to explicitly disable the option when testing removal behavior |
| napi/minify/src/options.rs | Added NAPI binding for the new option with proper type conversion |
| napi/minify/index.d.ts | Added TypeScript definition for invalidImportSideEffects option |
| tasks/track_memory_allocations/allocs_minifier.snap | Updated memory allocation snapshot reflecting minimal overhead from the new option check |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge activity
|
Added a new option (`invalid_import_side_effects`) to disable #16797 as it requires a new assumptions. refs rolldown/rolldown#7512 (comment)
442fb1c to
8e4409a
Compare
) These sideeffects are commonly ignored, so I think we should ignore them by default. - [Rollup REPL](https://rollupjs.org/repl/?version=4.57.1&shareable=eyJleGFtcGxlIjpudWxsLCJtb2R1bGVzIjpbeyJjb2RlIjoiaW1wb3J0IHsgZm9vIH0gZnJvbSAndW5rbm93bidcbmlmIChmYWxzZSkge1xuICBjb25zb2xlLmxvZyhmb28pXG59IiwiaXNFbnRyeSI6dHJ1ZSwibmFtZSI6Im1haW4uanMifV0sIm9wdGlvbnMiOnt9fQ==) - [esbuild try](https://esbuild.github.io/try/#dAAwLjI3LjIALS1taW5pZnkAaW1wb3J0IHsgZm9vIH0gZnJvbSAndW5rbm93bicKaWYgKGZhbHNlKSB7CiAgY29uc29sZS5sb2coZm9vKQp9) - [SWC playground](https://play.swc.rs/?version=1.15.11&code=H4sIAAAAAAAAAw3LMQ6AIBAEwP6S%2B8N2YOOfjOEMEW4NaCwIf9fpJ9eL7caAkZiwxorw%2BOl8PahkQ7St9LRgqAA7vbOktfCI%2F1hUpsoHV0iLrEQAAAA%3D&config=H4sIAAAAAAAAA41VS47bMAzd5xSB111MB0VR9ADd9QyCYlGOMrJoiFQ%2BGOTupRU7k5nQRjdBzKdHiuQj9b7ZbpsDtc3v7bv8lY%2FBZoJ8%2FxYLXRLbs1gaaHtLbQ4DN99m9EAj5G0kqKbrDWnY5g64suj15fXHxGgiIsHMmGx9SMFfHmO22A8ZiB5sYhWXpYfE9Jk%2FYRlPI8C5PNp3iBFsWkGMJRMSQwdZc9xijHYgMEebFS%2FjTW0OhFqIESwMzgwZBxVPLnDAJDGfUQfWmRYdKFDI0HI4gkaTWEJLJOkp%2BVTYwa50Xe3zFzYcbSyWlZhwri2R2ype9xiIjS9JK%2BENXKjBDZyK%2B5UZvMnAJadn3gFDWujJG4BUIFqiZHvQ%2FNYTXvS0xParzJC8SJYvCi761rJM0ElRTQheqexYGcgctG5mcKWFsbKtdp0JXigfBQcGvBetKK7pFLjda0H5MgB6BZD%2BWq%2Bp6gaY%2BxQu4ONArMB%2FJEvWBTad6C3vl1G69DuMKwF64D26lQPSCsZlOMuWOA%2FLeEkORBrg1COFKvC8BGQAGE2s%2B%2FJJGzIe4tF0EXcfa2I6cL3v4d6mrs77w75kHCIcIS7J%2BD9GZBWVux1HVc%2BT%2FTS98EsTpZXBC99fPj0bkspm%2Fq1JNT268pBQ1eTtOfnZfByaX465ak2gvzNRKv6W8JSazfUfly9ihugGAAA%3D) This was originally set to `true` (#17300) for Rolldown (rolldown/rolldown#7512 (comment)), but that should be instead handled on Rolldown side (rolldown/rolldown#8194).
Added a new option (
invalid_import_side_effects) to disable #16797 as it requires a new assumptions.refs rolldown/rolldown#7512 (comment)