[v3.0] Improve interop defaults

[lukastaegert]

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:

Description

The goal of this PR is to improve the defaults of the swamp that is "interop", i.e. how ESM input behaves when converted to and thrown into a CJS setting.

In Rollup, this is controlled by three options:

• output.interop: How to find the default import and handle namespace imports of external dependencies
• output.exports: What to do with default exports in your entry points
• output.esModule: Whether to add the "__esModule" special property as a hint when generating non-ESM output

The overall goal is to change the defaults so that Node’s interop behaviour is more or less the default behaviour. In Node

• The default export of a CommonJS module is module.exports
• Named exports are properties of module.exports (if Node detects them. But I think it should be fine if Rollup handles all named exports, otherwise we would need to replicate the exact detection algorithm of NodeJS, which I would want to avoid)
• __esModule is a regular export and not added by default
• You can check if a namespace import was from an ES module by checking namespace[Symbol.toStringTag] === 'Module'

This means I want to implement the following:

• The new default value for output.interop is "default"
• The output.interop values true and false are deprecated. However, I think the "duck-typing" interop of true is still very useful (i.e. for the default export, we check if there is a default property on module.exports first and return that, otherwise we return module.exports) because it works very well across tools with the only downside that it does not support "default" as a regular property on module.exports. Therefore, it will make a return with a new name, probably "compat".
• output.exports will remain as it is and default to "auto", which corresponds well to Node's interop where module.exports is the default import.
• output.esModule will receive a new value "if-default-prop" which will become the new default and only add the flag if there is a default export and it is actually a property "default" on module.exports. This situation cannot be mapped to CommonJS in a way that would work with Node interop anyway and users already receive a warning. This should reduce the proliferation of __esModule properties in libraries.
• When the default export is a property "default" on module.exports, we additionally set module.exports[Symbol.toStringTag] to "Module" to promote this interop mechanism (if Symbols are permitted)
• For internal namespace object, we improve how checks for Symbol.toStringTag are handled so that they can be resolved statically
• We ensure that Symbol.toStringTag is added/handled for external namespaces. Check what Node does for namespace imports from CJS

cc @guybedford who may have an opinion on this (and of course everyone else is invited to give feedback as well).

[github-actions]

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#improve-interop

or load it into the REPL:
https://rollupjs.org/repl/?pr=4611

[codecov]

Codecov Report

Merging #4611 (2b0f991) into release-3.0.0 (eac06ac) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@              Coverage Diff               @@
##           release-3.0.0    #4611   +/-   ##
==============================================
  Coverage          98.97%   98.98%           
==============================================
  Files                211      211           
  Lines               7428     7451   +23     
  Branches            2101     2104    +3     
==============================================
+ Hits                7352     7375   +23     
  Misses                23       23           
  Partials              53       53           

Impacted Files	Coverage Δ
src/ast/nodes/shared/Expression.ts	100.00% <ø> (ø)
src/finalisers/index.ts	100.00% <ø> (ø)
src/Chunk.ts	100.00% <100.00%> (ø)
src/ast/nodes/ImportExpression.ts	100.00% <100.00%> (ø)
src/ast/nodes/MemberExpression.ts	100.00% <100.00%> (ø)
src/ast/nodes/shared/knownGlobals.ts	100.00% <100.00%> (ø)
src/ast/utils/PathTracker.ts	100.00% <100.00%> (ø)
src/ast/variables/GlobalVariable.ts	100.00% <100.00%> (ø)
src/ast/variables/NamespaceVariable.ts	100.00% <100.00%> (ø)
src/finalisers/amd.ts	100.00% <100.00%> (ø)
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[guybedford]

Great to see this work, thanks for putting thought to these important defaults for Rollup module output to be as compatible as possible within the ecosystem.

output.exports will remain as it is and default to "auto", which corresponds well to Node's interop where module.exports is the default import.

Excellent! Then we just need Babel to do this as well now! //cc @nicolo-ribaudo

For if-default-prop, am I right in thinking this itself only ever applies to the interop auto mode? If so, perhaps we should validate this option to throw an error if the output.interop is not auto, to avoid weird compat cases?

Finally it's now worth also keeping an eye on compat with cjs-module-lexer at this point, to avoid possible named exports regressions against Node.js. A simple approach might be to include a test that runs cjs-module-lexer on Rollup output so that future changes don't break named exports detection by mistake.

[lukastaegert]

For if-default-prop, am I right in thinking this itself only ever applies to the interop auto mode

No, it also applies to named export mode when there is a default export. Basically it covers exactly the case that cannot be mapped to NodeJS interop.

[lukastaegert]

A simple approach might be to include a test that runs cjs-module-lexer on Rollup output so that future changes don't break named exports detection by mistake.

PR welcome! But I do not think it is critical for Rollup 3 as neither Rollup 3 nor this PR change how exports themselves are written, they only change import interop and the question whether to add the __esModule flag.

[guybedford]

Agreed, if there is no output change it is unrelated to this PR, but worth bearing in mind for future changes and not forgetting about while on this topic!

[lukastaegert]

This is now ready for merge. As one feature, accesses to Symbol.toStringTag on namespaces can now be resolved statically, avoiding the need for a dynamic namespace: https://rollupjs.org/repl/?pr=4611&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMColMjBhcyUyMGZvbyUyMGZyb20lMjAnLiUyRmZvby5qcyclM0IlNUNuJTVDbmlmJTIwKGZvbyU1QlN5bWJvbC50b1N0cmluZ1RhZyU1RCUyMCUzRCUzRCUzRCUyMCdNb2R1bGUnKSUyMGNvbnNvbGUubG9nKCdPSycpJTNCJTVDbmVsc2UlMjBjb25zb2xlLmxvZygnRkFJTCcpJTNCJTIyJTJDJTIyaXNFbnRyeSUyMiUzQXRydWUlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIyZm9vLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMGNvbnN0JTIwZm9vJTIwJTNEJTIwJ2JhciclM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBZmFsc2UlN0QlNUQlMkMlMjJvcHRpb25zJTIyJTNBJTdCJTIyZm9ybWF0JTIyJTNBJTIyZXMlMjIlMkMlMjJuYW1lJTIyJTNBJTIybXlCdW5kbGUlMjIlMkMlMjJhbWQlMjIlM0ElN0IlMjJpZCUyMiUzQSUyMiUyMiU3RCUyQyUyMmdsb2JhbHMlMjIlM0ElN0IlN0QlN0QlMkMlMjJleGFtcGxlJTIyJTNBbnVsbCU3RA==

[rollup-bot]

This PR has been released as part of rollup@3.0.0-5. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-5 or npm install rollup@beta. It will likely become part of a regular release later.

[rollup-bot]

This PR has been released as part of rollup@3.0.0-6. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-6 or npm install rollup@beta. It will likely become part of a regular release later.

[rollup-bot]

This PR has been released as part of rollup@3.0.0-7. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-7 or npm install rollup@beta. It will likely become part of a regular release later.

[rollup-bot]

This PR has been released as part of rollup@3.0.0-8. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-8 or npm install rollup@beta. It will likely become part of a regular release later.

[rollup-bot]

This PR has been released as part of rollup@3.0.0. You can test it via npm install rollup.