You have to make sure the module.exports = expression is at the file root.
https://github.com/zloirock/core-js/blob/master/modules/_redefine.js <- this file is getting transformed to invalid code:

(export default function(O, key, val, safe){
  if(typeof val == 'function'){
    val.hasOwnProperty(SRC) || hide(val, SRC, O[key] ? '' + O[key] : TPL.join(String(key)));
    val.hasOwnProperty('name') || hide(val, 'name', key);
  }
  if(O === global){
    O[key] = val;
  } else {
    if(!safe){
      delete O[key];
      hide(O, key, val);
    } else {
      if(O[key])O[key] = val;
      else hide(O, key, val);
    }
  }
})(Function.prototype, TO_STRING, function toString(){
  return typeof this == 'function' && this[SRC] || $toString.call(this);
});

Also if you do the export resolution after transforming the require calls, you could also optimize cases like this: https://github.com/zloirock/core-js/blob/master/modules/_html.js

module.exports = require('./_global').document && document.documentElement;

Right now its not treated as staticValue because of the require() call. After the require transform, it should end up being a staticValue.

Same for https://github.com/zloirock/core-js/blob/master/modules/_path.js

That is a really ridiculous case for why this optimization is needed. No matter which module bundler you use, that 35 byte file surely ends up taking up more than double that in any bundler other than rollup :-)

Indeed. Ideally a single

module.exports = require('...');

statement should transform to

export * from '...';
export { default } from '...';

I'll look into all these things. 👍

@Swatinem Mind taking another look?

One step further, but now I have two related issues.

The build now breaks because Error: Module …/core-js/modules/es5.js does not export default (imported by /tmp/1151026-32286-1ve7p9z.js)

Like I said, its 2 problems at once:

• The file really does not write to module.exports, so no intro/outro is generated and thus no export defaults there. (Maybe you could just give a export default {} in that case.
• The auto generated file (which is basically the same as https://github.com/zloirock/core-js/blob/master/shim.js) has a lot of plain require calls, so imports for side effects. rollup-plugin-commonjs actually turns those into import foo from "…"; foo; which then breaks.

@Swatinem I pushed a commit to handle bare requires just seconds before your comment. 😝

Yes, I’ve seen it. :-)

So compiling core-js works now. The generated bundle is ~20k smaller, but the minified version is ~1k larger for whatever reason.

I think it's because of how webpack wraps each CommonJS module. This is the only overhead that separates each module:

}, function (module, exports, __webpack_require__) {

// minified
},function(a,b,c){

Rollup has more overhead. This is it's current wrapper:

var main = (function (module) {
  var exports = module.exports;
  ...
  return module.exports;
})({exports:{}});

export default main;

Let's look at an example to see how this ends up:

// main.js
module.exports = require('./something.js').Foo;

// with Webpack
function (module, exports, __webpack_require__) {
  module.exports = __webpack_require__(5).Foo;
}

// with Rollup
var main = (function (module) {
  var exports = module.exports;
  module.exports = require$$0.Foo; // the import available through an identifier
  return module.exports;
}({ exports: {} );

// minified Webpack, ~35 bytes
function(a,b,c){a.exports=c(5).Foo}

// minified Rollup, ~75 bytes
var a=function(o){o.exports;return o.exports=r.Foo,o.exports}({exports:{}})

Yes I remember rollup used to have a thinner wrapper a few versions ago.

Not sure if @zloirock would actually want to reconsider switching to rollup if it could generate smaller minified bundles :-)

Yes I remember rollup used to have a thinner wrapper a few versions ago.

That's right. @Rich-Harris From this Git commit message it appears that the modules are wrapped in closures to allow more readable outputs; is that the only reason? The wrappers as they are now add quire a bit of overhead.

If they are necessary for some other reason, they can be further optimised. I could whip something up, but if the wrappers aren't even necessary, I'd prefer to just have them removed.

@Swatinem you can add PR when these optimisations will be done. Could be good optimise core-js load time for the next big release, but without moving from commonjs modules.

Rebased. Having removed the module wrapper named exports get a bit trickier. They can conflict with local variables and therefore must get other names (a -> export$a) that have to get reexported (export { export$a as a}). This would get mitigated by keeping the wrappers which may end up costing less than without.

Compare

var exports = {}, module = { exports: exports };

// code...

var export$a = module.exports.a;
var export$b = module.exports.b;

export default module.exports;
export { export$a as a, export$b as b};

with

var exports = wrapper(function (module,exports) {
  // code...
});

export default exports;
export var a = exports.a;
export var b = exports.b;

I've been trying to build this branch, but the npm run build script relies on a default config file that I'm not able to find (and neither is Rollup AFAICT). The build artifact is thus non-existant.

I'm tying to determine if this patch improves the situation here: https://github.com/pygy/trollup

Here's the resulting bundle.js:

'use strict';

var baz = 4;

var qux = (function (module) {
var exports = module.exports;
module.exports.qux = 5
module.exports.extra = 6
return module.exports;
})({exports:{}});

var qux = qux.qux;

exports.baz = baz;
exports.qux = qux;

There are three issues BTW, one is the lack of pruning of the extra field, the other is all the useless var exports definition, and the last is the presence of two var qux declarations, which, while not strictly buggy is a bit confusing.

At last, I got it to build. First time I try to install npm modules from git. cloning the plugin in the node_modules directory did the trick.

So, now, I get this:

'use strict';

var baz = 4;

var exports$1 = {};
var module$1 = { exports: exports$1 };
module$1.exports.qux = 5
module$1.exports.extra = 6
var qux$1 = exports$1.qux;

exports.baz = baz;
exports.qux = qux$1;

Which is definitely better :-)

No tree shaking yet for the extra field, though.

Finally got a chance to dig into this properly. Afraid everything got screwed up a bit with the various overlapping branches and this isn't mergeable – sorry – so I've been experimenting with reinstating some of these additions without the confusion caused by the back-and-forth over module wrappers.

To be honest, I think we might be going a bit overboard here. The purpose of this plugin isn't really to optimise CommonJS modules, it's just to allow people to use external CommonJS dependencies until everyone starts distributing ES6 modules. Adding so much complexity to massage down the filesizes of transpiled legacy modules makes me nervous – it will make future fixes and additions (say, if we decide we really need to support lazy inline require calls, or support some Browserify idiom like polyfilling Buffer) harder. It's likely to consume energy that would be better spent on improving Rollup itself and helping people adopt ES6 modules than on reducing the incentive to switch.

What does everyone reckon?

I agree. I admit that I got a bit overboard with this. I got enticed by the possibility of Rollup beating CommonJS bundlers at their own game. 😜

I'll experiment with optimising CommonJS elsewhere.

I’m sad to see this go. But you are absolutely right to rather promote es2015 modules than to optimize legacy module systems.