search

breezewish / express-minify

Automatically minify and cache your javascript and css files.
https://npmjs.org/package/express-minify
MIT License
86 stars 18 forks source link

uglifyJS option and uglifyJS-X compatibility #52

Closed zevero closed 7 years ago

zevero commented 7 years ago

I gave it a try with uglify-es

app.use(minify({uglifyJS: require('uglify-es')})); //uglifyJS with ES6 support

but it does not work. There are uglifyJS2 and uglifyJS3 out there, which are stated to be mutually incompatible either.

What options do we really have with uglifyJS option? Can we document it?

Martii commented 7 years ago

@zevero Right now express-minify is only compatible with Uglify-JS@2.x. It will need to be modified which will bump out compatibility with v2.x from the mix. Major semver change over on that project. I've pinged breeswish last week about the changes.

breezewish commented 7 years ago

Sorry for the delay. Working on graduation project these months until mid June and after that I would take a look at this issue. PR are welcome though.

Martii commented 7 years ago

I can do that tomorrow when I get back to dev station but keep in mind that 2.x will no longer work.

breezewish commented 7 years ago

@Martii Seems that we need something like adapters so that both 2.x and es can be supported?

Martii commented 7 years ago

@breeswish Well the error on failures is no longer thrown so will need to either modify a test, or two, ... or throw an error manually in a trap like we did on OUJS. The instance that was added in #42 doesn't quite mention major semver compatibility which wasn't expected of course... but happened none-the-less.

Adapters might be a bit much for me as I don't know exactly how you would want that. I too will be a bit busy in the following weeks but I can at least submit a patch if you would like to migrate to v3.x of their package (npm package of uglify-js@3.x). 2.x is deprecated of UglifyJS2 (npm package of uglify-js@2.x). v3.x has been mostly stable in our experimental ES6+ minification (with npm package of uglify-es@3.x package).

Martii commented 7 years ago

@breeswish Current tests with mocha seem to be incompatible with 2.x and 3.x... which is what I figured. As I'm not real familiar with mocha, other than running the tests, I am unfortunately unable to take this on at this time due to other duties. I've tested the minifier.js locally with some changes and it works in all of our use cases and logically it should be the right step for your edit of inclusion of 2.x... but I don't want to miss something that we don't use so I'll defer to your expertise and perhaps the pings (GH mentions) I sent you approximately last week may give you a head start. :)

breezewish commented 7 years ago

I'm wondering if there any cases that UglifyJS 2 work previously but fail in UglifyJS 3? If not, I guess it might be better for express-minify to upgrade UglifyJS dependency from 2 to 3 while not breaking our users (since our users will get an expected output without any modifications).

Martii commented 7 years ago

@breeswish We're going to go hunting for an alternative to uglify-js since the project has gone downhill with maintaining. If we can find something a little more reliable that will be good to avoid their breaking changes ... both documented and mostly undocumented.

Appreciate your efforts and patience.

breezewish commented 7 years ago

Supported UglifyJs-3 and Uglify-Es in 1.0.0.