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

UglifyJS 3.0 changes (breaking changes only) #1411

Closed
avdg opened this issue Dec 29, 2016 · 25 comments
Closed

UglifyJS 3.0 changes (breaking changes only) #1411

avdg opened this issue Dec 29, 2016 · 25 comments

Comments

@avdg
Copy link
Contributor

@avdg avdg commented Dec 29, 2016

Feel free to drop candidate changes for uglify that shouldn't make normal releases, but still adds consistency and better experience to UglifyJS in this issue.

Please limit posting to references only.

@avdg

This comment has been minimized.

Copy link
Contributor Author

@avdg avdg commented Dec 29, 2016

  • #1366 Change simple_glob to glob
  • #1366 Deprecate DefaultsError.msg (use DefaultsError.message instead)
@kzc

This comment has been minimized.

Copy link
Contributor

@kzc kzc commented Dec 29, 2016

Since we're blue skying here:

  • #1280 bin/uglifyjs should be implemented solely with the minify() API

  • Simplify and flatten minify() API options - only a single option object combining parse, compress, mangle, mangleProps and output/beautify options: #1366 (comment)

  • Replace yargs CLI argument handling with something much smaller: #1280 (comment)

  • Replace the implementation of comma sequences (AST_Seq) from a tree node with car and cdr to a more efficient representation using an array. This would remove the need for deep recursion with respect to optimizing comma sequences and lower memory usage: #863 (comment)

  • Split Uglify into two separate modules - library and CLI - to benefit uglify wrappers and JS bundlers: #1280 (comment).

  • Uglify 3.0 development should probably take place in a new branch - and possibly a different project altogether - UglifyJS3.

Deferred for UglifyJS 4.x:

  • #448 merge Harmony/ES6+ support once mangle and compress scope issues resolved.

  • #968 Support ES6+ constructs in spidermonkey (ESTree) JSON import/export.

Not sure if any of this will see the light of day. I don't have much time to devote to this effort. UglifyJS2 is basically in maintenance mode at this point with a lack of volunteers.

@TheLarkInn

This comment has been minimized.

Copy link

@TheLarkInn TheLarkInn commented Jan 21, 2017

Hey is there any way we can help. Would some sort of sponsorship help increase support on the project. We (at webpack) really want to help. Not just because we leverage UglifyJs, but because of the impact it has had on the community.

How many contributors and daily support would help? I am willing to see if I can find both sponsors and those to help maintain the project.

-Sean

@hzoo

This comment has been minimized.

Copy link

@hzoo hzoo commented Jan 21, 2017

👍 Are we looking for sponsorship for maintenance, uglify 3.0 development, or both?


Just a thought for the future: I was thinking maybe UglifyJS and Babili (babel-minify) could merge? We can discuss elsewhere too (or in a different thread) if that's more comfortable. I would say it would be easier to maintain UglifyJS 2 and then for the future UglifyJS 3 it could just be Babili? We'll have to talk about all that though but just an idea I thought of.

Right now the Babili team is also very small: @kangax/@boopathi and me (only sometimes since I have trouble maintaining Babel in general) so we are also looking for help. And the whole Babel team is supporting it indirectly via improving Babel itself. But yeah, maybe it could be an opportunity to combine forces?

Reasoning: Given Babili is a babel-preset, it should support all ES2015+ features moving forward. This means that uglify won't need to focus on making it's on parser support every new syntax, and with Babylon hopefully outputting estree output again (this may take a while but is possible and we are willing to do it) we can all share the same AST format. Babel is used to transform ES2015+ to ES5, etc and so a minifier is a reasonable task to take on as well.

EDIT: And of course just combining forces and adding like 2-3 more people to the team isn't going to mean it's sustainable, just a way to interop better. We would still need a lot more contributors/maintainers going forward. Babel itself isn't sponsored or has full-time people either. I'm just hoping that this being something developers and companies really need will cause someone to help and do something.

Specifically via https://github.com/babel/babel-preset-env it will make it easy for developers to want to compile down to ES6 and above syntax if there target browsers support it natively. Thus you would want to use a minifier that understands all JS. It's in the interest of companies, web browsers, etc as well.

@avdg

This comment has been minimized.

Copy link
Contributor Author

@avdg avdg commented Jan 21, 2017

@hzoo yeah I was thinking about that too. Well, we still need competition (we are competing for the best product in the end). But too much competition may be costly on resources (maintaining project overhead, unneeded double implementations although it may preferably not cut redundancy if we have enough support for it).

Anyway, this topic is about changes for uglify 3.0 and not about how to maintain the project on a lower level.

Let's move this discussion to #1181 for now?

@hzoo

This comment has been minimized.

Copy link

@hzoo hzoo commented Jan 21, 2017

Ah I didn't see that issue! thanks for pointing that out.

@indutny

This comment has been minimized.

Copy link

@indutny indutny commented Jan 21, 2017

I'd suggest to create a Patreon campaign, there are definitely some people who will be happy to back the project there.

@daragao

This comment has been minimized.

Copy link

@daragao daragao commented Jan 21, 2017

Would love to help how ever I can, how can a newbie to open source contributions help?

@avdg

This comment has been minimized.

Copy link
Contributor Author

@avdg avdg commented Jan 21, 2017

@daragao current todo list can be found at #1433 (comment)

@avdg avdg changed the title UglifyJS 3.0 changes UglifyJS 3.0 changes (breaking changes only) Jan 21, 2017
@alexlamsl

This comment has been minimized.

Copy link
Collaborator

@alexlamsl alexlamsl commented Apr 10, 2017

Planning to create a 2.x branch this weekend, which means #1460 will get merged onto master.

