🛠️ Tooling: Switch from CJS to ESM

[JoshuaKGoldberg]

Overview

Splitting out from #423 / #422 -> #434: this repo right now is on the default legacy Node.js module system of CommonJS (CJS). The ecosystem as a whole is gradually moving to the more modern ECMAScript Modules (ESM). Roughly copying context from #402 (comment), there are two "module" systems (syntax) we can use for importing & exporting between files:

• CommonJS ("CJS"): an old Node.js-specific syntax that's always been available in Node.js
  • Exporting: module.exports.key = value;
  • Importing: const { key } = require("./file");
• ECMAScript Modules ("ESM"): a new JavaScript standard that's only become widely supported in the last few years
  • Exporting: export const key = value;
  • Importing: import { key } from "./file.js";

ESM is generally a better syntax for a bunch of technical points. But for legacy reasons Node.js defaults to interpreting .js files as CJS. Node.js can be told to change that in two ways:

• Broad: in a package.json, setting "type": "module switches the default to ESM
• Per-file: using an explicit .cjs or .mjs extension for a file switches the file to CJS or ESM, respectively

#402 used the per-file strategy for eslint.config.mjs. But it'd be nice to switch to ESM broadly. Proposal: shall we do this in a dedicated PR?

Additional Notes

I personally tend to do migrations like this manually as a way to ramp up on codebases. But https://nodejs.org/en/learn/getting-started/userland-migrations also exists to help out.

A really cool deep dive on require(esm) from a major Node.js contributor: https://joyeecheung.github.io/blog/2024/03/18/require-esm-in-node-js (this refers to the ability for CJS modules to require() ESM modules, which used to not be allowed).

[lwasser]

@JoshuaKGoldberg i have a followup question about this. I REALLY want to enhance our test suite. without integration tests, we can't be sure the pr's should be merged and our users won't know either.

my question for you is - when we migrate (i agree we should), would tests also use ESM? I read some weird stuff last night about test suites sometimes sticking with CJS (which i keep calling commonmark --oops). And that Jest might supportive CJS over esm (and possibly not support esm. BUT that vitest supports more modern approaches.

so i'm curious what you think there - IF and when i work on the integration tests, would we want to default to ESM for new modules?

[lwasser]

To me it's scary doing all of this refactoring and updating with out a robust test suite. And last night I did confirm a pr that looked good was breaking some of the CLI -OR it was broken before via another pr!

[JoshuaKGoldberg]

would tests also use ESM?

In my view, yes, everything should use ESM. Especially for tests where a framework like Vitests builds things for us (so you get even easier CJS/ESM interop).

If and when I work on the integration tests

🙏 yes please, +1 to having more robust tests - especially to make these kinds of changes less risky!

[lwasser]

would tests also use ESM?

In my view, yes, everything should use ESM. Especially for tests where a framework like Vitests builds things for us (so you get even easier CJS/ESM interop).

If and when I work on the integration tests

🙏 yes please, +1 to having more robust tests - especially to make these kinds of changes less risky!

Ok so last question (i think). 😆 Can we mix ESM and CJS using Jest or do we have to migrate to vitest to do that?

so what i'm asking really is - should we use CJS now for tests and migrate later since now we have jest? OR can we use ESM for new test modules with Jest?

And are you proposing a huge migration vs we tackle a few modules at once? Or will working on in in smaller chunks get messy?

[lwasser]

My main thought it trying to understand how we should break up the work and when to do what!

ie - we need tests to better understand each pr that we are making because the cli has been broken inadvertently (and understandably!) a few times now and tests pass! Yikes!

but i think migration is a great idea too. so what order makes the most sense. 🙃 as we have a chick//egg situation on our hands 🐔

[JoshuaKGoldberg]

Can we mix ESM and CJS using Jest or do we have to migrate to vitest to do that?

With both Jest and Vitest, we should be able to mix them. IME it's easier with Vitest.

And are you proposing a huge migration vs we tackle a few modules at once? Or will working on in in smaller chunks get messy?

I'm proposing an all-at-once migration. For most big touch-all-the-files migrations I actually do normally prefer smaller chunks. But for this one:

• It's very straightforward, mostly just changing syntax at the top & bottom of files
• There are no actual logic changes
• This CLI repo only has a few dozen files

[JoshuaKGoldberg]

For reference, here's roughly how that switch would look: JoshuaKGoldberg@b7199cb

That commit is based on #437 https://github.com/lwasser/cli/tree/build-babel. But to get tests to work we'd need to either play around with Jest shenanigans (😩) or merge in main's commits from #436.

[lwasser]

Oh wow - you have it done! so build-babel and vitest pr's are merged! I saw the approval so I just removed the extensions you mentioned were extra and did all of the cleanup and then merged.

i'm all for moving forward with this.
It's a big / small pr - lots of files, but not a huge amount of changes to each except a few.

is there a way to break it up just a little bit maybe 2 or 3 pr's for easier review?
Or do you think that's more work than it's worth? Either way thank you for this!! yay!

[lwasser]

I am now running into issues with testing and the ESM/CJS mix, and it's easier to write mocks in pure ESM. So I'm now even more in favor of moving forward with this. I'm trying to create mocks for the CLI, and ESM makes life a lot easier.

[JoshuaKGoldberg]

Status update: I have it mostly working locally but ran out of bandwidth before taking time off. I'll hope to get this pushed soon. If I don't get it by EOD Tuesday the 24th someone else should feel free to take over. 🥳