mishoo / UglifyJS

JavaScript parser / mangler / compressor / beautifier toolkit
http://lisperator.net/uglifyjs/
Other
13.09k stars 1.25k forks source link

Harmony support #448

Closed ozanmakes closed 7 years ago

ozanmakes commented 10 years ago

I'm trying to use the excellent livenode with a project which uses ES6 features introduced in node v11, but unfortunately UglifyJS's parser chokes when it encounters a let statement or a generator function (function *() { yield true }).

fabiosantoscode commented 9 years ago

@sbrl my idea was more like, release uglify-js as it is, but with the ES6 it currently supports.

@tresdin Destructuring is done, but it doesn't work with --mangle-props yet. I am currently working on ES6 strings (the ones with backticks), and plan to work on class, let, and computed properties ({["fo" + "o"]: "bar"}) next, but don't know which one I'll do first yet.

lewispham commented 9 years ago

Thanks for your contribution @fabiosantoscode . AFAIK, these ES6 features may affect UglifyJS results.

I think it would be nice to have for...of, arrow functions, class, let, const, destructing assignment, template strings, and Object initializer features in beta version since most of them are now supported in major browsers.

fabiosantoscode commented 9 years ago

Of those features you listed, these are the ones not done yet:

class
let
Template strings
Object initializer (computed property names, shorthand method names) (shorthand props are done)
Default parameters
Generator function
import
export

Const has been ready for quite some time.

g-ortuno commented 9 years ago

@fabiosantoscode my vote is for class and let!

Martii commented 9 years ago

for (object of objects) e.g. the of

fabiosantoscode commented 9 years ago

@Martii got that working in the harmony branch

Martii commented 9 years ago

@fabiosantoscode Awesome... just hope node at some point gets full ES6 support (w/o a build switch) so I can start using that rather than just uglify'ing it for serving.

fabiosantoscode commented 9 years ago

@askmatey if I recall correctly, you can use #harmony instead of a commit hash

@Martii Indeed, using traceur is a bother and has quite a performance toll.

niloy commented 8 years ago

+1 👍

callaars commented 8 years ago

+1

wilk commented 8 years ago

+1

terinjokes commented 8 years ago

@askmatey Modifying gulp-uglify's package.json is unnecessary, you can provide your own version of UglifyJS by importing gulp-uglify/minifier instead:

var gulpUglify = require('gulp-uglify/minifier');
var uglifyJS = require('uglify-js');

