ADR: CommonJS and ESM decision

[kjugi]

As discussed in our last meeting https://github.com/expressjs/discussions/issues/320

Summarizing the decision around ESM builds.

[kjugi]

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

[UlisesGascon]

maybe as a blog post geared more toward end users

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

[samuelstroschein]

Chipping in from an "I want to use express dependencies but need ESM" perspective, see https://github.com/pillarjs/path-to-regexp/issues/347.

If the maintenance effort of libraries like path-to-regexp to distribute CJS and ESM does not lead to additional overhead (@blakeembrey in https://github.com/expressjs/discussions/pull/323#discussion_r1929203580), 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 https://github.com/pillarjs/path-to-regexp/issues/347 and import errors occur https://github.com/pillarjs/path-to-regexp/issues/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.

[bjohansebas]

Btw, the development version of Multer (https://github.com/expressjs/multer/pull/399) is only for ESM, which means that in that development version, it should be converted to CommonJS.

[ctcpip]

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.

[dynst]

A switch actually requires more than just dropping support for old Node.js versions.

This block of code in the View constructor is currently dynamically, and synchronously, calling require(arbitrarystring), but in ESM, that kind of arbitrary import would have to be part of an async function, because it would have to use await import(...) syntax instead.

https://github.com/expressjs/express/blob/54af593b739ea44674e4a445efa15b8024f093da/lib/view.js#L75-L88