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

ES6 & Dead Code Elimination #1379

Closed
mattdesl opened this Issue Sep 10, 2015 · 15 comments

Comments

Projects
None yet
10 participants
@mattdesl
Contributor

mattdesl commented Sep 10, 2015

I recently got pointed rollup, which produces clean builds by assuming the source is using ES6 import/export. It then can mash all the modules into the same closure, instead of the current browser-pack approach that tends to produce a lot of extra boilerplate and closures. It can also tree-shake unused functions.

It would be worth exploring whether something similar is possible in browserify (or, as a plugin/transform independently of browserify).

Big things to figure out:

  • a means of resolving to ES6 source for packages written in ES6 and pre-published to ES5. Right now rollup uses esnext:main which isn't exactly standard, and maybe could be named better (i.e. what happens with ES7?). It would also be good to have other bundlers like jspm and webpack agree on the same approach.
  • how to take the rollup approach (single closure) rather than bundle-pack approach (closure per module) and what kind of problems this might impose
@substack

This comment has been minimized.

Show comment
Hide comment
@substack

substack Oct 28, 2015

Collaborator

Is this really so specific to ES6? It seems like an optimizer that worked for ES6 should also be capable of analyzing require() with a string argument as if it were a statically analyzable expression. I might revisit this problem soon, but perhaps not.

Collaborator

substack commented Oct 28, 2015

Is this really so specific to ES6? It seems like an optimizer that worked for ES6 should also be capable of analyzing require() with a string argument as if it were a statically analyzable expression. I might revisit this problem soon, but perhaps not.

@marlun

This comment has been minimized.

Show comment
Hide comment
@marlun

marlun Dec 27, 2015

I'm also interested in how this would work with Browserify.

marlun commented Dec 27, 2015

I'm also interested in how this would work with Browserify.

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson Feb 12, 2016

One of the more poorly-understood benefits of Rollup's approach is that, in addition to tree-shaking, it also hoists modules into a single scope. This is really only possible because of the semantics of ES6 modules, which unlike CommonJS does not enforce a separate function scope for every module.

The traditional CJS solution to tree-shaking is to make your modules as tiny as possible, or use tricks like require('some/deep/func'), but the problem there is that the more you modularize, the more you introduce cruft when you browserify/webpack, due to the required function-scoping boilerplate for every module. You're punished for doing the right thing.

We noticed this problem in PouchDB, so we recently introduced Rollup as a prebuild step before Browserify, and migrated from CJS to ES6. We had a codebase with lots of small submodules (many files were a single function, 5-20 lines or so). We also had no dead code, as verified by Istanbul coverage tests. However, even without any tree-shaking, Rollup managed to save us 3KB out of 48KB total, which is nothing to sneeze at.

If Browserify or a Browserify plugin could do this same trick, we'd probably be happy to cut Rollup out of our build pipeline and use Browserify only. :)

One of the more poorly-understood benefits of Rollup's approach is that, in addition to tree-shaking, it also hoists modules into a single scope. This is really only possible because of the semantics of ES6 modules, which unlike CommonJS does not enforce a separate function scope for every module.

The traditional CJS solution to tree-shaking is to make your modules as tiny as possible, or use tricks like require('some/deep/func'), but the problem there is that the more you modularize, the more you introduce cruft when you browserify/webpack, due to the required function-scoping boilerplate for every module. You're punished for doing the right thing.

We noticed this problem in PouchDB, so we recently introduced Rollup as a prebuild step before Browserify, and migrated from CJS to ES6. We had a codebase with lots of small submodules (many files were a single function, 5-20 lines or so). We also had no dead code, as verified by Istanbul coverage tests. However, even without any tree-shaking, Rollup managed to save us 3KB out of 48KB total, which is nothing to sneeze at.

If Browserify or a Browserify plugin could do this same trick, we'd probably be happy to cut Rollup out of our build pipeline and use Browserify only. :)

@capaj

This comment has been minimized.

Show comment
Hide comment
@capaj

capaj Feb 12, 2016

@nolanlawson you probably don't consider JSPM a viable alternative, but if you did you probably knew that JSPM uses rollup since version 0.17.0 internally for it's bundling optimization. So with JSPM you could cut rollup out of the pipeline.

capaj commented Feb 12, 2016

@nolanlawson you probably don't consider JSPM a viable alternative, but if you did you probably knew that JSPM uses rollup since version 0.17.0 internally for it's bundling optimization. So with JSPM you could cut rollup out of the pipeline.

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson Feb 12, 2016

Actually I didn't know that; thanks for the tip! :)

Actually I didn't know that; thanks for the tip! :)

@maxogden

This comment has been minimized.

Show comment
Hide comment
@maxogden

maxogden Feb 12, 2016

Member

@nolanlawson Do you have an example of the cruft, e.g. the 6.7% you were able to remove, maybe as a github diff?

Member

maxogden commented Feb 12, 2016

@nolanlawson Do you have an example of the cruft, e.g. the 6.7% you were able to remove, maybe as a github diff?

@reconbot

This comment has been minimized.

