systemjs / builder

SystemJS build tool
MIT License
466 stars 122 forks source link

Use terser (uglify-js fork) to support minification of more permissive babel presets #857

Closed fdintino closed 5 years ago

fdintino commented 5 years ago

Currently, only ES5 javascript can be minified, due to the reliance on uglify-js. Terser is the actively maintained fork of what used to be uglify-es, the harmony branch for uglify-js@3. By using terse the systemjs builder is able to support a broader range of ECMAScript syntax.

There were significant API changes between uglify-js@2 and uglify-js@3, and so also with terser, but the updates required in the systemjs builder minify function were fairly straightforward. This PR hews more closely to the original code than #815 does, and consequently I think it avoids regressing any features (for instance, the ability to include sourceContents in source maps or to control the comment stripping is retained).

As browsers continue to support more features beyond ES5, and because babel recommends use of the 'env' preset, it will become increasingly likely that users' babel transpilation settings will cause errors when trying to minify with jspm and friends.

As part of this PR I've added a test suite that fails before this fix and passes afterwards.

refs #815, #726

asapach commented 5 years ago

Unfortunately uglify-es is no longer supported: mishoo/UglifyJS2#3156 You can try this instead: https://github.com/fabiosantoscode/terser

fdintino commented 5 years ago

@asapach thank you for the heads up about uglify-es. I can't say that I'm surprised that they would suddenly stop supporting something so widely used without a mention anywhere except in that one issue (as far as I can tell). I'm hopeful that the maintainers of terser are better open-source citizens.

fdintino commented 5 years ago

@guybedford I know this project won't be receiving many (if any) updates now that jspm 2 is out. But is there any chance this might be merged in?

guybedford commented 5 years ago

Sure, let's do it.

guybedford commented 5 years ago

Published in 0.16.14.

@fdintino if you are relying on this and would like to be a co-maintainer of this project I'd be happy to add you. Just let me know.

guybedford commented 5 years ago

Unfortunately it seems the internal terser import is broken in modern terser, so I've locked the terser version to 3.8.1 and released this in 0.16.15.

fdintino commented 5 years ago

Ah, I'm sorry about that. I should have checked that it still worked with the most recent version of terser before asking for it to be merged. I'm glad that Terser's source map class is exposed publicly (so that it shouldn't need the hacky pseudo-import that this PR uses). I can take a stab at that and open another PR to have it work with modern terser.

I'd be happy to be a co-maintainer. At some point I'll be upgrading my projects to jspm 2, but at least for the moment I'm still using jspm 0.17 and systemjs-builder.

guybedford commented 5 years ago

@fdintino sure, I've added you as a collaborator. If you want to put together an updated PR I'll gladly review further and will aim to not let you wait quite so long :) Thanks for your work here.