Switch from CommonJS to ES Modules

[jkuester]

Describe the issue

For better or worse it seems that ES Modules are not going to go away. Various parts of the JS ecosystem have started dropping support for CommonJS modules (which is what we are using currently).

These are the currently known dependency packages whose latest versions only support ESM:

• @octokit/core
• @octokit/plugin-paginate-graphql
• @types/chai
• @types/chai-as-promised
• chai
• chai-as-promised
• chai-exclude
• node-fetch
• url-join

Describe the improvement you'd like

Unfortunately, I expect converting to ESM to be very painful. I did this recently with CHToolbox and documented the experience. The main issue for cht-core is probably going to be the same sinon troubles. I think esmock is a reasonable solution here, but it will still involve a lot of changes to the test setup code....

Describe alternatives you've considered

We could just give up on any dependencies that are ESM only.

[dianabarsan]

Thank you for summarizing this and flagging all dependencies that are ESM only.

I agree that this will require a titanic effort to convert everything.
What we have done, so far, was to use older versions of these updated dependencies that used to support CommonJS.

But, for some of these dependencies that you have listed:

-> @octokit/core
-> @octokit/plugin-paginate-graphql

These are only used in our release notes script, which we could independently switch to ESM.

-> @types/chai
-> @types/chai-as-promised
-> chai
-> chai-as-promised
-> chai-exclude

Testing frameworks are not high priority in terms of upgrading, unless there are some serious security concerns. Putting it this much effort for syntactic sugar or small improvements in the framework seems like a bad deal to me.

-> node-fetch

Afaik only PouchDb uses this, so it's not our direct dependency, and Pouch locks to the CommonJS version.
We use bundled fetch for our HTTP communication.

-> url-join

This is only used in outbound. I have used bundled path package to join URLs before, and it seems to be an ... "accepted" (tolerated) way of doing this. So this seems like the only dependency that we can consider dropping.

[jkuester]

100% agree with your assessment here! The current ESM dependency list does not provide any pressing need to upgrade. My main purpose for raising this ticket now was just to give a convenient ESM sticky to link to from the dependency upgrade docs so we can easily track a running list of dependencies that devs do not need to bother trying to upgrade.

It will be interesting to see where the ecosystem goes from here. I worry that more and more projects will do the chai thing and go ESM only. But so far there still seems to be a strong ecosystem for CommonJS... 🤞

[jkuester]

If/when this is ever addressed, we need to remove the dependencies tagged with this issue from the dependabot ignore list.