gulp.src(['*.js'])
  .pipe(gulpUglify({}, uglifyJS)  /* opts, UglifyJS */
  .pipe(gulp.dest('dist/'));
weekens commented 8 years ago

+1

cin210 commented 8 years ago

+1

fabiosantoscode commented 8 years ago

Chill, everyone. This is being worked on

zhiqingchen commented 8 years ago

good news

avdg commented 8 years ago

If someone like to experiment on harmony uglify in experimental usage, maybe this could help https://stackoverflow.com/questions/16350673/depend-on-a-branch-or-tag-using-a-git-url-in-a-package-json.

Note: experimental usage only, things may break!

Marqin commented 8 years ago

@fabiosantoscode On your "not working" list there is no Spread Operator, but it's not working for me: Unexpected token: punc (.) [./app/index.js:22,0] on uglify-js 2.6.1.

fabiosantoscode commented 8 years ago

@Marqin you need to grab the uglifyjs code in the harmony branch, not from npm. The new stuff hasn't been merged in yet but you are welcome to try it, and more than welcome to give any feedback or bugs you might find ;)

CrimsonVex commented 8 years ago

How do we use the new code to support ES6? I'm not familiar with the procedure to overwrite the uglifyjs I'm using via NPM.

avdg commented 8 years ago

@Sasstraliss Watch a few comments above (https://github.com/mishoo/UglifyJS2/issues/448#issuecomment-163775898)

Martii commented 8 years ago

@avdg After the holiday (this includes new years) I'll see about opening a test route on OUJS that will give some more testing data... I'll need to trap any errors (hopefully) that pop up because I don't want to trip our production into a restart.

fabiosantoscode commented 8 years ago

@Martii awesome! I haven't worked much on that branch the past months, but besides the open pull request for defaults, there's generators and little more left.

I have used some of the new features in my MP project, it's a (terrible) game written in es6 which doesn't use a es6-es5 compiler if it detects the browser supports some es6. It minifies the es6 with uglifyjs if these features are supported.

Martii commented 8 years ago

@fabiosantoscode We're currently running on npm@2.x so it shouldn't be too difficult but if node updates to npm@3.x , and we do that before the route, I'm not sure how express-minify will handle this in the "flat, flat, flatter" dep tree that 3.x gives of npm ... with having one dependency in the package.json asking for one version of UglifyJS2 and our depth 0 request from GH... time will tell when I get back to main dev station after the holidaze.

vpj commented 8 years ago

+1

What is your favorite es6 feature?

Generators!

RevanProdigalKnight commented 8 years ago

Could we please get an updated list of what's still left before the harmony branch is ready?

tenKinetic commented 8 years ago

Nice work people (mostly @fabiosantoscode?). Finally had an afternoon to sit down and see if I can get the harmony branch to provide me with some uglified ES6 classes rather than ES5+polyfill. Took 20 mins because of the time and effort you guys have put in. Thanks.

buunguyen commented 8 years ago

If Uglifier supports generators, we can use it to process transpiled code because generators can't be transpiled. Usage of other ES6 features can at least be handled by Babel. IMO, support for generators should have high priority.

mgol commented 8 years ago

@buunguyen What do you mean by "generators can't be transpiled"? They sure can.

buunguyen commented 8 years ago

Sorry, you're totally right. I've always thought Babel can't transpile generators, turn out it's a configuration problem. Thanks for clearing my head.

fabiosantoscode commented 8 years ago

Generators have been addressed in a pull request a while ago, but it seems like it's been abandoned.

All that's missing besides generators, if I recall correctly, is some parts of restructuring objects and arguments, specifically constructs like var { a:b=3 } = foo;.

I don't think I wish to implement the import statement until the spec on how they actually work is completed

RevanProdigalKnight commented 8 years ago

@fabiosantoscode According to MDN the spec is completed, but no browsers or runtimes have implemented it yet - just transpilers (Traceur, Babel, etc.). It doesn't look like there's really anything that can be minified in an import statement aside from getting rid of spaces around commas and curly braces. The export statement looks to be pretty similar.

RamIdeas commented 8 years ago

It doesn't look like there's really anything that can be minified in an import statement...

@RevanProdigalKnight Err... variable names?

sbrl commented 8 years ago

@RevanProdigalKnight @fabiosantoscode There's active development going on in the v8 engine on destructuring: https://bugs.chromium.org/p/v8/issues/detail?id=811

topaxi commented 8 years ago

@RevanProdigalKnight imports could be renamed to minify the output:

import foo, { bar, baz } from "foo"

can be minified to:

import f,{bar as b,baz as c}from"foo";

or

import { default as bar } from "bar"

to:

import b from"bar";
RevanProdigalKnight commented 8 years ago

@RamIdeas, @topaxi Unless I'm mistaken, the only names that would be minifiable would be the aliases (name as alias) in the import statement, which technically speaking, is an as statement inside the import statement, and could probably be adapted from the existing var, let, or const code with some modifications.

I'm also guessing the actual module names in the import/export would probably have to be left alone so as to avoid cross-file collisions or other problems when you aren't the owner/creator of the other file (e.g. external libraries).

e.g. import reallylongmodulename as alias from "module" -> import reallylongmodulename as a from "module"

topaxi commented 8 years ago

You can't alias the default import, it's already an alias:

import foo from 'foo'

Is the short form of:

import { default as foo } from 'foo'

Martii commented 8 years ago

@fabiosantoscode Does the Harmony branch actually obfuscate ES6 code?

I just did one single test on my eldest and most known user.js and the code itself doesn't seem to resemble the native/raw code I put in... unfortunately I can't test it since USO went offline but perhaps the mirror I can get it working on even though parts of the mirror are missing.


P.S. strict ES5 appears to obfuscate too with identifier changes... didn't notice this with ES5 before... so n/m on the question. Sorry to bother.

Other than this surprise... seems to be working with my legacy Moz ES6 code so far with the server. :)

Martii commented 8 years ago

This is a little off-topic... but we've just gotten one report of ES5 alleged failure... I can't see anything unique pattern wise in that users most installed user script but I'm not the UglifyJS2 expert here. express-minify is pulling current release 2.6.1 and within the next few days or earlier I'll be (hopefully) switching to the harmony branch specific to this issue... which should be the same for ES5 based code I think.

Obviously that report is super vague and I don't doubt there will be some out there that won't want to do minification which is why they have a choice on our site instead of imposed like some... but as I mentioned in our issue ticket we've been minifying all node .js since express-minify came into being in our project so imho that invalidates a bit with lack of citation from that author.

avdg commented 8 years ago

@martii Please open a new issue in case you manage to isolate the bug somewhat in such a way others can repeat (a script that fails compiling, a test case that shows the code isn't doing the same after minimizing, etc...).

Also note that I'm currently running test262 es5 tests (yeah, these are the standard es tests) and I've managed already to track quite many bugs and squash a few of them (with some help from others).

Test262 es6 tests is scheduled on my part to be done later when I have the time to do this (though I don't have a date on when I do this). Though I can already guess that arrow functions are likely not to behave properly on a subject like scoping (though such cases only proof that testing is urgently needed).

For those who want to know: I'm using the tester from https://github.com/mishoo/UglifyJS2/pull/821

fabiosantoscode commented 8 years ago

From the many projects I've used uglifyjs in, even with the unsafe option I never experienced any bug either during compilation or in the resulting code. This is super solid software.

I can't say the same for my branch yet, since it hasn't been battle tested. I just used it for one project, and used very little ES6.

About import, I'd say this can be released. I have a feeling that browsers will never implement, and if they do, we can always implement it ourselves then! It is in the interest in keeping uglifyjs simple and practical, as opposed to being complete, even more so than the browsers we're supposed to be targeting.

On 09:38, Tue, 19 Jan 2016 Marti Martz notifications@github.com wrote:

This is a little off-topic... but we've just gotten one report https://openuserjs.org/discuss/How_to_disable_install_with_minification_on_my_script of ES5 alleged failure... I can't see anything unique pattern wise in that users most installed script but I'm not the UglifyJS2 expert here. express-minify is pulling current release 2.6.1 and within the next few days or earlier I'll be switching to the harmony branch specific to this issue... which should be the same for ES5 based code I think.

Obviously that report is super vague and I don't doubt there will be some out there that won't want to do minification which is why they have a choice on our site instead of imposed like some... but as I mentioned in our issue ticket we've been minifying all node .js since express-minify came into being in our project so imho that invalidates a bit with lack of citation from that author.

— Reply to this email directly or view it on GitHub https://github.com/mishoo/UglifyJS2/issues/448#issuecomment-172792982.

Martii commented 8 years ago

@fabiosantoscode

Well it's not just about browsers implementing it... node packages can include ES6 in it too... so I get double-duty testing here... this is why I wanted some ES5 data in first which of course I'll report what I can as mentioned by @avdg in new issues... but not everyone in the user script community is vocal... so sometimes a small few get to guess and/or figure it out. I have seen some .user.js in the past that had import statements... had no clue on how it was used at the time when it was new (some time back in the last 10 years)... but I do see it in use now on some websites... and it will probably be in node based packages sooner than later.

lewispham commented 8 years ago

How can I report an issue that's related to harmony branch?

avdg commented 8 years ago

You can open an issue and mention its harmony related. Note that all work is done in public and probably all work done here is voluntarily with limited resources (it consumes time from humans) and that the code is public (so anyone feeling to submit fixes can submit them if they know the code and how to use github).

Reported harmony issues may be labeled with the harmony labels later on with those having the authorization to do that.

mishoo commented 8 years ago

@tresdin I added a harmony label, just use it when creating an issue. As a personal preference, I'd also prepend [ES6] to the subject; I know that's redundant with the label, but it helps when I see a list of issues in my mailbox.

I welcome more people to test this branch and report bugs/send fixes. However I'm not (yet?) writing ES6 myself, nor do I have any need to minify it. I switched to harmony branch once and accidentally found some bugs (didn't care to investigate, but they seemed related to keeping the correct start / end offsets in the AST nodes). So I second @fabiosantoscode here—I do worry about stability and prefer not to merge this branch until proven solid.

Martii commented 8 years ago

@mishoo

Understanding that the branch is considered unstable... comments like these have me a bit concerned for testing... if the ES6 branch hoses one of our node package .js files that wouldn't be good for production... so far I don't see any issues with our packages but I periodically do dep updates and I can test general usage but I'd have to back out to the ES5 release branch if there is a problem. One of my points on dev OUJS is to do a test route but I don't know if UglifyJS2 can be done with a new statement or not so I can isolate the instance... typically in node it's just require('something') which is a static object I think... so I have a conundrum here. (with express-minify in the mix too ... please note sometimes I miss the obvious as my mind is multi-tasking elsewhere too)

fabiosantoscode commented 8 years ago

Have 2 uglifyjs!

"uglifyjs": " whatever.version.lol", "uglifyjsintheharmonybranch": "git repo"

(Haven't tested it. Does it work?)

On 16:43, Thu, 21 Jan 2016 Marti Martz notifications@github.com wrote:

@mishoo https://github.com/mishoo

Understanding that branch is considered unstable... comments like these have me a bit concerned for testing... if ES6 hoses one of our node package .js files the harmony branch that wouldn't be good for production... so far I don't see any issues with our packages but I periodically do dep updates and I can test general usage but I'd have to back out to the ES5 release branch. One of my points on dev OUJS is to do a test route but I don't know if UglifyJS2 can be done with a new statement or not so I can isolate the instance... typically in node it's just require('something') which is a static object I think... so I have a conundrum here.

— Reply to this email directly or view it on GitHub https://github.com/mishoo/UglifyJS2/issues/448#issuecomment-173629539.

mishoo commented 8 years ago

@fabiosantoscode — Of course it works. It's called a fork. ;-) I have nothing against forks—do it, advertise it, have people using it and make it stable, and later we can merge back. At worst we end up with two divergent versions, but this seems unlikely.

fabiosantoscode commented 8 years ago

It's not a fork, it's me trying to get npm to download 2 uglifyjs modules under different names. There can be only one node_modules/uglifyjs

Martii commented 8 years ago

@fabiosantoscode

There can be only one node_modules/uglifyjs

That's definitely one of the issues with npm@3.x ... we are currently on npm@2.x which might allow it since the deps are not "flat, flatter..." as paraphrased from npm. As I'm watching and interacting with node right now with their tests and npm@3.x it could come any day now... this presents a short term interruption issue... so I would have to find a solution that will do it both ways.

I'm not "as worried" with packages .js because I can usually catch those... but it does take longer to do the full battery of tests I do for the site every dep update which slows our development. I debug packages too... and if it's a ES6 glitch that shows up that's when I get to toggle it to the ES5 release branch and wait and see if it happens there (blech). We do have two clones on the VPS so I can toggle between them... but that requires human interaction... hopefully I would be conscious to observe potential issues.

I'll probably just take a leap of faith and any consequences will rest on my shoulders... but I don't want to upset our users... an occasional inconvenience is okay but if it's too long then they get restless.

Another option might be, which is lengthy, is to only UglifyJS2 the scripts and go serve the static packages .min.js (if available) with express-minify and tell it to ignore everything else but the user scripts... but I'd still like to have my cake and eat it. ;) :)