[v3.0] Improve interop defaults#4611
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#improve-interopor load it into the REPL: |
Codecov Report
@@ 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
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
dbda147 to
df5317b
Compare
df5317b to
96af403
Compare
|
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.
Excellent! Then we just need Babel to do this as well now! //cc @nicolo-ribaudo For 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. |
No, it also applies to |
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 |
|
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! |
* Dummy commit * Change default for output.interop to "default", deprecate boolean and add "compat" * Add esModule: 'if-default-prop' and make it the default value * Statically resolve Symbol.toStringTag
|
This PR has been released as part of [email protected]. Note that this is a pre-release, so to test it, you need to install Rollup via |
* Dummy commit * Change default for output.interop to "default", deprecate boolean and add "compat" * Add esModule: 'if-default-prop' and make it the default value * Statically resolve Symbol.toStringTag
|
This PR has been released as part of [email protected]. Note that this is a pre-release, so to test it, you need to install Rollup via |
* Dummy commit * Change default for output.interop to "default", deprecate boolean and add "compat" * Add esModule: 'if-default-prop' and make it the default value * Statically resolve Symbol.toStringTag
|
This PR has been released as part of [email protected]. Note that this is a pre-release, so to test it, you need to install Rollup via |
|
This PR has been released as part of [email protected]. Note that this is a pre-release, so to test it, you need to install Rollup via |
* Dummy commit * Change default for output.interop to "default", deprecate boolean and add "compat" * Add esModule: 'if-default-prop' and make it the default value * Statically resolve Symbol.toStringTag
|
This PR has been released as part of [email protected]. You can test it via |
3 participants
This PR contains:
Are tests included?
Breaking Changes?
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 dependenciesoutput.exports: What to do with default exports in your entry pointsoutput.esModule: Whether to add the "__esModule" special property as a hint when generating non-ESM outputThe overall goal is to change the defaults so that Node’s interop behaviour is more or less the default behaviour. In Node
module.exportsmodule.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)__esModuleis a regular export and not added by defaultnamespace[Symbol.toStringTag] === 'Module'This means I want to implement the following:
output.interopis"default"output.interopvaluestrueandfalseare deprecated. However, I think the "duck-typing" interop oftrueis still very useful (i.e. for the default export, we check if there is adefaultproperty onmodule.exportsfirst and return that, otherwise we returnmodule.exports) because it works very well across tools with the only downside that it does not support "default" as a regular property onmodule.exports. Therefore, it will make a return with a new name, probably "compat".output.exportswill remain as it is and default to"auto", which corresponds well to Node's interop wheremodule.exportsis the default import.output.esModulewill 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" onmodule.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__esModuleproperties in libraries.module.exports, we additionally setmodule.exports[Symbol.toStringTag]to"Module"to promote this interop mechanism (if Symbols are permitted)Symbol.toStringTagare handled so that they can be resolved staticallySymbol.toStringTagis added/handled for external namespaces. Check what Node does for namespace imports from CJScc @guybedford who may have an opinion on this (and of course everyone else is invited to give feedback as well).