Inline interopDefault in config loading

[guybedford]

This inlines the interopDefault handling.

Note that we don't need to explicitly handle the null case (the rare chance of module.exports = undefined), because that would fail anyway in subsequent code paths.

The benefit of this approach is that as discussed previously I'm working on supporting Rollup itself in a transformation that can convert CJS packages to ESM packages, and we can support dynamic require fine when it is detected as Promise.resolve(require(x)), but the interopDefault wrapper stops this from working out.

Would be amazing to merge this to support the dynamic import conversion for full Rollup support in this work here.

[lukastaegert]

There sure is no problem in merging this but I really do not get what this change should accomplish as this only affects the code Rollup uses when loading config files and does not change any code Rollup will generate?

If you actually want to change the code generated by Rollup, note that I am thinking about reworking Rollup's interop handling as the current approach does not allow to easily forward dynamic bindings of default exports (basically this approach—a very recent addition:
https://rollupjs.org/repl?version=1.7.4&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMCU3QmR5bmFtaWNCaW5kaW5nJTdEJTIwZnJvbSUyMCdleHRlcm5hbCclM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJjanMlMjIlMkMlMjJuYW1lJTIyJTNBJTIybXlCdW5kbGUlMjIlMkMlMjJhbWQlMjIlM0ElN0IlMjJpZCUyMiUzQSUyMiUyMiU3RCUyQyUyMmdsb2JhbHMlMjIlM0ElN0IlN0QlN0QlMkMlMjJleGFtcGxlJTIyJTNBbnVsbCU3RA==
does not easily translate to default exports as the complete interop would need to be repeated for each access).

My goal would be to take some inspiration from babel here and use a normalization similar to theirs:

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

so that when an object containing a default key is imported, we do not accidentally destroy the live-binding by extracting the default once.

Maybe this has some bearing on what you have in mind?

Otherwise if this is really only about Rollup's code itself, I can sure merge it.

[guybedford]

There sure is no problem in merging this but I really do not get what this change should accomplish as this only affects the code Rollup uses when loading config files and does not change any code Rollup will generate?

Yes exactly, this is an esoteric case of supporting Rollup itself through jspm. Would be much appreciated to support it.

This is only about dynamic imports - which are supported fine for generated code using the same convention - https://rollupjs.org/repl?version=1.7.4&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydHMuZ2V0RHluYW1pYyUyMCUzRCUyMGZ1bmN0aW9uJTIwKCklMjAlN0IlNUNuJTVDdHJldHVybiUyMGltcG9ydCgnZHluYW1pYycpJTNCJTVDbiU3RCUyMiUyQyUyMmlzRW50cnklMjIlM0F0cnVlJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmNqcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

It would be great to keep this property for similar reasons as it means generated CJS code can be converted to ESM with dynamic import support.

Note also that now that node --experimental-modules is stable we should also start to move towards stricter interop that only permits a default export from CommonJS, as that is all Node will do now, which will effectively dictate the running ES Modules conventions going forward.

But yes, this is just about Rollup itself.

[lukastaegert]

Note also that now that node --experimental-modules is stable we should also start to move towards stricter interop that only permits a default export from CommonJS, as that is all Node will do now, which will effectively dictate the running ES Modules conventions going forward

I have been thinking a lot about interop lately and my goal for future versions of Rollup currently is:

• introduce a way to configure interop to closely match what Babel does, which is that you only get a default export unless the __esModule prop exists
• the reason why Rollup is different here is that some people started passing dynamic namespace objects around, attaching them to global variables and using them as dependency for IIFE bundles. To accommodate these cases, there should be a way to attach the __esModule prop to either all namespace objects or at least those that are passed around.
• in a new major version, the first or both of these behaviours should be on by default

Adding another "strict" interop mode that does not allow for named exports at all is definitely an option but I do not see this being on by default, not as long as there are many code-bases out there that depend on Babel transformed code.