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

Regression: if without braces, followed by const or let immediately after the semicolon, produces broken code #132

Closed
chris-morgan opened this issue May 9, 2018 · 4 comments
Labels

Comments

@chris-morgan
Copy link
Contributor

chris-morgan commented May 9, 2018

This regressed between 1.16.0 and 1.17.0, in 40baf2e.

Minimal test case:

if(0);const e=0

This produces:

if(0){ ;var e=0

Before 40baf2e, it produced this:

if(0){ ; }var e=0

Any whitespace after the semicolon, and it compiles correctly; it’s only if the semicolon is immediately followed by let or const that it breaks.

for and while also exhibit this bug.

I feed some minified code through bublé; I hit this when upgrading from 1.15.2 to the latest and it all stopped working.

@chris-morgan chris-morgan changed the title Regression: if without braces, followed immediately by const or let on the same line, produces broken code Regression: if without braces, followed by const or let immediately after the semicolon, produces broken code May 9, 2018
@chris-morgan
Copy link
Contributor Author

Looking at this commit this regressed in, it’s clearly related to upgrading magic-string from 0.14.0 to 0.22.4. It seems to me more likely that this is a bug in magic-string than buble itself, but I’m sufficiently uncertain about it all that this is the best place to file and track it.

@aleclarson
Copy link
Contributor

Why do you minify before Bublé?

@chris-morgan
Copy link
Contributor Author

@aleclarson Not that it particularly matters—this being a rather severe regression on legitimate code—but there are two main reasons why minified code may be fed through Bublé. I hit both of them.

  1. The more likely one: you’re feeding all your code through Bublé, and you happen to have some third-party minified code in there alongside it.

  2. For historical reasons of dead code removal: UglifyJS can remove dead code in the presence of declared globals and local constants. This latter used to require you to use const or a /* @const */ comment on var, though some time in the last year it was extended to be able to cope with unadorned var (hence, historical reasons). Therefore, I fed it through UglifyJS to remove dead code, then Bublé to transform all the things (including let and const to var), then UglifyJS again to minify the transformed code.

@adrianheine
Copy link
Member

Thanks for your report, I could easily reproduce the issue.

$ echo 'if(0);const e=0' | bin/buble 
if(0){ ;var e=0

I obviously suspected the const transform to be the issue, so I deactivated it, and got valid output:

$ echo 'if(0);const e=0' | bin/buble -n letConst
if(0){ ; }const e=0

So I checked src/program/types/VariableDeclaration.js and found code.overwrite in line 17 which in such cases usually just need a contentOnly option set. The test suite still passes.

aleclarson pushed a commit to aleclarson/buble that referenced this issue Jul 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants