ADR: CommonJS and ESM decision#323
Conversation
UlisesGascon
left a comment
There was a problem hiding this comment.
Great work doing the consolidation @kjugi! Thanks ❤️
I added some suggestions regarding tone and context, feel free to discus/accept/reject them ;)
bjohansebas
left a comment
There was a problem hiding this comment.
LGTM, With the changes proposed by Ulises
|
Thank you @UlisesGascon for your great suggestions! It elevates this document to a new level. It's more mature now ✌️ |
wesleytodd
left a comment
There was a problem hiding this comment.
I am good with this as it is more of a "tracking document", but it may end up being something end-users look at and I am not sure it fully satisfies the questions they might have. We should keep an eye out for feedback on if we should make a user facing statement on this at some point, maybe as a blog post geared more toward end users.
100% that is something that we can do. Who wants to take leadership? :) |
|
Chipping in from an "I want to use express dependencies but need ESM" perspective, see pillarjs/path-to-regexp#347. If the maintenance effort of libraries like Adopting path-to-regexp is a tough choice for me now which leads to:
I assume that experience will be universal across express js dependencies. Again, all under the presumption that distributing ESM in dependencies yields no additional overhead. |
This was a main point of our discussion. It is not clear to me that it should be universal. The one thing we all agreed on is that we MUST ship CJS, but there seems to be no reason to limit packages which want to also ship ESM from doing so. That was the main feedback on this doc from our meeting yesterday IIUC. |
|
There are inherent costs and dangers in shipping a dual-format package - it exacerbates the likelihood of duplicated (and thus out of sync) state; it's unlikely both formats are being tested (whether they're hand-written, increasing the likelihood of deviations, or transpiled, increasing the likelihood of transformation bugs); it increases the package size (which seems to be important to the same folks that want native ESM). Also, node can import CJS, and every build tool can translate it to ESM just fine if needed, so I'm still not clear on the value. |
|
@ljharb hope to share some light on the necessity of ESM. Ofc you can make the decision to ignore the (minor) group that needs ESM. That's completely OK. Everything is a trade-off.
I can't adopt
ESM people care about tree-shaking, not the package size. |
|
Yeah, I agree that I continue to be unclear on the value. That said, we did discuss on the call yesterday that we should expect them to be tested if we are shipping dual mode packages. |
|
@samuelstroschein right - i'm saying that CJS treeshakes the same as ESM, and if there's a specific tool you're using that doesn't support it, i'd encourage you to advocate they do so. |
|
If someone comes with proof that there is some technical barrier to efficiently using CJS in a real world use case, I would absolutely love to help, but that is not what has happened in any of these threads (see my question in the one linked about treeshaking). EDIT: I realized this might be unclear in this comment so want to point out that I shared my opinion above that we should not stand against packages shipping dual mode if the folks maintaining those want to. |
|
Btw, the development version of Multer (expressjs/multer#399) is only for ESM, which means that in that development version, it should be converted to CommonJS. |
slagiewka
left a comment
There was a problem hiding this comment.
For going ESM-only, the elephant in the room is TypeScript. Users wanting to run require(esm) (because their output from tsc is CJS) will need to either set --module nodenext when on Typescript 5.8 OR wait for 5.9 to use --module node20.
More on that here: microsoft/TypeScript#60761
If this doc would consider next major releases that target Node 20+ the summary would be simple: choosing ESM over CJS is a matter of preference. Publishing dual package makes no sense.
However, it can also be a matter of vision. If the express ecosystem wants to be considered modern, it would use modern standards as long as it allows the stability it's renown for.
|
This proposed ADR carves out the question of environment support, so that this ADR can focus on CJS vs ESM rather than runtime/env support. |
ctcpip
left a comment
There was a problem hiding this comment.
blocking for now until we resolve all issues, and, ideally, settle on the language for a related ADR, which has, in a sense, a broader scope, and should allow us to clean up some of the language here
|
A switch actually requires more than just dropping support for old Node.js versions. This block of code in the |
|
There's a fair amount of use cases for ESM builds of libraries such as Perhaps that's true also for That said, in browser-land, there's a lot of packages still making use of "Why not just use CJS, just bundle..." I know that in most ecosystems, having a CJS-to-ESM conversion happening in your local dev and build tools is common (Vite being the most popular at this time), that said, there are plenty of ecosystems where the focus is on "browser-compatible out of the box". For example, CDNs will often mirror NPM registry and only handle bare import specifier resolution (or not, and expect you to deal with it via importmap, JSPM is an example of such a CDN), and nothing else, meaning that CJS-only packages are not usable from such CDNs. Especially in corporate environments where microfrontends and heavy CDN usage is quite common, this poses a significant challenge. This is just one example, I just happen to be quite active in frontend environments where a CJS-to-ESM conversion isn't a given, or is actively avoided for toolchain-minimalism or performance-related reasons. Do what you will with that perspective... just wanted to make sure this is expressed and heard, and with some luck, considered in the decision making around this. |
Co-authored-by: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Co-authored-by: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Co-authored-by: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Co-authored-by: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Co-authored-by: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Co-authored-by: Chris de Almeida <ctcpip@users.noreply.github.com>
Co-authored-by: Wes Todd <wes@wesleytodd.com>
eb0ae33 to
a9ea109
Compare
|
Locking so we don't lose conversation in zombie PRs. Please create a new issue or PR for further discussion/iteration if needed. |
As discussed in our last meeting #320
Summarizing the decision around ESM builds.