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

Use Babili for minification #726

Open
guybedford opened this issue Oct 25, 2016 · 4 comments
Open

Use Babili for minification #726

guybedford opened this issue Oct 25, 2016 · 4 comments

Comments

@guybedford
Copy link
Member

We can add a step to the compiler at https://github.com/systemjs/builder/blob/master/compilers/compiler.js, that returns source, sourceMap, minifiedSource, minifiedSourceMap by running another compile phase on the same AST after the initial unminified compile. We can then cache all four along with the load record and solve the problem of having granular caching of minification as well (#209).

If anyone is interested in working on this contributions would be really really amazing.

@timfish
Copy link

timfish commented Jun 25, 2017

I had a quick look at what would happen if I replace minify() in output.js with some rather hacky code calling the latest uglify-es:

function minify(output, fileName, mangle, uglifyOpts) {
  var uglify = require('uglify-es');

  let files = {};
  files[fileName] = output.source;

  var result = uglify.minify(files, {
    sourceMap: {
      content: output.sourceMap,
      url: fileName + ".map"
    }
  });

  output.source = result.code;
  output.sourceMap = result.map;

  return output;
}

All the tests pass apart from these two:

  106 passing (16s)
  1 pending
  2 failing

  1) Test tree builds - Babel SFX tree build:
     Error: Phantom test failed test/test-sfx.html failed.
      at Error (native)
      at ChildProcess.<anonymous> (C:\Users\tim\Documents\Repos\systemjs-builder\test\test-build.js:28:16)
      at maybeClose (internal/child_process.js:877:16)
      at Process.ChildProcess._handle.onexit (internal/child_process.js:226:5)

  2) Test tree builds - TypeScript SFX tree build:
     Error: Phantom test failed test/test-sfx.html failed.
      at Error (native)
      at ChildProcess.<anonymous> (C:\Users\tim\Documents\Repos\systemjs-builder\test\test-build.js:28:16)
      at maybeClose (internal/child_process.js:877:16)
      at Process.ChildProcess._handle.onexit (internal/child_process.js:226:5)

I tried these changes on v0.15.x as that's the version which aurelia-bundler is still targeting but I couldn't actually get the bundle to load in my app. The new bundle is calling System.registry.get in places where previously it called System.get and falls over quickly. I'm guessing I haven't worked out which SystemJS version goes with which bundler version. For various reasons I'm still stuck on systemjs@0.19.

@guybedford
Copy link
Member Author

@timfish amazing, thanks for giving this a go! It's been sitting here way to long just waiting to be picked up by someone...

Yes the builder version 0.15 branch is for SystemJS 0.19. The code is exactly the same though, so if you wanted to try an initial PR against the 0.15 branch here, we can then merge it into the 0.16 branch as well when it's ready.

For the build failures, it may be due to the fact that certain globals must not be altered for builds. A careful comparison of the output should help track that down here. I'd be happy to give that a go too, if there was a PR to work with as a reference.

Not that carefully checking the source maps with multiple files is an important part of polishing this work for release, but having something to work with as a start would be great.

@timfish
Copy link

timfish commented Jun 28, 2017

One thing I did notice was that aurelia-bundler is failing to bundle async code even with minify: false so I've got to have a look there too. I assumed with minify disabled, the code wouldn't even go through uglify.

Ideally we'd keep uglifyOpts the same so it wouldn't be a breaking change. However, in v2 uglifyOpts is used only for compression options and in uglify@harmony, compression options are only part of the config.

I'm guessing if you'd want this on both 0.15 and 0.16 we'll keep the same configuration options for now?

@guybedford
Copy link
Member Author

@timfish the builder project on the 0.15 branch still runs code through Traceur. This is removed in the 0.16 branch though I believe so minify should be the only dependency there. Let me know if there is anything I can do to help with the 0.20 SystemJS upgrade here as well if that is a goal.

Yes uglifyOpts is a little inconsistent right now, there's also #689 here. Ideally, backwards compatibillity with a cleaner modern interface and to deprecate the old would be the best, but otherwise a breaking change could be possible if that isn't an option.

fdintino added a commit to fdintino/builder that referenced this issue Aug 23, 2018
Currently, only ES5 javascript can be minified, due to the reliance on
uglify-js. With uglify-es (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 / uglify-es, 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.

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.

refs systemjs#815, systemjs#726
fdintino added a commit to fdintino/builder that referenced this issue Aug 23, 2018
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 added a commit to fdintino/builder that referenced this issue Aug 24, 2018
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 added a commit to fdintino/builder that referenced this issue Aug 24, 2018
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 added a commit to fdintino/builder that referenced this issue Aug 24, 2018
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 added a commit to fdintino/builder that referenced this issue Sep 10, 2018
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 added a commit to fdintino/builder that referenced this issue Sep 10, 2018
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 added a commit to fdintino/builder that referenced this issue Sep 10, 2018
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 added a commit to fdintino/builder that referenced this issue Jan 24, 2019
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants