Bundling pre-ES6 modules about rollup HOT 36 CLOSED

I've gone back and forth on this. For the time being I think it's better to focus solely on ES6 modules, at least until we iron out all the kinks, but long term we definitely need a sane way to handle legacy module formats.

In theory, it can be done by overriding the resolveId function to accept CommonJS modules, and adding a transform step that wraps or rewrites modules so that Rollup can analyse them. To what extent you can convert CommonJS etc, I don't know - in a lot of cases it'd be straightforward, but I've no idea how you'd handle things like this (it's not quite as contrived as it looks)...

var myLib = exports;
myLib.foo = 'foo';

augment(myLib);
function augment(lib) { lib.bar = 'bar'; }

...so a strategy of wrapping rather than rewriting is probably better.

Luckily @guybedford has spent a lot of time thinking about these problems, and has done some initial research on incorporating Rollup's strategy inside JSPM: systemjs/builder#199. This seems like the best route forward, since JSPM is a soup-to-nuts solution (repository, bundler, module loader polyfill etc all talking the same language) that already handles legacy formats.

For right now though, my workaround is to have a two-phase bundling process, exactly as you suggest. First, I bundle my app code (plus any npm depedencies that expose jsnext:main, i.e. anything I wrote myself 😆) as CommonJS, and then I pass that bundle over to Browserify (but could be Webpack or whatever), which takes it from there. It works perfectly well, it's just a PITA to have to juggle all these different tools which is why I'm excited to see where JSPM goes.

from rollup.

Actually, @callumlocke, if you want an to import lodash into your ES6 project, you can just include the ES6 build of lodash. On npm it's called lodash-es. It has a "jsnext:main" field defined, so it should be ready to go. I've been able import it in my bundles, and once #47 is approved, the only hiccup I've encountered with it so far should be resolved.

from rollup.

I'm pretty confused, does this replace Webpack? Thanks!

from rollup.

The wiki says it better than I can.

from rollup.

@Rich-Harris:

First, I bundle my app code (plus any npm depedencies that expose jsnext:main, i.e. anything I wrote myself 😆) as CommonJS, and then I pass that bundle over to Browserify (but could be Webpack or whatever), which takes it from there.

That sounds good! Just to clarify, with this rollup-then-browserify approach, would your source code contain contain a mixture of import statements and require calls? ...i.e. use import for your own local ES6 modules AND for third party modules that expose a jsnext:main, but use require for everything else)?

from rollup.

@callumlocke you could, though more likely you'd write your app code all as ES6...

import external from 'third-party';
import foo from './my-utils';

