support ES6 default require for packages

[aminya]

Description of the change

In short:

This allows loading the packages that are Es module and export their function wrapped in a default object. This is the case for any package that uses Babel 6, Babel 7, or other modern compilers for transpiling their package.

Longer:

The behavior of ES6 is to use exports.default = .... for exporting something as default.
This is equal to using module.exports = .... or module.exports = {...}.

In ES6 requiring the modules that exported like need explicitly calling .default

const x = require(module).default // require(module) is wrong

_require function in this pull request checks if default property exists and returns what it is inside it instead of returning the useless module itself (which will not have the activate function for the package).

_require function checks this phenomenon and adds support for it. Many compilers or bundlers use _interopDefault which serves the same purpose.

Drawbacks

This function is written pretty conservatively and has no drawbacks.

In Atom, all the packages can only export their functions either named or wrapped in an object. There is no real useful default export that can be called directly. So the mixed situation does not happen, and we can safely merge this PR.

Verification

The CI passes

Release Notes

• Adding support for the packages that export their main functions under an object and are transpiled using modern transpilers (e.g babel 6 or later, etc).

[DeeDeeG]

Just commenting to register that I've seen this. I am not versed with JavaScript or ES versions, and frankly my strong area is in tooling/build troubleshooting (dev ops??), so I can't review the exact implementation here.

But I appreciate the conservative design approach, and I see that CI passes, so that is an approach I can get behind. "RSLGTM", as they say?

I'm not sure what you want to use this with, presumably some ES6 code Atom-side or package-side. So if there is a use-case that would add some context for understanding this, even as a person somewhat ignorant of ES6 such as myself.

(Trying to review or comment on stuff even if I don't have much useful to add.)

[aminya]

Thanks for the review. JavaScript is pretty simple. You can learn and master it in a day 😉

[aminya]

The upstream PR is made atom#21112.
Merging this.

[DeeDeeG]

I see this is needed for Babel 7. Which is needed for newer Node (12.17+, 14.0.0+).

In which case, great to have this taken care of!

[DeeDeeG]

I suppose a non-ES6 package could adopt a bad convention, where it exports a "default" module even though it is ES5 or older... Right? And in conjunction with this PR, it could change what is exported and what code is available from the package.

Is this in acceptable risk? Or is that scenario not even problematic? (For instance, I suppose if someone had hypothetically made a "default" module to export, using that one is probably pretty safe.)

[aminya]

There is no risk for Atom packages. Atom packages that have exported activate function are only loaded using this method. This only fixes the main file of an Atom package, and the internal relations should be updated as well.

For internal Node requires, we need babel-plugin-add-module-exports which adds:

module.exports = exports.default

to the code in some situations. This is included in our big babel PR: #67

We may be able to do a better job than this plugin by scanning the ways the exports of a module are imported in other packages. That means instead of modifying the exports, we modify the imports only if needed. But for now, I don't have a plan for that. Instead of transforming things manually inside Atom, we might just offload all of these to a bundler like Parcel or Rollup, which take account for these types of things. This can improve performance as well.

The real risk is staying in pre-2015 which fears every one who expects a modern Atom.

[DeeDeeG]

Thanks for the explanation and context.