search

babel / minify

:scissors: An ES6+ aware minifier based on the Babel toolchain (beta)
https://babeljs.io/repl
MIT License
4.39k stars 225 forks source link

[Bug]: @babel/traverse v7.17.10 broken `this` scope with dead code elimination plugin #1028

Open qtiki opened 2 years ago

qtiki commented 2 years ago

💻

How are you using Babel?

@babel/cli

Input code

Reproduced in Babel REPL.

Configuration file name

No response

Configuration

No response

Current and expected behavior

Enabling babel-plugin-minify-dead-code-elimination plugin destroys this scope for arrow functions inside classes.

The var _this = this workaround should not be eliminated as it is absolutely necessary for the this scope to work in locally defined arrow functions.

Environment

Possible solution

No response

Additional context

I narrowed the problem down locally to @babel/traverse v7.17.10. By reverting that package to v7.17.9 this problem does not occur.

babel-bot commented 2 years ago

Hey @qtiki! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

qtiki commented 2 years ago

It seems that @babel/traverse v7.17.10 has only this one commit which is probably the cause for this - or then the minify plugin needs to be updated somehow to accommodate for these changes (I'm not a Babel developer so I have no clue). FYI @JLHwung

JLHwung commented 2 years ago

Thanks for the report. This issue should be fixed in the minify plugin.

https://github.com/babel/minify/blob/1ad7838116ec34621d39bb1b4a985e7601eab659/packages/babel-plugin-minify-dead-code-elimination/src/index.js#L357-L359

Before v7.17.10 Babel does not report this as a pure expression, so dce will not try to remove _this = this. However, after 7.17.0 we should check whether the original non-arrow scope is same with the replacement non-arrow scope. If they are same, we can safely replace _this by this, otherwise we should bail.

ehoogeveen-medweb commented 2 years ago

Note that deadcode also suffers from https://github.com/babel/minify/issues/981, so I'd recommend simply disabling it (along with simplify for https://github.com/babel/minify/issues/999 and builtIns for https://github.com/babel/minify/issues/904 if you run into it).

Unfortunately this project appears to be unmaintained (the last commit was in August of 2020), so looking into other minifiers might also not be a bad idea.

qtiki commented 2 years ago

Unfortunately this project appears to be unmaintained (the last commit was in August of 2020), so looking into other minifiers might also not be a bad idea.

This is quite bad. We have a build pipeline that relies on minify-replace and minify-dead-code-elimination working in tandem to have different compile targets. It's quite critical to us so just disabling deadcode elimination is not an option. I'll look into other options but basically this means that we'll need to stick to an older Babel version until we can work out a replacement.