[@rollup/plugin-commonjs] Cannot handle CJS -> ESM imports

[FredKSchott]

• Rollup Plugin Name: @rollup/plugin-commonjs
• Rollup Plugin Version: Latest
• Rollup Version: Latest
• Operating System (or Browser): MacOS, Repl.it
• Node Version: v12.16.3

How Do We Reproduce?

1. Run: https://repl.it/repls/WorthyPrevailingOutcomes
2. This is a simplified version of what's happening when you try to bundle the "styled-normalize" package.

Expected Behavior

css and createGlobalStyle are imported correctly.

Actual Behavior

(Looking at output/bundle.js) css and createGlobalStyle are being pulled off of the default export (any empty object) instead of the module namespace (where they exist as exports).

See: https://www.pika.dev/npm/snowpack/discuss/194

[shellscape]

@FredKSchott would love to review a PR for this. AFAIK we have no active maintainers for commonjs other than @danielgindi who has occasional time on the plugin.

[danielgindi]

Also - when importing the real styled-components - it chooses the esm version and uses the import statement, no synthesizing around cjs.

[FredKSchott]

no problem, happy to clarify:

1. The value of css could be anything, the problem being illustrated is that the import of css is broken (undefined) in the output regardless of value.
2. Yup, I mimic styled-component’s ESM module entrypoint here. The problem is the broken import when styled-normalize imports the ESM styled-components code.

[danielgindi]

Okay really get what you said in the issue description now. The css and the createGlobalStyle are being read from the default (module.exports.default) instead of from module.exports.

I think that this behavior is expected. That the __esModule compatibility flag between cjs and esm is meant to declare that there's a default to import, and when require is being used - you have nothing you can do about it.
It would be really difficult to know whether we have to import the exports part or the exports.default part. Complex to the point where it's not worth it and I've seen modules just double-export their stuff over the default object to be somewhat compatible with cjs.

You can read about some of the issues with this interop/hack here: webpack/webpack#7973
Or here: https://medium.com/@dlmanning/interoperability-between-es-modules-and-common-js-is-a-mistake-fb9ac71d96fd

And I'm mentioning webpack, because we are following the same interop standard that babel have founded.

In theory, if you used let { css } = require('styled-components') we should be able to detect that we want a named import here, and do a different import. But this will possibly break existing code depending on this distortion.
If I get some free time I might take this as a daily quiz and perhaps solve this.

To sum up:
We detect whether the module has a default export - if so - we import that into our CJS module.
If it does not - we import the collection of named exports.
It's either or, we can't have both.
We may have a way to solve this in the future, using destructuring from a require call, but in it's current form, your code is ambiguous and has no good solution but the backwards-compatible solution that babel has standardized.

I think that this can be closed.

[danielgindi]

Apparently it challenged me and I just had to try it. Dammit. So there's a PR now.

[FredKSchott]

Hahaha nice!

[danielgindi]

@FredKSchott I'm f'ed up like that... ;-)

[marvinhagemeister]

I think I've been running into the same issue this morning. Made another repro case where the named exports are pulled off of the default: https://repl.it/@marvinhagemeist/UntidyWiryConditionals

Actual:

var styledComponents = require('styled-components');
var styledComponents__default = _interopDefault(styledComponents);

// ... snip

exports.reset = styledComponents__default.css(`
  * { box-sizing: border-box; }
`);

Expected:

var styledComponents = require('styled-components');
var styledComponents__default = _interopDefault(styledComponents);

// ... snip

exports.reset = styledComponents.css(`
  * { box-sizing: border-box; }
`);

[lukastaegert]

I started #481 to move all interop issues including this one forward.