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

[ES6] fix block elimination #2023

Merged
merged 1 commit into from May 30, 2017
Merged

[ES6] fix block elimination #2023

merged 1 commit into from May 30, 2017

Conversation

alexlamsl
Copy link
Collaborator

fixes #1664
fixes #1672

@alexlamsl alexlamsl changed the title fix block elimination [ES6] fix block elimination May 30, 2017
@kzc
Copy link
Contributor

kzc commented May 30, 2017

@alexlamsl Thanks for sorting out block scoping. This is probably the last big ES6 show stopper. uglify-es has really come together over the last few weeks. It's quite usable now.

Have you ever tried running test/ufuzz.js on harmony? It should handle ES5 as well as master I imagine.

@alexlamsl
Copy link
Collaborator Author

uglify-es seems to help with debugging and improving master as well, so taking care of the low hanging fruits to make it somewhat usable is a logical step.

I haven't tried running test/ufuzz.js or test/mozilla-ast.js on harmony at all, but I guess they can be avenues to progressively resolve and support ES6 constructs.

@kzc
Copy link
Contributor

kzc commented May 30, 2017

I haven't tried running test/ufuzz.js or test/mozilla-ast.js on harmony at all, but I guess they can be avenues to progressively resolve and support ES6 constructs.

I was just interested if there were any significant ES5 regressions on harmony that only a fuzzer would bring to light.

A few projects are skipping uglify-js@3 and jumping directly to uglify-es@3.

@alexlamsl
Copy link
Collaborator Author

A few projects are skipping uglify-js@3 and jumping directly to uglify-es@3.

I noticed that, though my primary focus is on master so they will have to rely on proper issue reports and/or their own contributions to get the ball rolling.

@alexlamsl alexlamsl merged commit 0cc6ded into mishoo:harmony May 30, 2017
@alexlamsl alexlamsl deleted the issue-1664 branch May 30, 2017 06:59
@alexlamsl
Copy link
Collaborator Author

OT: here are the top 10 dependents to uglify-js, sorted by downloads in May 2017

package downloads
handlebars 8,556,894
html-minifier 3,893,351
gulp-uglify 1,519,334
grunt-contrib-uglify 963,780
accord 616,609
broccoli-uglify-sourcemap 223,050
cwise 168,447
jspm 157,130
grunt-inline 45,216
harp-minify 40,242

@kzc
Copy link
Contributor

kzc commented Jun 2, 2017

These are monthly stats? uglify-js reportedly had 927K downloads today, and 19.6M for the month.

@alexlamsl I see you're there with html-minifier in the #2 spot. But I'm surprised that webpack is not in the list - it cites uglify-js as a dependency and had 306K downloads today.

@alexlamsl
Copy link
Collaborator Author

These are monthly stats?

Yes.

But I'm surprised that webpack is not in the list.

That makes two of us.

handlebars is on uglify-js 2.6 though.

@kzc
Copy link
Contributor

kzc commented Jun 2, 2017

Did npm provide the list?

@alexlamsl
Copy link
Collaborator Author

Did npm provide the list?

Indirectly: https://www.npmjs.com/package/npm-get-top-dependents

@kzc
Copy link
Contributor

kzc commented Jun 2, 2017

This list appears to be sorted by downloads:
https://www.npmjs.com/browse/depended/uglify-js

@alexlamsl
Copy link
Collaborator Author

Hmm, I guess that module has a bug or two then. Oh well.

@kzc
Copy link
Contributor

kzc commented Jun 2, 2017

uglify-es is chugging along - 1295 downloads today.

@alexlamsl
Copy link
Collaborator Author

In that case - jade seems to have renamed to pug for at least two years now, yet the former still have 3M whereas the latter is only approaching 1M.

@alexlamsl
Copy link
Collaborator Author

uglify-es is chugging along - 1295 downloads today.

Yes, I'm monitoring that for #2023 (comment)

@alexlamsl
Copy link
Collaborator Author

To be honest, I used to value usage statistics as a way to gauge stability of a codebase. But in recent years I find it to be increasingly unreliable, particularly when it comes to security vulnerability.

