feat: Support loading ES Modules

[elijaholmos]

Summary

This PR builds upon the original work of #251, which is over 1.5 years old at the time of writing. Several of the core changes are the same, but I've merged this branch with origin/main to ensure the code is as up-to-date as possible.

Notes

• ESM is only supported by the asynchronous API, because it is enabled by the asynchronous import() function.
• So .mjs is only added to the default searchPlaces of the asynchronous API.
• Support for Node.js 10 and 12 are dropped completely, since they are both EOL
• Bumped the versions of several packages to get the latest ESM-transpilation features
  • typescript 3.9.6 -> 4.8.4
  • @babel/cli 7.10.4 -> 7.19.3
  • @babel/core 7.10.4 -> 7.20.2
  • @babel/preset-env 7.10.4 -> 7.20.2
  • @babel/preset-typescript 7.10.4 -> 7.18.6
• Unit tests are being a pain. I've already spent too much of my own time trying to get those to work, and it'll be a few days before I can sit down again and struggle with those. Unpleasant business, but if they're a requirement for this PR to be merged, I'll try my best. Definitely not opposed to some outside help.
• FWIW, I've packaged the module and loaded it in a sample project, and was successfully able to read a .mjs config and .js config when package.json type is set to module, in Node 16.18
• The to-dos left in Support ECMAScript modules (ESM) #251 (comment) have not yet been added to this PR

[elijaholmos]

I'm going to be honest, I don't have the ability or the patience to work through setting up ESM unit tests. Jest is giving me inconsistent, random errors, varying from "Call retries were exceeded" to "Jest worker encountered 4 child process exceptions, exceeding retry limit" to several other ambiguous messages. Googling and stackoverflow and github issues have made little to no progress. I can't even get the tests @davidtheclark originally set up in https://github.com/davidtheclark/cosmiconfig/tree/esm to completely pass on my Windows machine.

I would love to see ESM support for this library, and I understand how incomplete unit tests are a blocker. I guess the best solution is to slap the "help wanted" label on this PR and hope some savior with considerably more Jest experience than I have comes along to help clean things up.

[geisterfurz007]

I have fiddled around with this for quite some time today and eventually drilled my way down to some dynamic import callback error from node itself that caused all async testcases to blow up that tried to use the loadJs loader which in turn uses await import().

I found that removing the "exclude" section from @babel/preset-env helped with that. Now the test fail because of paths missing their respective .config variants amongst other things but all testsuits run through (Windows 10, Node 16.13.1).

Those other things sadly include the pesky "Jest encountered an unexpected token" error that I have learned to fear (s. screenshot below) but maybe clearing the crashing tests helps open a path for this PR to be continued? I would love to help, so if you have anything that you think I could play around with for a bit to see if it gets rid of the syntax errors, let me know!

[geisterfurz007]

As a sidenote: I saw sindresorhus/import-fresh#22 suggesting ESM is not supported for import with import-fresh (which uses require under the hood); various dirty workarounds (be it https://github.com/small-tech/import-fresh#readme or nodejs/node#49442) didn't seem to be fruitful but I didn't have much time to try them out yet; maybe you can find a way to apply them though!

[elijaholmos]

Thank you @geisterfurz007 for helping move this PR along. I saw you forked my fork; would you be able to push to yours the code that you wrote to get all the tests running?

[geisterfurz007]

Hmmm @elijaholmos, I was just about to do that and double checked but the tests only complete within Webstorm but still fail in the CLI. I'll continue to investigate and if I get somewhere, I'll open a PR on your fork if that's fine with you!

[elijaholmos]

@geisterfurz007 that's absolutely fine with me, thank you for the update!

[geisterfurz007]

Another update from my end @elijaholmos:

Changing the babel config to target node 16 and then running the tests in WSL finally revealed more details:

Doing some research on the error makes it look like this 2 year old issue is connected. In fact there are many people in there and the related Chromium / V8 ticket who are facing the same troubles (with and without relation to use of import() in jest specifically).

My understanding is that the errors you got were caused by Jest failing to run the test three times (since the underlying process crashed without error output 4 times in a row).

