feat: add preserve_imports_with_side_effects option for simplify optimization #6962

nissy-dev · 2023-02-19T08:58:47Z

Description: Fix #6771

This implementation is still rough, and there are still some implementation decisions that need to be decided on that I mentioned in my comments.

BREAKING CHANGE: Yes

Related issue (if exists):

nissy-dev · 2023-02-19T09:01:25Z

crates/swc/src/config/mod.rs

+
+    #[serde(default)]
+    pub drop_unused_imports: BoolConfig<false>,


There was a way to modify the simplify option directly, but I added it in this way to avoid breaking change. What do you think?

Can you move this to the simplify option?

crates/swc_bundler/src/bundler/optimize.rs

crates/swc_ecma_transforms_optimization/src/simplify/dce/mod.rs

kdy1 · 2023-02-22T10:57:10Z

I'll merge PRs with breaking changes at once

kdy1

Thank you!
As it's a breaking change anyway, we can use correct way.

kdy1 · 2023-02-23T04:40:34Z

crates/swc/src/config/mod.rs

+
+    #[serde(default)]
+    pub drop_unused_imports: BoolConfig<false>,


Can you move this to the simplify option?

manucorporat · 2023-02-23T14:42:21Z

I am thinking that it would be soo good, to have a callback to resolve if a indentifier has side effects or not:

So instead of being a bool it could be a function, that is being passed the import source and return a bool

kdy1 · 2023-02-23T14:50:49Z

Nope, it should never be

kdy1 · 2023-02-23T14:51:02Z

Rust-side api is fine, but js api is not fine

nissy-dev · 2023-02-23T15:11:53Z

@manucorporat Which API do you use, JS API or Rust API?
It is possible to change the simplifier function (in crates/swc_ecma_transforms_optimization/src/simplify/mod.rs) to accept callbacks. If you want to do so, it would be great if you could give me more details about the callback you want to add.

kdy1 · 2023-02-28T03:37:40Z

crates/swc/tests/fixture/globals/env-map-computed/input/.swcrc