Currently on my TODO list:

  • merge bin/uglifyjs and minify()
    • pass through options.output to parser
    • allow true for options.mangleProperties (currently you need at least {})
  • enhance reduce_vars to deal with #27, #138 & #315

I'm thinking of having minify() does fromString: true by default, or just remove anything fs from it altogether.

@avdg

This comment has been minimized.

Copy link
Contributor Author

@avdg avdg commented Apr 10, 2017

uh... yeah, that portable api is quite important...

@kzc

This comment has been minimized.

Copy link
Contributor

@kzc kzc commented Apr 10, 2017

pass through options.output to parser

@alexlamsl If you're going to break the API in 3.x anyway, why not pass a single minify-like options object to all components and end the confusion? Each of the components could benefit from knowing options in another - mangler and compress, mangle and mangleProperties, etc.

@alexlamsl

This comment has been minimized.

Copy link
Collaborator

@alexlamsl alexlamsl commented Apr 10, 2017

@kzc sure, but are we keeping the grouping as well? That is, passing options to all 4 stages that contains compress, mangle, output & mangleProperties as child objects.

The other two options are:

  • flatten the whole options object
  • keep the hierarchy, but pull out stuff like screw_ie8
@kzc

This comment has been minimized.

Copy link
Contributor

@kzc kzc commented Apr 10, 2017

Purely flat options is certainly the simplest from the users' perspective, but it could be a problem as -c toplevel and -m toplevel are distinct options as you know. Same deal with keep_fnames and screw_ie8.

Maybe keep the minify options hierarchy but also have top level options overrides? That way it could appear flat but one could use the minify options hierarchy if need be?

@kzc

This comment has been minimized.

Copy link
Contributor

@kzc kzc commented Apr 10, 2017

Or perhaps flatten out all options with a prefix for the component?

  • compress_keep_fnames
  • compress_toplevel
  • mangle_keep_fnames
  • mangle_toplevel
    etc.

Just throwing ideas out there.

@alexlamsl

This comment has been minimized.

Copy link
Collaborator

@alexlamsl alexlamsl commented Apr 10, 2017

Maybe keep the minify options hierarchy but also have top level options overrides? That way it could appear flat but one could use the minify options hierarchy if need be?

Sounds reasonable - have top-level options like toplevel and screw_ie8 acting as shortcuts to set into compress, mangle and output, like what CLI has for --support-ie8 today.

@kzc

This comment has been minimized.

Copy link
Contributor

@kzc kzc commented Apr 10, 2017

Sounds reasonable - have top-level options like toplevel and screw_ie8 acting as shortcuts to set into compress, mangle and output, like what CLI has for --support-ie8 today.

Sure, and minify() would benefit from the same options scheme.

@kzc

This comment has been minimized.

Copy link
Contributor

@kzc kzc commented Apr 10, 2017

Bonus points if 3.x's options would be backwards compatible with 2.x. It is difficult to get downstream projects to update their APIs.

But if not possible or not practical, 3.x is the time for a clean break.

@alexlamsl

This comment has been minimized.

Copy link
Collaborator

@alexlamsl alexlamsl commented Apr 10, 2017

I think we can keep mostly used options compatible, especially if we are introducing new shortcut options that only sets the existing ones.

I'm actually tempted to remove all file-related operations from minify() - bin/uglifyjs will still handle files (obviously) but through its own dependence on fs and may be even glob.

But I don't really see a point in minify() reading in files, especially when it can't actually write anything out to the file system even with the current versions.

(Okay, technically it writes out name caches as files under some conditions, but passing in and returning an object with key names as paths and values as file contents shouldn't be too difficult.)

@avdg

This comment has been minimized.

Copy link
Contributor Author

@avdg avdg commented Apr 10, 2017

Hmm... do we need objects to represent files and pass that as parameter to minify?

(something like {name: "foo", content: "var bar = '123'";})

@alexlamsl

This comment has been minimized.

Copy link
Collaborator

@alexlamsl alexlamsl commented Apr 10, 2017

Hmm... do we need objects to represent files and pass that as parameter to minify?

That's what I'm thinking if we were to get rid of file handling within minify(). So something like:

minify({
    "main.js": "println(42);",
    "lib/a.js": "function println(n) { console.log(n); return n; }"
}, {
    screw_ie8: false,
    toplevel: true,
    output: {
        expression: true
    }
});
@avdg

This comment has been minimized.

Copy link
Contributor Author

@avdg avdg commented Apr 10, 2017

or that :-) or both if we need to add more properties later some day

@kzc

This comment has been minimized.

Copy link
Contributor

@kzc kzc commented Apr 10, 2017

But I don't really see a point in minify() reading in files, especially when it can't actually write anything out to the file system even with the current versions.

That API has been abstracted in #1366 such that one could easily implement in-memory data structures representing a file system. This would be useful in a pure browser-side uglify implementation.

@avdg

This comment has been minimized.

Copy link
Contributor Author

@avdg avdg commented Apr 10, 2017

maybe even all 3 :/

But we have async stuff, generators, promises in js, or simple preload the files before using minify (and use a lookup table as file api). Lots of way to control how files are loaded. Just overloading the file api and you get your own behaviour.

@calebboyd

This comment has been minimized.

Copy link

@calebboyd calebboyd commented Apr 26, 2017

I'm not too familiar with the project, but ran into an issue with some bundlers, choking on the eval'd code in tools/node.js (eg, the code that needs to be loaded can't be analyzed) when webpack, rollup etc is used. Wondering if anyone thinks its a good idea to change to a more traditional bundling method.

@alexlamsl alexlamsl closed this May 6, 2017
@azu azu mentioned this issue May 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.