Build with babel 6

[peterdenev]

Build with babel 6. Tested with rollup, babel 6, decorators-legacy

[jayphelps]

awesome! 🎉 I prefer the single line re-export instead of the more verbose two step process, so if you could switch it back, this otherwise will be good to go.

[peterdenev]

Hi, I prefer the single line too, but after babel process the file can not be read properly from rollup.

[jayphelps]

@peterdenev can you clarify? Are you saying that consumers of the CJS output, using rollup, have some sort of error? what error? Is this known upstream by the rollup maintainers?

[peterdenev]

babel 6 compile the single line import/export like this:

//...
var _override = require('./override');

Object.defineProperty(exports, 'override', {
  enumerable: true,
  get: function get() {
    return _interopRequireDefault(_override).default;
  }
});
//...

and the result can not be read properly from export variable.

Old babel 5 (your current version) compiles to something like:

//...
var _override = require('./override');
exports.override = _interopRequire(_override);
//...

If we make the import and export separated the compiled (babel 6) will be like:

//...
var _override = require('./override');
var _override2 = _interopRequireDefault(_override);
//...

As you see the result will be more like the current version and it is clearer. So the problem is not in the rollup, so the babel transformation. If you have other questions, just ask.

[jayphelps]

It's my understanding that you must use rollup-plugin-commonjs with CJS modules you import. So it seems like the problem is that it's not currently smart enough to autodetect these because they're not simple exports.foo = value. Instead, people would need to explicitly declare the expected exports via namedExports config.

If this is all true, it's quite a bummer. I'd really rather not sacrifice our code quality to work better with rollup users, which I imagine is a fairly small set of users. That said, I'm not writing it off outright. We'd want a heft comment inside that file explaining why we're not directly re-exporting, otherwise I'll forget or someone else will come along and change it unknowingly.

I'll also take a look at the rollup-plugin-commonjs source to see if it can be trivially fixed to support this case. No promises, but at first glance, with an ast it should be fairly easy to programmatically detect--but there may be a good reason it doesn't yet.

[jayphelps]

I might be more inclined to output an ES modules-only build, that rollup can use directly instead of the CJS modules build.

[peterdenev]

OK. Let's leave the main loader file as in the origin one. I will investigate my case externally.

[jayphelps]

Thanks again for this PR! We ended up going with #106 since it had the ES modules stuff as well. I really appreciate you taking the time though!