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

Update uglify #453

Merged
merged 2 commits into from
Mar 1, 2017
Merged

Update uglify #453

merged 2 commits into from
Mar 1, 2017

Conversation

XhmikosR
Copy link
Member

/CC @ @alexlamsl: are there any breaking changes in 2.8.0 we should take care of? So far I see Windows builds fail due to WARN: Output exceeds 40 characters.

@XhmikosR XhmikosR requested a review from vladikoff February 28, 2017 14:04
@alexlamsl
Copy link
Contributor

@XhmikosR that warning is new but harmless - it merely states the fact that the despite specifying output = {max_line_len: 40} it was unable to quite fit the line into that stated limit.

I guess a test in this project has set that specifically.

@XhmikosR
Copy link
Member Author

I'll see how to work around this then.

Are there any breaking changes though?

@alexlamsl
Copy link
Contributor

global_defs now applies to globally declared variables - but this option does nothing by default.

Not sure if these counts as well, but collapse_vars and reduce_vars are now enabled by default.

@XhmikosR
Copy link
Member Author

OK, thanks. Those shouldn't affect us.

@alexlamsl
Copy link
Contributor

@XhmikosR please consider applying the one-liner from mishoo/UglifyJS#1516 (comment)

@XhmikosR
Copy link
Member Author

XhmikosR commented Mar 1, 2017

@alexlamsl; you mean in this branch? Note that I have no idea why they say there they get 2.8.0 since we use a tilde for uglify so it shouldn't get that.

@alexlamsl
Copy link
Contributor

@XhmikosR I mumbled about that in mishoo/UglifyJS#1519 (comment) as well.

Anyway, this new method is introduced back in uglify-js 2.6.3, and without it passes (& now reduce_vars) cannot be used, i.e. they will be no-op.

@XhmikosR
Copy link
Member Author

XhmikosR commented Mar 1, 2017

@alexlamsl: so I apply that one line patch and I get different output. Is that normal?

I pushed the patches in this branch for you to have a quick look.

@alexlamsl
Copy link
Contributor

@XhmikosR the minified .js looks correct to me (I don't have enough knowledge on source maps)

Thanks!

@XhmikosR
Copy link
Member Author

XhmikosR commented Mar 1, 2017

I'm gonna merge this and release the new version in a few hours.

@kzc
Copy link

kzc commented Mar 1, 2017

2.8.3 will be released shortly:

mishoo/UglifyJS#1523

@XhmikosR
Copy link
Member Author

XhmikosR commented Mar 1, 2017

@kzc: although it shouldn't matter due to us using tilde for uglify version, I can wait for 2.8.3, rebase and then release the new version.

@kzc
Copy link

kzc commented Mar 1, 2017

@XhmikosR True, but that uglify-js@2.8.2 error is fairly likely to occur in the wild.

@alexlamsl
Copy link
Contributor

alexlamsl commented Mar 1, 2017

uglify-js 2.8.3 released.

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

Successfully merging this pull request may close these issues.

3 participants