fix(compiler): Force rollup to use named exports

[pmdartus]

Details

This PR forces rollup bundle generation to use named exports.

Background

Currently, rollup attempts to detect how exports should be generated based on the exported values from the LWC module entry.

1. If there is a default export, export the default property as-is.
2. If there is a named export (named), rollup returns an object with the named property.
3. If there are both a default export and a named export (named), rollup returns an object with the default and the named property. It's important to note that if no output.exports property is set, rollup warns in this case.

On the other hand, when importing the default value from an external module, rollup injects some interop code extract the code right default value. This interop checks the presence of the default property on the exported object using module.hasOwnProperty('default').

We recently ran into a case where the interop code throws an error when the interop code throws an error because hasOwnProperty is not defined on the external module. This issue happens only if a module (module/a) has a single default export and the default export value has no prototype. In this case, if another module (module/b) imports the default export the interop code will throw because hasOwnProperty is not defined on the object.

/* `module/a` original code */
export default { __proto__: null, name: 'a' };

/* `module/a` generated code */
define('module/a', function () { 'use strict';
	var main = { __proto__: null, name: 'a' };
	return main;
});

/* `module/b` original code */
import a from 'module/a';
console.log(a.name);

/* `module/b` generated code */
define('module/b', ['module/a'], function (a) { 'use strict';
	a = a && a.hasOwnProperty('default') ? a['default'] : a;
	      // ^ throws here because a does inherit from object!
	console.log(a.name);
});

By forcing the rollup options.exports flag to be named, modules with a single default export (case 1.) to return an object with the default property. The generated output for cases 2. and 3. remain identical.

/* `module/a` generated code with options.named="named" */
define(['exports'], function (exports) { 'use strict';
	var main = { __proto__: null, name: 'a' };
	exports.default = main;
	Object.defineProperty(exports, '__esModule', { value: true });
});

Using this approach we can guarantee that the exported returned object inherits from Object and will not throw when invoking hasOwnProperty.

Does this PR introduce breaking changes?

• ✅ No, it does not introduce breaking changes.

The PR fulfills these requirements:

• Have tests for the proposed changes been added? ✅
• Have you followed these instructions to clearly describe the issue being fixed or feature enhanced? ✅

GUS work item

W-7272006, W-7352554

[caridy]

Although I'm fine with this change, and I think it does make sense, there are other issues to consider. Locker being one of them, e.g.: locker has to sign and loop through the exports if needed (with default export only, everything is quicker.

[dgitin]

We (me and @jdalton) were looking at the changes in rollup and it seems there is quite a bit of history behind rollup's interop code but we don't think it's controversial to fix this in rollup code. In this case this could be a path worth exploring rather than making changes in 2 of our stacks (LWC & Locker). Have we considered this option?

History of changes possibly related the current issue
2019 changes https://github.com/rollup/rollup/pull/3084/files
2017 changes https://github.com/rollup/rollup/pull/1487/files

Possible fix could be to use Object.prototype.hasOwnProperty.call(....) in rollup interop layer.

cc @caridy @pmdartus

[pmdartus]

locker has to sign and loop through the exports if needed

I don't think the performance hit is a concern, in most of the case only a small amount of properties are exported outside a module. Also, the locker signature should only happen once at evaluation time.

Possible fix could be to use Object.prototype.hasOwnProperty.call(....) in rollup interop layer.

This option has been considered. @jodarove proposed himself to do that fix, but I saw that @jdalton just opened a PR for this: rollup/rollup#3420.

---

While I think it's import to fix rollup, forcing the named export out of the compiler, would allow us to remove unnecessary code that we had to introduce into Aura. I am fine postponing this change to the next release, but it would be great to tackle this as soon as possible next release.

[caridy]

Next release sounds fine, we can coordinate between the two teams.

[apapko]

@pmdartus can you please rebase and merge

[khangsfdc]

This would also prevent recursive/circular dependency issue for default only exports: rollup/rollup#3384

[pmdartus]

I am done with the rebase. @apapko @jodarove could you review the PR?

[jodarove]

looks good! @pmdartus pls remember to update lwc-platform and cut a release