fix(commonjs): convert module.exports with __esModule property(#939)

[githoniel]

Rollup Plugin Name: commonjs

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers: #939

Description

transform commonjs as module.exports = { __esModule: true, default: ... } to wrap runtime code

[guybedford]

@githoniel as well as the unit test here, could you include a full end-to-end test of the original use case provided in #939?

It looks like the output of that test is currently:

{ default: 'symbol', __esModule };
'symbol2'

Can you clarify what the output of that same test becomes with this PR?

[githoniel]

@guybedford thank for your time.

Look at this https://replit.com/@githoniel/rollup-plugin-repro-1 or the linked issue #939

many old package include babel-runtime publish its code like below

module.exports = { "default": require("core-js/features/object/get-own-property-descriptor"), __esModule: true };

import it will get an object as { "default": getOwnPropertyDescriptorFn, __esModule: true } with commojs plugin but should get getOwnPropertyDescriptorFn itself, it is not wrapped by

export function getDefaultExportFromCjs (x) {
    return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;
}

I have add test sample for it

[guybedford]

One issue here is this is the opposite interop pattern to Node.js itself... making this plugin unsuitable for building Node.js applications.

[githoniel]

any idea？shall we add option like target: 'browse' | 'node'? or detect environment at runtime？

[guybedford]

@githoniel I believe this is supposed to be controled by the defaultIsModuleExports configuration option - https://github.com/rollup/plugins/tree/master/packages/commonjs#defaultismoduleexports. So this PR should be sure to follow the semantics of that option as well I think.

[githoniel]

@githoniel I believe this is supposed to be controled by the defaultIsModuleExports configuration option - https://github.com/rollup/plugins/tree/master/packages/commonjs#defaultismoduleexports. So this PR should be sure to follow the semantics of that option as well I think.

I have make it work and update new tests

[guybedford]

Great work checking these cases, thanks for the PR.

[guybedford]

I know the prettier issue is unrelated, but are you able to get the CI to pass on this PR?

[guybedford]

Finally passing - thanks again for your work on this.