@kzc
Copy link
Contributor

kzc commented Jun 2, 2017

Downloads stats are useful in deciding which of several potential functionally equivalent packages to choose.

@alexlamsl
Copy link
Collaborator Author

Downloads stats are useful in deciding which of several potential functionally equivalent packages to choose.

That helped me back when I migrate from yargs to commander for html-minifier.

@alexlamsl
Copy link
Collaborator Author

handlebars is on uglify-js 2.6 though.

Scrap that - ^2.6 means they are using 2.8.27

@kzc
Copy link
Contributor

kzc commented Jun 2, 2017

There was a third-party package version visualizer on the web that I can no longer find. It showed the breakdown of downloads per package version.

@alexlamsl
Copy link
Collaborator Author

I'd be surprised if there is a breakdown by versions, as I don't see it mentioned here:

https://github.com/npm/registry/blob/master/docs/download-counts.md

@kzc
Copy link
Contributor

kzc commented Jun 2, 2017

I don't know if the version counts were real - it could have been a random number generator for all I knew. But I recall that the version downloads they displayed skewed to older versions.

@alexlamsl
Copy link
Collaborator Author

it could have been a random number generator for all I knew.

You may (not) be surprised how often I think that when I see a table of numbers.

@kzc
Copy link
Contributor

kzc commented Aug 19, 2017

OT - just noticed that the uglify-es npm download count is around 7,000 per weekday - with relatively few recent bug reports. Either it's fairly stable or it's mostly being used on ES5 code.

Is the intention to leave uglify-js at ES5 permanently and let people migrate to uglify-es over time? Or is there some uglify-es download threshold where harmony will be merged into master for uglify-js@4?

@alexlamsl
Copy link
Collaborator Author

@kzc I think keeping it separate like this for the forseeable future isn't bad, considering uglify-js has better stability (may converge in due course) and performance (may remain forever so due to ES6+ complexity) than uglify-es.

I'd also prefer ES6+ users to fix and improve uglify-es themselves.

@kzc
Copy link
Contributor

kzc commented Aug 19, 2017

The 50% harmony performance penalty will not likely ever be remedied due to ES6+ complexity. But eventually ES5-only use will drop so the switchover to ES6 will be unavoidable.

As far as uglify-es stability goes, test/ufuzz.js could be run on harmony without modifications to test if any ES5 constructs break. There's probably a few ES5 corner cases that a couple days of harmony fuzzing would reveal should anyone care to do it.

I think should uglify-es ever reach 100K daily downloads without frequent bug reports it'd be stable enough to be merged into master.

I'd also prefer ES6+ users to fix and improve uglify-es themselves.

The pool of people who could fix non-trivial uglify bugs is quite small.

I'm done with new ES6+ uglify-es features. Right now it's more or less reached language parity with ES8 and Chrome 60. Bug fixes aside, uglify-es should be good enough for a few years. I'm not crazy about some of the new ES proposals that'd require grammar changes.

Should browser advancements like Binary AST come to fruition, tools like uglify may be unnecessary or significantly less important in a couple of years.

@alexlamsl
Copy link
Collaborator Author

I think should uglify-es ever reach 100K daily downloads without frequent bug reports it'd be stable enough to be merged into master.

It's just semantics IMHO, since if we merge harmony we'll be making a branch for current master.

Should browser advancements like Binary AST come to fruition, tools like uglify may be unnecessary or significantly less important in a couple of years.

I was hoping AI would take over all programming tasks by then 😉

@kzc
Copy link
Contributor

kzc commented Aug 19, 2017

It's just semantics IMHO, since if we merge harmony we'll be making a branch for current master.

Whatever works. There are many ways to accomplish the switchover.

I suppose the github default branch for uglify-js could be harmony at that point. The harmony and master branches and their respective histories should still exist for legacy users - ideally with the branches' original names. At that point uglify-js@4 could simply be a minimal shim wrapper for the uglify-es@4.x package so that uglify-js and uglify-es user projects would not break and we can avoid explaining another uglify module name switchover.

I was hoping AI would take over all programming tasks by then

Whichever comes first.

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.

None yet

2 participants