/* ... *.

...and create a CommonJS bundle with Rollup, specifying which modules are external, then hand it over to Browserify:

rollup --input main.js\
       --format cjs\
       --external third-party,other-third-party\
       --output tmp/bundle.js

browserify tmp/bundle.js > dist/bundle.js

That's what I do, anyway!

from rollup.

Ahh ok, I did try that at first, but rollup errored out when it failed to resolve a third party import (which didn't have a jsnext:main), because I wasn't using the cjs format.

So basically the cjs format means: rollup will bundle whatever it can into the file, but for unresolvable imports it will just convert the import statement into a require call. Is that a good summary?

from rollup.

@callumlocke Yes, that's correct. What Rollup can't pull into the bundle will be "imported" using the idioms of the target module format.

// ES2015
import * as bar from './bar.js';

// CommonJS
var bar = require( './bar.js' );

// AMD
define(['./bar.js'], function (bar) {
  ...
});

// IIFE, use the `globals` option to tweak global names
(function (bar) {
  ...
)(bar);

And UMD being a combination of the last three.

from rollup.

Hello @Rich-Harris (and others), Thank you for rollup. I'm using it in two of my new project.

Related to this issue. I'd really like to use rollup to pull in a few small helper utilities when building for the browser (for example a Object.assign shim and node.js util's isUndefined). Rollup seems ideal for this. Since rollup currently does not support importing from CJS modules what's the suggested temporary fix? The easy thing to do is copy and paste. We could also fork and convert a few core CJS modules to ES6 (https://github.com/defunctzombie/node-util/blob/master/util.js for example). Or wait for something else (e.g. future version of SystemJS builder using rollup).

I should add while I love JSPM for building apps I don't want to use JSPM (or Browserify) for building modules at this point. Rollup is much cleaner.

Edit: I also saw what you were doing with the *-jsnext repos.

from rollup.

...and create a CommonJS bundle with Rollup, specifying which modules are external, then hand it over to Browserify:

rollup --input main.js\
       --format cjs\
       --external third-party,other-third-party\
       --output tmp/bundle.js

browserify tmp/bundle.js > dist/bundle.js

Could there be a new option like --auto-externals, which would tell rollup to assume I want to treat any unresolvable imports as external?

from rollup.

@Victorystick:

Actually, @callumlocke, if you want an to import lodash into your ES6 project, you can just include the ES6 build of lodash. On npm it's called lodash-es. It has a "jsnext:main" field defined, so it should be ready to go. I've been able import it in my bundles, and once #47 is approved, the only hiccup I've encountered with it so far should be resolved.

Importing an individual method using e.g. import {zipObject} from 'lodash-es'; doesn't work (error: Module ~/foo/node_modules/lodash-es/lodash.js does not export zipObject).

Looks like this is because the module doesn't export zipObject from './array/zipObject'. Instead it just exports one big default (the entire lodash library). Is this going to be changed in lodash-es? Or is this a limitation of rollup, that it doesn't yet know how to grab a single property off a big default export in the absence of a named export?

from rollup.

@callumlocke If you look, the main entry point for the lib just imports a bunch of smaller modules. What I've been doing (if I don't want to import the entire gigantic library) is just import the modules I want:

import findIndex from "../lib/lodash/array/findIndex";

It's been working great for me.

from rollup.

@Hypercubed

Since rollup currently does not support importing from CJS modules what's the suggested temporary fix?

Depending on how many helpers we're talking about, copy and paste might be the best option, if it's the difference between being able to keep the bundle entirely inside Rollup and having to pass it off to Browserify for a second step. Though I'm still wondering about the possibility of somehow transforming CommonJS modules into ES6 during the transformation phase.

Long-term though, the best option is to start creating ES6 modules and open sourcing them! 😀

@callumlocke

Could there be a new option like --auto-externals

Maybe... with Esperanto (Rollup's predecessor) I found that being permissive caused more problems than it solved (e.g. if you got the wrong number of ../s when importing ../../utils/foo, it would assume you were referencing an external module), though if it was an option perhaps that would be okay. I typically use the JS API rather than the CLI, and do this sort of thing:

{
  entry: 'app.js',
  format: 'cjs',
  external: (function () {
    var dependencies = require( './package.json' ).dependencies;

    return Object.keys( dependencies ).filter( function ( name ) {
      return !require( name + '/package.json' )[ 'jsnext:main' ];
    });
  }())
}

An option would be simpler.

Importing an individual method using e.g. import {zipObject} from 'lodash-es'; doesn't work

That's a shame. It would be great to be able to use lodash as a 'true' ES6 module and get the tree-shaking benefits without having to remember (or lookup) the fact that zipObject lives in the array folder. There would be some complicating factors, e.g. chaining would only work if you included the whole library (because everything needs to live on lodash.prototype), so it would actually be a different library. You'd have to ask @jdalton if he has any plans in that direction!

Or is this a limitation of rollup, that it doesn't yet know how to grab a single property off a big default export

It's more a limitation of static analysis in as dynamic a language as JavaScript, unfortunately.

@timshannon FYI in more recent versions of Rollup, if you have lodash installed as an npm module, you can do this if it's easier:

import findIndex from "lodash/array/findIndex";

from rollup.

@timshannon FYI in more recent versions of Rollup, if you have lodash installed as an npm module, you can do this if it's easier:

import findIndex from "lodash/array/findIndex";

This is how I've been doing it. Until lodash handles is exports properly.

from rollup.

Totally open to getting this supported. I've been kicking around a work-in-progress with @megawac in the es branch. I think he ran into rollup issues so we kind of hit a wall. If you have pointers or suggestions to get it straightened out that'd be great.

from rollup.

For those interested, here is the test case I whipped up to test rollup with
lodash-es https://github.com/megawac/rollup-lodash-es-test

On Mon, Oct 12, 2015 at 11:33 AM, John-David Dalton <
[email protected]> wrote:

Totally open to getting this working. I've been working with @megawac
https://github.com/megawac in the es branch
https://github.com/lodash/lodash/tree/es. I think he ran into rollup
issues so we kind of hit a wall. If you have pointers or suggestions to get
it working that'd be great.

—
Reply to this email directly or view it on GitHub
#45 (comment).

from rollup.

@jdalton Awesome! It might be as straightforward as creating a similar build that cuts everything except the export block from lodash.js, though that might be based on a superficial understanding.

I could try throwing a lodash-jsnext project together (similar to d3-jsnext and three-jsnext) to test out this hypothesis, if that's helpful?

@megawac Thanks. It's likely that the example-main.js version is pulling in all the extraneous clutter because of everything that needs to get added to lodash.prototype, and there are probably a few things we're including out of an abundance of caution (#179 is designed to eliminate some of those cases). A non-chainable version of the library (i.e. where there's no lodash function exported as default) would probably go a long way towards leaner builds.

from rollup.

@Rich-Harris nice snippet, I'll use that at some point. I can see the problems with doing it magically. An alternative would be for rollup not to error out straight away, but carry on building the import tree, and then throw one big error at the end with a message listing all of the imports that failed due to a lack of jsnext:main. Then I could just copy and paste this list into my own --externals list (after skimming it and making sure I'm happy with it). It might sound like a small thing, but anything that makes it easier to try out rollup on a large existing codebase could be good for adoption!

from rollup.

I'll try adding extra foo.default module versions of foo modules to better isolate them.

from rollup.

We're making some progress here as of the 0.20.0 release and the rollup-plugin-commonjs!

from rollup.

@Rich-Harris @megawac

Awesome! It might be as straightforward as creating a similar build that cuts everything except the export block from lodash.js, though that might be based on a superficial understanding.

I've updated the es branch.

from rollup.

I've updated the es branch.

That's beautiful! 😄

from rollup.

@jdalton nice commit!

I've done some experiments, and it is now possible to import individual functions using the nicer syntax: import {foo, bar} from 'lodash-es';

But it looks like rollup's tree-shaking doesn't work with this at all – you still need to import from individual function files if you want an optimal bundle. Not sure if this is a problem with rollup or a problem with the way lodash-es is organised. @Rich-Harris?

The following experiments were done with rollup --format=iife using rollup v0.19.2.

Importing from lodash-es main

import {chunk, zipObject} from 'lodash-es';

console.log(zipObject(chunk(['a', 'b', 'c', 'd'], 2)));

Result: ~429kB

Importing from categories

import {chunk, zipObject} from 'lodash-es/array';

console.log(zipObject(chunk(['a', 'b', 'c', 'd'], 2)));

Bundle size: ~92kB

Importing individual functions

import chunk from 'lodash-es/array/chunk';
import zipObject from 'lodash-es/array/zipObject';

console.log(zipObject(chunk(['a', 'b', 'c', 'd'], 2)));

Bundle size: ~4kB

from rollup.

Excited about the new Lodash build. I haven't tried it yet, but my hunch is that unnecessary code is being included because Rollup is being overly cautious about side-effects (see #179). If that's the case, this gives us a powerful incentive to get that issue resolved...

from rollup.

I haven't tried it yet, but my hunch is that unnecessary code is being included because Rollup is being overly cautious about side-effects

Maybe further down the dep chain.
The lodash module and category modules, as they are in edge, have no function calls.

from rollup.

I've updated the es branch to work with Babel 6.

from rollup.

Is there something I can do with how I generate lodash-es modules to make tree-shaking easier?

from rollup.

@jdalton Finally had a chance to play around with this a bit more. The tl;dr is that for the time being, Lodash users should continue to import individual files:

import zipObject from 'lodash-es/array/zipObject.js';

There definitely isn't a silver bullet in terms of small changes to Lodash that would no longer trigger Rollup's over-eager side-effect detection. There are lots of small things like this...

/** Used to resolve the decompiled source of functions. */
var fnToString = Function.prototype.toString;

/** Used to check objects for own properties. */
var hasOwnProperty = objectProto.hasOwnProperty;

/** Used to detect if a method is native. */
var reIsNative = RegExp('^' +
  fnToString.call(hasOwnProperty).replace(reRegExpChar, '\\$&')
  .replace(/hasOwnProperty|(function).*?(?=\\\()| for .+?(?=\\\])/g, '$1.*?') + '$'
);

...that add up to a lot of 'unnecessarily' included code. (In this case, even though Rollup is smart enough to realise that calling RegExp won't have any side-effects other than to assign a value to reIsNative, it panics around fnToString.call(...) and the .replace(...) calls, because it doesn't know what methods it's calling – it could be mutating an object called fnToString, for example.)

Beyond that, side-effects multiply – once Rollup thinks it needs to include one chunk of code, that has cascading effects in terms of what other bits of code it thinks it need to include. It's possible to be a lot more ruthless, but not without risking breaking things.

So while there are things that Lodash could do to make things marginally easier static-analysis wise, I'm hesitant to suggest them – they would probably involve rather large changes and it still wouldn't be as efficient as importing directly from individual files.

Having said that, most of the false positives appear to fall into two or three categories. I'm very hopeful that we'll be able to introduce some much more sophisticated static analysis that would make Rollup much less over-cautious:

• we could track values as they're assigned to variables. While there are lots of cases where we obviously can't reliably do this statically, there are lots of places where we can – for example, if we know that the value of the isArray declaration is Array.isArray, we can be satisfied that any calls to isArray don't have side-effects
• by the same token, in the example above, if we know that the value of fnToString is Function.prototype.toString, and that no-one has assigned a call (or computed) property to it, then we also know that fnToString.call(...) has no side-effects and returns a string. We can also know that calling .replace(...) on that string has no side-effects (certainly if the second argument is not a function) and returns another string. In aggregate, we could determine statically that the code in the sample above does not have side-effects, and therefore needn't be included unless we're using reIsNative
• Rollup is also very cautious about functions that potentially mutate their arguments. In many cases it needn't be, especially if we're able to track values

The good news is that Lodash is an absolute goldmine in terms of battle-testing some of these theories.

from rollup.

@Rich-Harris

Finally had a chance to play around with this a bit more. The tl;dr is that for the time being, Lodash users should continue to import individual files:

import zipObject from 'lodash-es/array/zipObject.js';

Thanks for digging in! All of this side-effects detection is way more complicated than I thought the rollup detection would be. Is there a way to just detect imports of each module to build a tree of used modules and then know which to include in the bundle?

In the case of lodash-es import {zipObject} from 'lodash-es' looks to an
export {default as zipObject} from './array/zipObject' of lodash.js which looks to array/zipObject which has import baseSet from '../internal/baseSet' which then looks to internal/baseSet, and so on.

from rollup.

Perhaps custom compiler metadata could be provided with an option or hook
input to indicate which modules are side effect free to enable more
agressive optimization here?

On Thu, 31 Dec 2015 08:51 John-David Dalton [email protected]
wrote:

In the case of lodash-es import { zipObject } from 'lodash-es' ties to an export
{ default as zipObject } from './array/zipObject' of lodash.js which
looks to array/zipObject which has import baseSet from
'../internal/baseSet'; which then looks to internal/baseSet, and so on.

—
Reply to this email directly or view it on GitHub
#45 (comment).

from rollup.

I like Guy's idea here. While we can try to statically analyze to determine
the answer, it eventually boils down to needing to run the code (which, if
you want to be reeeaaally crazy we could do in a sandbox 😉).
On Thu, Dec 31, 2015 at 1:18 AM Guy Bedford [email protected]
wrote:

Perhaps custom compiler metadata could be provided with an option or hook
input to indicate which modules are side effect free to enable more
agressive optimization here?

On Thu, 31 Dec 2015 08:51 John-David Dalton [email protected]
wrote:

In the case of lodash-es import { zipObject } from 'lodash-es' ties to
an export
{ default as zipObject } from './array/zipObject' of lodash.js which
looks to array/zipObject which has import baseSet from
'../internal/baseSet'; which then looks to internal/baseSet, and so on.

—
Reply to this email directly or view it on GitHub
#45 (comment).

—
Reply to this email directly or view it on GitHub
#45 (comment).

from rollup.

All of this side-effects detection is way more complicated than I thought the rollup detection would be. Is there a way to just detect imports of each module to build a tree of used modules and then know which to include in the bundle?

That's actually how early versions of Rollup worked – it wouldn't even load a module until one of its exports was referenced by code that was directly reachable from the entry point. Unfortunately that turns out to be problematic in cases like this – the config.asap = async line gets dropped unless you analyse that code and determine that it affects the config object, which changes how defer behaves. We see similar things in D3.

Perhaps custom compiler metadata could be provided with an option or hook input

It's certainly an idea worth exploring. For a brief moment there was an aggressive option that didn't worry about side-effects, but it proved too failure-prone. The more we can do via static analysis the better, but a combination of approaches might be better still.

from rollup.

For a brief moment there was an aggressive option that didn't worry about side-effects, but it proved too failure-prone.

The lodash-es case is probably as safe as you're going to get.
All method dependencies are tracked to generate builds (AMD, ES, Node, etc) based on it.

In the meantime there's always babel-plugin-lodash to assist.
It's also great combo'ed with lodash-webpack-plugin.

from rollup.

hey @jdalton, what's the latest status of lodash-es and babel-plugin-lodash? What's the best practice at the moment for bundling lodash with rollup for clientside code (if you want to make sure only a minimal set of lodash functions make it into the bundle)?

from rollup.

@callumlocke lodash-es and babel-plugin-lodash are trucking along. The FAQ covers es6 to es6 transforms but if you'd like to contribute an example working with rollup that would be great.

from rollup.

Going to close this as it's digressed a long way from where it started – bundling CommonJS now works reasonably well in most cases and there are other issues relating to tricky tree-shaking cases.

from rollup.