Default export added on *-imported CJS module causes problems

[marijnh]

This seems to only cause problems in a slightly convoluted setup, but here goes:

I have a.js which is:

import * as b from "./b"
window.B = b

With b.js itself being rollup output, something like this:

Object.defineProperty(exports, '__esModule', { value: true });
exports.x = 10

If I roll up a.js with this plugin enabled, this is what comes out:

'use strict';

function unwrapExports (x) {
	return x && x.__esModule ? x['default'] : x;
}

function createCommonjsModule(fn, module) {
	return module = { exports: {} }, fn(module, module.exports), module.exports;
}

var b = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, '__esModule', { value: true });
exports.x = 10;
});

var b$1 = unwrapExports(b);
var b_1 = b.x;


var b$2 = Object.freeze({
	default: b$1,
	__moduleExports: b,
	x: b_1
});

window.B = b$2;

Here b doesn't have a default export, but unwrapExports will dereference it anyway, since it has an __esModule property. Thus b$1, added as default on the final export object, is undefined.

Now if I try to use this as an external from a CJS file in another bundle, rollup will generate something like this for that import:

b = b && b.hasOwnProperty('default') ? b['default'] : b;

Which, since b has a default property which is undefined, will yield undefined.

I think the problem is in the code generating the unwrapExports call—that seems to be trying to add a 'fallback' default export in case this is a CommonJS module, but for an ES module that doesn't actually have a default export, no default export should be propagated.

Since you can't conditionally export stuff, and it looks like the information of whether there is an actual default export isn't available at the point where this code is generated (const defaultExport in transformCommonjs in src/transform.js), working around it at that point may be tricky.

Any ideas?

[marijnh]

Okay, a kludge that seems to work is to set defaultExport to the empty string when hasDefaultExport is false and this is an ES module. I've submitted #225 with such a patch, let me know whether it makes sense.

[marijnh]

So that PR was not right, and the problem I touched on (not knowing in advance whether you need an export default at all) seems to be a good diagnosis of the problem. As a crude workaround, we could change the invocation in the consuming process to ignore undefined default exports

b = b && b.hasOwnProperty('default') && b['default'] !== undefined ? b['default'] : b;

But technically, the default export is allowed to be undefined, so that's not great.

[Rich-Harris]

I believe we can close this now that 8.2.4 has been released — sorry for the wait