@@ -2,7 +2,6 @@
    "jsc": {
        "transform": {
            "optimizer": {
-                "simplify": false,


Why did you touch this? Please revert

@kdy1 Thank you for your reviews. You mean I need to modify simplify option which receive boolean value and json? Like this.

pub struct OptimizerConfig { #[serde(default)] pub simplify: SimplifyOption, } #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] pub enum SimplifyOption { Bool(bool), Json(SimplifyJsonOption), } impl Default for SimplifyOption { fn default() -> Self { SimplifyOption::Bool(true) } } #[derive(Debug, Default, Clone, Copy, Serialize, Deserialize)] #[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct SimplifyJsonOption { pub preserve_imports_with_side_effects: bool, }

If you think so, I will refactor in this way

Yes, thank you!

This reverts commit cb6f660.

kdy1 · 2023-03-03T02:00:53Z

crates/swc_ecma_transforms_optimization/src/simplify/dce/mod.rs

@@ -30,6 +30,7 @@ use crate::debug_assert_valid;
 pub fn dce(
    config: Config,
    unresolved_mark: Mark,
+    preserve_imports_with_side_effects: bool,


I think this should go into Config type

I moved this option to dce config.

kdy1

Thank you!

swc-bump:

swc_ecma_transforms_optimization --breaking

swc-bot

Automated review comment generated by auto-rebase script

feat: add drop_unused_imports option for simplify optimization

238baf9

nissy-dev commented Feb 19, 2023

View reviewed changes

crates/swc_bundler/src/bundler/optimize.rs Outdated Show resolved Hide resolved

kdy1 reviewed Feb 19, 2023

View reviewed changes

crates/swc_ecma_transforms_optimization/src/simplify/dce/mod.rs Outdated Show resolved Hide resolved

kdy1 added this to the Planned milestone Feb 19, 2023

magic-akari mentioned this pull request Feb 20, 2023

fix(es/minifier): Drop unused import bindings #6967

Merged

nissy-dev closed this Feb 21, 2023

nissy-dev reopened this Feb 21, 2023

Merge branch 'main' into fix-6771

6d0ae8e

kdy1 requested changes Feb 23, 2023

View reviewed changes

nissy-dev added 2 commits February 23, 2023 22:18

fix: refactor option

ddba656

fix: rename option

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

5ee9469

nissy-dev changed the title ~~feat: add drop_unused_imports option for simplify optimization~~ feat: add preserve_imports_with_side_effects option for simplify optimization Feb 23, 2023

fix: update option name

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

6114dce

nissy-dev added 2 commits February 24, 2023 00:15

fix: update tests

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

ea95e08

fix: update node test

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

cb6f660

kdy1 reviewed Feb 28, 2023

View reviewed changes

kdy1 marked this pull request as draft February 28, 2023 03:37

nissy-dev added 5 commits March 2, 2023 01:53

fix: revert test config

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

d52f486

Revert "fix: update node test"

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

7c24306

This reverts commit cb6f660.

feat: refactor simplify option

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

d296f8b

feat: refactor default function

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

65861d8

fix: ci error

eaeea1a

nissy-dev marked this pull request as ready for review March 2, 2023 15:05

kdy1 requested changes Mar 3, 2023

View reviewed changes

kdy1 self-assigned this Mar 3, 2023

fix: move option to Config

c89bd09

kdy1 approved these changes Mar 4, 2023

View reviewed changes

kdy1 enabled auto-merge (squash) March 4, 2023 12:52

Merge branch 'main' into fix-6771

35c138f

swc-bot reviewed Mar 4, 2023

View reviewed changes

kdy1 merged commit 67d0a89 into swc-project:main Mar 5, 2023

kdy1 modified the milestones: Planned, v1.3.38 Mar 6, 2023

swc-project locked as resolved and limited conversation to collaborators Apr 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

feat: add preserve_imports_with_side_effects option for simplify optimization #6962

feat: add preserve_imports_with_side_effects option for simplify optimization #6962

nissy-dev commented Feb 19, 2023 •

edited

Loading

nissy-dev Feb 19, 2023

kdy1 Feb 23, 2023 •

edited

Loading

kdy1 commented Feb 22, 2023

kdy1 left a comment

kdy1 Feb 23, 2023 •

edited

Loading

manucorporat commented Feb 23, 2023

kdy1 commented Feb 23, 2023

kdy1 commented Feb 23, 2023

nissy-dev commented Feb 23, 2023 •

edited

Loading

kdy1 Feb 28, 2023

nissy-dev Feb 28, 2023

kdy1 Feb 28, 2023

nissy-dev Mar 2, 2023

kdy1 Mar 3, 2023

nissy-dev Mar 4, 2023

kdy1 left a comment

swc-bot left a comment


		#[serde(default)]
		pub drop_unused_imports: BoolConfig<false>,

feat: add preserve_imports_with_side_effects option for simplify optimization #6962

feat: add preserve_imports_with_side_effects option for simplify optimization #6962

Conversation

nissy-dev commented Feb 19, 2023 • edited Loading

nissy-dev Feb 19, 2023

Choose a reason for hiding this comment

kdy1 Feb 23, 2023 • edited Loading

Choose a reason for hiding this comment

kdy1 commented Feb 22, 2023

kdy1 left a comment

Choose a reason for hiding this comment

kdy1 Feb 23, 2023 • edited Loading

Choose a reason for hiding this comment

manucorporat commented Feb 23, 2023

kdy1 commented Feb 23, 2023

kdy1 commented Feb 23, 2023

nissy-dev commented Feb 23, 2023 • edited Loading

kdy1 Feb 28, 2023

Choose a reason for hiding this comment

nissy-dev Feb 28, 2023

Choose a reason for hiding this comment

kdy1 Feb 28, 2023

Choose a reason for hiding this comment

nissy-dev Mar 2, 2023

Choose a reason for hiding this comment

kdy1 Mar 3, 2023

Choose a reason for hiding this comment

nissy-dev Mar 4, 2023

Choose a reason for hiding this comment

kdy1 left a comment

Choose a reason for hiding this comment

swc-bot left a comment

Choose a reason for hiding this comment

nissy-dev commented Feb 19, 2023 •

edited

Loading

kdy1 Feb 23, 2023 •

edited

Loading

kdy1 Feb 23, 2023 •

edited

Loading

nissy-dev commented Feb 23, 2023 •

edited

Loading