ADR: CommonJS and ESM decision

[kjugi]

As discussed in our last meeting #320

Summarizing the decision around ESM builds.

[UlisesGascon]

Great work doing the consolidation @kjugi! Thanks ❤️

I added some suggestions regarding tone and context, feel free to discus/accept/reject them ;)

[bjohansebas]

LGTM, With the changes proposed by Ulises

[kjugi]

Thank you @UlisesGascon for your great suggestions! It elevates this document to a new level. It's more mature now ✌️
@ljharb Thank you too for your comments and attention to detail! 🥇

[wesleytodd]

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.

[UlisesGascon]

maybe as a blog post geared more toward end users

100% that is something that we can do. Who wants to take leadership? :)

[ctcpip]

this is not true though, right? deno can indeed use cjs

[ljharb]

can it? i thought jsr packages could be consumed by cjs but deno still can only consume esm packages

[bjohansebas]

the premise of Deno was that it was only ESM, although I would have to verify it. Now you've made me doubt.

[ctcpip]

https://deno.com/blog/v2.0-release-candidate#improved-commonjs-support

[ctcpip]

https://docs.deno.com/examples/express_tutorial/

[bjohansebas]

Okay, they do support it, so it’s probably fine to say that only Deno 1 doesn’t support it, right?

[ctcpip]

I think we can just remove this line. or rather, change it to reflect that packages may or may not work in browser environments

[bjohansebas]

+1 'may not work in browser environments.'

[kjugi]

Updated! Please resolve if it's fine now ✌️

[blakeembrey]

I would love to expose ESM and did so historically, I presume I should not since it’d be a dedicated effort?

[ctcpip]

Which packages export ESM today?

[blakeembrey]

Historically I did it for path-to-regexp but removed it in the latest major to align with the TC, it wasn’t any additional overhead to maintain. It was more for people’s build tooling than node ESM though, as adding it predated it being formalized.

[kjugi]

Do you think I should include this fact from the past? I've been thinking about how to put this together now and I'm not sure.
The document should state the last known status. We can list exceptions now (if any) and explain this fact or update the document later when we agree on which one we want to change. There is a Changelog on the bottom so it should be easy to mark that out in the future.

[bjohansebas]

We would need to change the phrase 'historically,' as it gives the impression that it was never done, but as Blake explains, it was done in some package.

[blakeembrey]

Being isomorphic has been intentional for the packages I maintain, and I do make a dedicated effort to keep it isomorphic. This includes edge environments that are plain JavaScript. The lack of CI for these environments aren’t connected to their support.

[ljharb]

I don't agree with that last statement; something isn't actually supported unless it's actually tested.

[blakeembrey]

That’s a reasonable take, but doesn’t preclude that I do support them. Some things are unreasonable to have direct tests. There’s a reason we often only test each major node release instead of every single version, and we don’t claim those aren’t supported. It’s the same that I don’t test directly in CI for Cloudflare Workers (honestly not sure how I would tbh) but I do support them, by virtue of knowing it’s V8 and disallowing native node APIs.

[blakeembrey]

TL;DR is that I don’t think this assertion belongs in the doc, something being isomorphic may be related but is not connected to ESM support. I don’t want to give an impression that some of the packages in these organizations are not designed to support other JavaScript environments, regardless of ESM support, since it could be referred to as a way to drop existing supported environments without a major.

[ctcpip]

I agree, we should decouple isomorphism from module system compatibility. The root of this was that ESM exports would be justified for packages for which we wanted to support isomorphism. In other words, if the package was not intended to be isomorphic, there is perceived little value in exporting ESM.

I also agree that we cannot (and should not attempt) to test against every possible environment. However, isomorphic packages should get some level of testing outside of node, and this should hopefully not be prohibitively difficult.

We have already run into situations where we are not certain if a package is properly working in other environments, in part due to lack of isomorphism in the test suite. (Although admittedly router is not the best example of a package for which we would want to support isomorphism.)

[blakeembrey]

Maybe but sadly we’re stuck with the tooling we have today, not the ideal world, so I thought it worth mentioning this isn’t just a browser concern.

Ref: https://github.com/evanw/esbuild/blob/main/docs/architecture.md#es6-linking

This is possible with ES6 modules but not with CommonJS because ES6 imports are bound at compile time while CommonJS imports are bound at run time.

Just to be clear, I’m not at all advocating for ESM here, I’m only providing context on why it shouldn’t be dismissed as “it’s a browser thing”.

[ljharb]

ESM imports can be static or dynamic; so can CJS requires - since tooling has to account for both, there's literally no technical difference between them. Nothing is possible for ESM that's not possible for CJS if you're talking about a build tool.

[blakeembrey]

That sounds like something to take up with the tools, I’m just pointing out that common build tools (both Webpack and esbuild) require ESM for tree shaking, and these are common tools to bundle for server side environments that may have size limits (including ones that run node). Things haven’t split cleanly into “ship it onto a server” vs “ship it to a browser” for years now, as I often ship code to server less platforms.

Again, don’t take this as arguing for ESM support in this thread, but to argue against that it’s only relevant for browsers.

[ljharb]

I completely agree :-) i just think it's important to point out that it's a failing in the tools, not an inherent difference in the module format.

[kjugi]

Ok, I have applied some changes. Please verify. Also, I think I'm parking the "isomorphic" topic completely. As mentioned in the comments, let's focus on export type here. I can start another doc about isomorphic and its status right after this one if needed.

[blakeembrey]

There is a greater negative impact which is that other users fork the packages to ESM and those packages lack security updates over time which leaves users in a worse position.

[kjugi]

That's a valid point! Added, thanks! Please resolve if it's fine now ✌️

[kjugi]

This part needs polishing. Soften the language and consider exporting ESM when it requires little effort. Include libraries maintained by @blakeembrey, which now include ESM export. It should be clear that our CI tests do not cover this for today.
"CommonJS is a requirement, but ESM is not"

[samuelstroschein]

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 path-to-regexp to distribute CJS and ESM does not lead to additional overhead (@blakeembrey in #323 (comment)), why should the desire for express js to consume dependencies as CJS block dependencies from emitting ESM at their will?

Adopting path-to-regexp is a tough choice for me now which leads to:

• consistently pushing for ESM OR
• fork/not adopt path-to-regexp

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.

[wesleytodd]

I assume that experience will be universal across express js dependencies.

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.

[ljharb]

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.

[samuelstroschein]

@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.

, so I'm still not clear on the value.

I can't adopt path-to-regexp if it's not ESM because tree-shaking doesn't work pillarjs/path-to-regexp#347 and import errors occur pillarjs/path-to-regexp#346.

it increases the package size (which seems to be important to the same folks that want native ESM).

ESM people care about tree-shaking, not the package size.

[wesleytodd]

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.

[ljharb]

@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.

[wesleytodd]

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.