I might be wrong, but I feel like this change might have to wait for that V8 problem to get fixed and ported to node before the tests are reasonably achievable annoyingly. If anything helps, we are not alone; there are hundreds of devs complaining about that ticket being solely responsible for halting the move to ESM...

[elijaholmos]

@geisterfurz007 thank you for doing some research into this. The whole CJS/ESM debate is blown so far out of proportion and it's caused an unnecessary amount of problems in the Node.js community. It's a shame that this PR needs to be halted until the Node.js team ports the V8 fix, but I suppose there is not much we can do about it. Thank you again.

[revmischa]

Jest and ESM do not play well together. Try out vitest, it is easy to switch over and it works much better.

[geisterfurz007]

Jest and ESM do not play well together. Try out vitest, it is easy to switch over and it works much better.

@davidtheclark would this be something you are open for?

[darkobits]

I just made the switch from Jest to Vitest recently for the same reaons (lack of ESM support, API bloat) and I would agree, it's a relatively painless migration (they tried to keep a lot of the API's the same, making for easy refactoring) and Vitest is an amazing piece of software.

While I'm not a contributor here, I do rely on cosmiconfig and am eagerly awaiting ESM config support, and would be delighted to pitch in on a Jest-to-Vitest migration effort if it got the green light. 😸

[d-fischer]

I would be okay with replacing jest with vitest under the following conditions:

• it can still report coverage to codecov
• coverage is still 100% (don't have to migrate all tests, as I suspect there's a whole bunch of redundancy in the current tests, but if you don't want to look into that and just migrate everything, that's fine too)
• it's a separate PR and I'll merge it first so this PR can be rebased on it

[beerose]

I started the migration work here: #294, but it requires some changes to the underlying tests. I spent some time on that and got to 8 failed | 202 passed tests, but the remaining 8 tests are less trivial. E.g. some tests mock fs so that it impacts unrelated tests — each test suite passes in isolation but they fail if run all at once. On the other hand, there’s --isolate flag in vitest, but it doesn’t work with --no-threads and I need --no-threads because the tests also use process.chdir (which needs it).

Some ideas are around dependency injection and / or using something like proxyquire for mocks to ensure isolation.

[beerose]

Hey @d-fischer! I just finished the jest -> vitest migration. Here's my PR: #294. Could you take a look and approve the CI workflow?

[d-fischer]

vitest is now merged into the main branch - you may now rebase and go on 🎉

[beerose]

@elijaholmos, @d-fischer — now that cosmiconfig has vitest setup, is there anything I can do to help you move this PR forward?

[elijaholmos]

Hi @beerose, thanks for offering!

Looking at your awesome vitest PR, it appears that it should be compatible with the changes I made in this one. The ES-module-compatibility is part of the core package functionality and should not be impacted by the unit testing suite.

The most helpful thing would be to update this branch with the tests you authored in nodejs/modules#294, so that this branch contains the vitest suite and not the Jest suite. If you wanted to go a step further, you could even update the tests I added to specifically test the ESM functionality, but I could also look into that on my own time (probably in a week or two).

Let me know if you have any questions. Thanks again!

[Jason3S]

I ran into the problem of how to import an ESM module from a CJS file. I ended up creating @cspell/dynamic-import to simplify the issue.

As pointed out above, Jest is not well suited to test ESM modules. Switching to Vitest worked quite well.

I also use synckit to turn async calls into sync calls. It uses a ingenious trick with workers to make it work: Atomics.wait(sharedBufferView, 0, 0, timeout).

[beerose]

@elijaholmos Sorry for a bit late. I'll go ahead and update this PR with the tests over the next few days.

[sebastianrothe]

Looks good

[beerose]

Here's a PR with Vitest tests: #304. Will continue working there

[tianyingchun]

what progress of this?

[beerose]

@tianyingchun you can follow it here: #304. Currently working on adjusting tests after recent migration to vitest: github.com//pull/294

[tianyingchun]

it has bug i think in 8.2.0

https://github.com/Codex-/cosmiconfig-typescript-loader/issues/60
@beerose