Skip to content
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

Minifier should panic with a descriptive error message on JSX/TypeScript #9204

Closed
colinaaa opened this issue Jul 11, 2024 · 5 comments · Fixed by #9207 or #9271
Closed

Minifier should panic with a descriptive error message on JSX/TypeScript #9204

colinaaa opened this issue Jul 11, 2024 · 5 comments · Fixed by #9207 or #9271
Assignees
Labels
Milestone

Comments

@colinaaa
Copy link

Describe the bug

We are using the SWC minifier with Rust API. But the output is incorrect with JSX is preserved.

If a variable is only being used as a JSXElementName, it will be mistakenly removed by SWC minifier.

Input code

const Foo = createFoo();
export function App() {
    return (
        <view>
            <Foo />
        </view>
    );
}

Config

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": true
    },
    "target": "es2022",
    "loose": false,
    "minify": {
      "compress": {
        "arguments": true,
        "arrows": true,
        "booleans": true,
        "booleans_as_integers": true,
        "collapse_vars": true,
        "comparisons": true,
        "computed_props": true,
        "conditionals": true,
        "dead_code": false,
        "directives": true,
        "drop_console": true,
        "drop_debugger": true,
        "evaluate": true,
        "expression": true,
        "hoist_funs": true,
        "hoist_props": true,
        "hoist_vars": true,
        "if_return": true,
        "join_vars": false,
        "keep_classnames": true,
        "keep_fargs": true,
        "keep_fnames": false,
        "keep_infinity": true,
        "loops": true,
        "negate_iife": false,
        "properties": true,
        "reduce_funcs": true,
        "reduce_vars": false,
        "side_effects": true,
        "switches": true,
        "typeofs": true,
        "unsafe": false,
        "unsafe_arrows": false,
        "unsafe_comps": false,
        "unsafe_Function": false,
        "unsafe_math": false,
        "unsafe_symbols": false,
        "unsafe_methods": false,
        "unsafe_proto": false,
        "unsafe_regexp": false,
        "unsafe_undefined": false,
        "unused": true,
        "const_to_let": true,
        "pristine_globals": true
      },
      "mangle": false
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": false,
  "isModule": true
}

Playground link (or link to the minimal reproduction)

https://play.swc.rs/?version=1.6.7&code=H4sIAAAAAAAAA0vOzysuUXDLz1ewVUguSk0sSQWyNTStuVIrCvKLShTSSvOSSzLz8xQcCwo0NBWquRSAoCi1pLQoT0EDzAEBm7LM1HI7OBcsBDJTHyFmo49QAzS%2BlgsAf4CxUHoAAAA%3D&config=H4sIAAAAAAAAA32UO3LjMAyG%2B5zCozrFjostcoDt9gwcWgRlZilCQ4CONRnffaFnPDboTsIHEMQPEN9vh0PzSW3zcfiWT%2FkZbCbI%2B79YaExsr2JpeByA2hwGbt43yjQhzgVmy20BDdvcAU9BQMdfx%2BMa0EREAjF7GwlWWx9S8ON9yhb7IQPRnU2scmTpITGtCd%2FvUcYvxX5CjGDTC2IsmZAYOsiKV4sx2oHAXKyOe9ErEGoZJlgYnBkyDipPLnDAJFI8UwfWmRbdg1gLCxlaDhfQ4iSZxCWS8irUwal03dzkBwwXG4tlJRCuc0Pkts%2FsjIHY%2BKKJsLCKAgvUpQ3eZOCSlXyfGNIW9SjNPwApP1qiZHtNn9nByyhV2RaoHh2Sl2nl8TlYJlurMUEngpoQvNbJSRjIHLSbZnClhUnXtk4rMlBwYMB7GRMllr4Ct2ct5%2FTC0StAmmvVChZg9gdY4dNjeIH%2FSJG8DFfFo7d8rlMa%2BxPGFwl64DO6Fw7SCcY6zrIgrkOdl%2BRAJgOc6lJoBs8LQMaf0cR5Uz7QQfYKy4mmi3j6WRGrw21fwL1NXdw7syzht9Wh6dGVGa7bfervspR%2FNz9O2%2F7dL94E%2BrtFzklv%2FwFbOOlnKQYAAA%3D%3D

SWC Info output

No response

Expected behavior

The const Foo is preserved in the output:

const Foo = createFoo();
export function App() {
    return (
        <view>
            <Foo />
        </view>
    );
}

Actual behavior

The const Foo is removed. Result in Foo is not defined error at runtime.

createFoo();
export function App() {
    return <view>

            <Foo/>

        </view>;
}

Version

1.6.7

Additional context

No response

@colinaaa colinaaa added the C-bug label Jul 11, 2024
@kdy1 kdy1 self-assigned this Jul 11, 2024
@kdy1 kdy1 modified the milestone: Planned Jul 11, 2024
@kdy1 kdy1 removed their assignment Jul 11, 2024
@kdy1
Copy link
Member

kdy1 commented Jul 11, 2024

This is expected, but I think minifeir should panic instead

@kdy1 kdy1 added this to the Planned milestone Jul 11, 2024
@kdy1 kdy1 self-assigned this Jul 11, 2024
@kdy1 kdy1 changed the title swc minifier mistakenly removed variables used in JSXElementName Minifier should panic with a descriptive error message on JSX/TypeScript Jul 11, 2024
@colinaaa
Copy link
Author

I managed to fix this by adding a report_usage for JSXElementName to the UsageAnalyzer:

self.with_ctx(ctx).report_usage(i);

    #[cfg_attr(feature = "debug", tracing::instrument(skip_all))]
    fn visit_jsx_element_name(&mut self, n: &JSXElementName) {
        let ctx = Ctx {
            in_pat_of_var_decl: false,
            in_pat_of_param: false,
            in_catch_param: false,
            var_decl_kind_of_pat: None,
            in_pat_of_var_decl_with_init: false,
            ..self.ctx
        };

        n.visit_children_with(&mut *self.with_ctx(ctx));

        if let JSXElementName::Ident(i) = n {
            self.with_ctx(ctx).report_usage(i);
        }
    }

Would do you think about fixing it instead of panic?

@kdy1
Copy link
Member

kdy1 commented Jul 11, 2024

JSX is not a standard, so I think it should panic instead of handling them.

kdy1 added a commit that referenced this issue Jul 11, 2024
**Description:**

These macros are optimization hint on release builds.

**Related issue:**

 - Closes #9204
@kdy1 kdy1 modified the milestones: Planned, v1.7.0 Jul 17, 2024
@kdy1
Copy link
Member

kdy1 commented Jul 18, 2024

I found that we should support this due to Evaluator

@kdy1 kdy1 reopened this Jul 18, 2024
@kdy1 kdy1 modified the milestones: v1.7.0, Planned Jul 18, 2024
kdy1 added a commit that referenced this issue Jul 18, 2024
@kdy1 kdy1 modified the milestones: Planned, v1.7.1 Jul 23, 2024
@swc-bot
Copy link
Collaborator

swc-bot commented Aug 22, 2024

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 participants