Skip to content
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

Investigate issues with Rollup? #1

Closed
trueadm opened this issue Nov 13, 2015 · 3 comments
Closed

Investigate issues with Rollup? #1

trueadm opened this issue Nov 13, 2015 · 3 comments

Comments

@trueadm
Copy link

trueadm commented Nov 13, 2015

Why are these issues with Rollup happening, specifically (taken from the readme by @dchambers):

  • I can't get any benefit from tree-shaking of NPM libraries because I can't enable the jsnext flag.
  • I can't have source-maps enabled because the CommonJS transformation plug-in doesn't support source-maps.

Maybe @Rich-Harris can shine some light on these?

@Rich-Harris
Copy link

Bundling CommonJS libraries isn't really Rollup's forte – compatibility with CommonJS (via rollup-plugin-commonjs) only really exists as a way to ease the transition to JavaScript modules from legacy formats.

That said...

Rollup isn't the fastest (WebPack is)

This is true if you're bundling CommonJS modules, because at present those modules have to be parsed twice (once by rollup-plugin-commonjs, once by Rollup itself). With ES6 modules, Rollup wins comfortably :-)

...and doesn't always produce the smallest bundles (for CommonJS, browserify does)

I added an uglify and gzip step to my fork of this repo. Results:

670K Nov 13 10:48 browserify.js
214K Nov 13 10:49 browserify.min.js
 58K Nov 13 10:49 browserify.min.js.gz
691K Nov 13 10:49 rollup.js
140K Nov 13 10:49 rollup.min.js
 40K Nov 13 10:49 rollup.min.js.gz
680K Nov 13 10:48 webpack.js
190K Nov 13 10:49 webpack.min.js
 55K Nov 13 10:49 webpack.min.js.gz

As you can see, Rollup produces dramatically smaller bundles. (I'm actually rather surprised by this, given that we're not exactly playing on Rollup's home turf! I think it's partly because Rollup isn't injecting a useless process shim. This highlights one difference between Rollup and Browserify/Webpack – it doesn't, at least currently, go to any effort to simulate a Node.js environment in the browser.)

I can't get any benefit from tree-shaking of NPM libraries because I can't enable the jsnext flag.

Unfortunately, Redux's jsnext:main points to its source code, not a distributable ES6 bundle. Because of that, the build process for this repo needs to transpile Redux's source code. Redux uses Babel 5, this repo uses Babel 6, causing the build to fail. These kinds of incompatibilities are why jsnext:main should not be used to point to source code. I'd be interested in poking at this a bit more to try and get it to work regardless, though note that you probably wouldn't gain much from tree-shaking in this situation anyway.

I don't think React exposes a jsnext:main.

I can't have source-maps enabled because the CommonJS transformation plug-in doesn't support source-maps

It does! You need to enable sourcemaps with the replace plugin too though. I'm thinking about encouraging all plugins to generate sourcemaps by default, to avoid this sort of thing – slight performance overhead but probably worth it.

@dchambers
Copy link
Contributor

@Rich-Harris, wow, thanks so much for the taking the time to provide such a comprehensive response.

Unfortunately, Redux's jsnext:main points to its source code, not a distributable ES6 bundle. Because of that, the build process for this repo needs to transpile Redux's source code. Redux uses Babel 5, this repo uses Babel 6, causing the build to fail. These kinds of incompatibilities are why jsnext:main should not be used to point to source code.

When I originally read your jsnext:main page I noticed that you pointed to an ES6 bundle in the example, but I thought this must be a mistake. I imagine the Redux devs came to the same conclusion, so it's probably worth updating the page to explain that this is intentional and why it needs to be done that way, as you have here.

It does! You need to enable sourcemaps with the replace plugin too though. I'm thinking about encouraging all plugins to generate sourcemaps by default, to avoid this sort of thing – slight performance overhead but probably worth it.

Good to know! I found the whole process of setting up the rollup config to support real-word NPM libraries involved quite a lot of jumping between different projects for code config snippets. Although I understand that the rollup plugin system is flexible enough to support other use cases, it might be worth having an example config block for NPM users that saves people the hassle of figuring it out themselves, and then hassling you when they don't get it quite right ;-).

@Rich-Harris
Copy link

Yes, that's probably a good idea – there's a lot of confusion around jsnext:main (and I've probably contributed to it...) and I'll try and get round to making that clearer in the wiki.

Ditto for the npm use case! There's a placeholder page here – https://github.com/rollup/rollup/wiki/Bundling-CommonJS-modules – that I keep meaning to flesh out with a starter repo etc. On the sourcemaps front, I've updated the existing plugins so that they generate sourcemaps automatically, which should reduce friction a bit.

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

No branches or pull requests

3 participants