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

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

Open
1 task
qtiki opened this issue May 1, 2022 · 5 comments
Open
1 task

Comments

@qtiki
Copy link

qtiki commented May 1, 2022

💻

  • Would you like to work on a fix?

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

  • Babel version v7.17.11

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
Copy link

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 qtiki changed the title [Bug]: @babel/traverse v7.17.10 broken this scope with dead code elimination plugin [Bug]: @babel/traverse v7.17.11 broken this scope with dead code elimination plugin May 1, 2022
@qtiki qtiki changed the title [Bug]: @babel/traverse v7.17.11 broken this scope with dead code elimination plugin [Bug]: @babel/traverse v7.17.10 broken this scope with dead code elimination plugin May 1, 2022
@qtiki
Copy link
Author

qtiki commented May 1, 2022

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 JLHwung transferred this issue from babel/babel May 2, 2022
@JLHwung
Copy link
Contributor

JLHwung commented May 2, 2022

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

if (!scope.isPure(replacement, true) && !isReferencedBefore) {
continue;
}

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
Copy link

Note that deadcode also suffers from #981, so I'd recommend simply disabling it (along with simplify for #999 and builtIns for #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
Copy link
Author

qtiki commented May 3, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants