Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

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

Merged
merged 2 commits into from
May 21, 2019

Conversation

fdintino
Copy link
Collaborator

@fdintino fdintino commented Aug 23, 2018

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
Copy link
Member

asapach commented Aug 23, 2018

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

@fdintino
Copy link
Collaborator Author

@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 fdintino changed the title Use uglify-es to support minification of more permissive babel presets Use terser (uglify-js fork) to support minification of more permissive babel presets Aug 23, 2018
@fdintino fdintino force-pushed the add-es6-minify-support branch 3 times, most recently from d8c37fd to aae4cfd Compare August 24, 2018 14:56
@fdintino fdintino force-pushed the add-es6-minify-support branch 3 times, most recently from c8050c2 to 0016815 Compare September 10, 2018 15:47
Currently, only ES5 javascript can be minified, due to the reliance on
uglify-js. With terser (a maintained fork of the harmony branch of
UglifyJS2) it is possible to support a broader range of javascript
syntax.

The API changed between uglify-js@2 and uglify-js@3 / terser, but the
translation to the new API was fairly straightforward. It hews more
closely to the original code than systemjs#815, 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 long as it uses
uglify-js.

refs systemjs#815, systemjs#726
@fdintino
Copy link
Collaborator Author

@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
Copy link
Member

Sure, let's do it.

@guybedford guybedford merged commit 76c130a into systemjs:master May 21, 2019
@guybedford
Copy link
Member

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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants