swc-project / swc

Rust-based platform for the Web
https://swc.rs
Apache License 2.0
30.96k stars 1.21k forks source link

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

Closed colinaaa closed 2 months ago

colinaaa commented 2 months ago

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

kdy1 commented 2 months ago

This is expected, but I think minifeir should panic instead

colinaaa commented 2 months ago

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

https://github.com/swc-project/swc/blob/da9844b8d56c65f5a0176e3c720e4435694f5756/crates/swc_ecma_usage_analyzer/src/analyzer/mod.rs#L697

    #[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 commented 2 months ago

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

kdy1 commented 2 months ago

I found that we should support this due to Evaluator

swc-bot commented 1 month ago

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.