Show comment
Hide comment
@reconbot

reconbot Feb 12, 2016

@nolanlawson has a neat repo with some obvious differences. You'd want to compare these rollup and browserify examples.

@nolanlawson has a neat repo with some obvious differences. You'd want to compare these rollup and browserify examples.

@maxogden

This comment has been minimized.

Show comment
Hide comment
@maxogden

maxogden Feb 12, 2016

Member

@reconbot Cool thanks. For me it would be this one https://github.com/nolanlawson/rollup-comparison/blob/master/dist/bundle-browserify-es5-only.js. Would be cool to have some real modules in there too, but I suppose you'd see the 6.7% difference as it is in pouchdb, though obviously the amount of overall cruft depends on average module size.

For me it comes down to a size/complexity tradeoff, so I like to factor in things like how often tools break, how fast they are, if they are compatible with the runtimes i'm using, if I have to transpile or not. I'm still unsure about all of those things when it comes to ES6 modules, but at least now I have a better standing of the size differences inherent in the modules systems.

Member

maxogden commented Feb 12, 2016

@reconbot Cool thanks. For me it would be this one https://github.com/nolanlawson/rollup-comparison/blob/master/dist/bundle-browserify-es5-only.js. Would be cool to have some real modules in there too, but I suppose you'd see the 6.7% difference as it is in pouchdb, though obviously the amount of overall cruft depends on average module size.

For me it comes down to a size/complexity tradeoff, so I like to factor in things like how often tools break, how fast they are, if they are compatible with the runtimes i'm using, if I have to transpile or not. I'm still unsure about all of those things when it comes to ES6 modules, but at least now I have a better standing of the size differences inherent in the modules systems.

@drpicox

This comment has been minimized.

Show comment
Hide comment
@drpicox

drpicox Sep 6, 2016

Rollupify does not works exactly as one expect that it should do.

rollupify only works on ES6/ES2015 modules. Any require() statements will be left untouched, and passed on to Browserify like normal.
But it happens that because it concatenates all files into the current one, when browserify process requires it looses the path.

For example:

// index.js
import { Bob } from './people/bob.js';
Bob.wave();
// ./people/bob.js
let text = require('./text.json');
export class Bob {
    wave() { console.log(text.hello); }
}

Does not work:

Error: Cannot find module './text.json' from '/Users/.../rollupify-poc'

but with this change, it works:

// ./people/bob.js
let text = require('./people/text.json');
export class Bob {
    wave() { console.log(text.hello); }
}

Which is not the expected, neither the convenient.

drpicox commented Sep 6, 2016

Rollupify does not works exactly as one expect that it should do.

rollupify only works on ES6/ES2015 modules. Any require() statements will be left untouched, and passed on to Browserify like normal.
But it happens that because it concatenates all files into the current one, when browserify process requires it looses the path.

For example:

// index.js
import { Bob } from './people/bob.js';
Bob.wave();
// ./people/bob.js
let text = require('./text.json');
export class Bob {
    wave() { console.log(text.hello); }
}

Does not work:

Error: Cannot find module './text.json' from '/Users/.../rollupify-poc'

but with this change, it works:

// ./people/bob.js
let text = require('./people/text.json');
export class Bob {
    wave() { console.log(text.hello); }
}

Which is not the expected, neither the convenient.

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson Sep 9, 2016

Thank you, that's a good point. I tended to assume while writing rollupify that folks would only use require() for external modules (e.g. require('inherits')) which of course works fine.

Could you please file that as an issue on rollupify?

Thank you, that's a good point. I tended to assume while writing rollupify that folks would only use require() for external modules (e.g. require('inherits')) which of course works fine.

Could you please file that as an issue on rollupify?

@drpicox

This comment has been minimized.

Show comment
Hide comment

drpicox commented Sep 9, 2016

Ok!

@snuggs

This comment has been minimized.

Show comment
Hide comment
@snuggs

snuggs Sep 9, 2016

@drpicox ping me on the issue. We'll get you situated. /cc @nolanlawson @substack

snuggs commented Sep 9, 2016

@drpicox ping me on the issue. We'll get you situated. /cc @nolanlawson @substack

@substack

This comment has been minimized.

Show comment
Hide comment
@substack

substack Nov 19, 2017

Collaborator

I think we can close this issue now thanks to the great work of @indutny and @goto-bus-stop with common-shake, browser-pack-flat, and common-shakeify.

Using a combination these tools:

you'll get tree shaking and dead code elimination.

Or you can use tinyify as a plugin which wraps up the aforementioned tools into an easy-to-use zero-configuration package:

browserify -p tinyify main.js

bankai includes these tinyify optimizations by default.

Collaborator

substack commented Nov 19, 2017

I think we can close this issue now thanks to the great work of @indutny and @goto-bus-stop with common-shake, browser-pack-flat, and common-shakeify.

Using a combination these tools:

you'll get tree shaking and dead code elimination.

Or you can use tinyify as a plugin which wraps up the aforementioned tools into an easy-to-use zero-configuration package:

browserify -p tinyify main.js

bankai includes these tinyify optimizations by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment