Skip to content

ADR: CommonJS and ESM decision #323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

ADR: CommonJS and ESM decision #323

Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2667117
Create isomorphism-support init doc
kjugi Jan 15, 2025
cca6311
update extension for file
kjugi Jan 15, 2025
d8c9118
Add Link to issue
kjugi Jan 15, 2025
cda116a
Add another issue to the list
kjugi Jan 16, 2025
b8d5a59
Update isomorphism-support.md
kjugi Jan 16, 2025
7fa2531
Update isomorphism-support.md
kjugi Jan 16, 2025
0f72b27
Update docs/adr/isomorphism-support.md
kjugi Jan 18, 2025
8007ef6
Update docs/adr/isomorphism-support.md
kjugi Jan 18, 2025
ca5dcb0
Update docs/adr/isomorphism-support.md
kjugi Jan 18, 2025
43b0ed8
Update docs/adr/isomorphism-support.md
kjugi Jan 18, 2025
166e5f8
Apply suggestions from code review
kjugi Jan 18, 2025
9fe1075
Update isomorphism-support.md
kjugi Jan 18, 2025
4ca69bc
Update docs/adr/isomorphism-support.md
kjugi Feb 1, 2025
01d4903
Rename isomorphism-support.md to esm-support.md
kjugi Feb 1, 2025
68eb3d0
Update esm-support.md
kjugi Feb 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions docs/adr/isomorphism-support.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# ADR: Export the libraries specifically in ESM format

## Status

Proposed

## Submitters

- @kjugi
- @wesleytodd
- @ctcpip

## Decision Owners

- @expressjs/express-tc

## Context

The document's objective is to gather all notable comments and thoughts in one place and track potential changes in this topic. We have noticed that it is repeated frequently in many issues from the community and we need to take action.

We have acknowledged the need and discussion around it touched on multiple scenarios. Including:
- rethinking the process and exposing both options (ESM and CommonJS) for all libraries
- expose both options (ESM and CommonJS) for selected libraries
- keeping default settings as the main target is on the server

**Why do we need this decision?**
We aimed to consolidate the Technical Committee's (TC) opinion on this topic. It is important to emphasize that Express is an HTTP framework specifically designed for Node.js. Over the years, technology has evolved, and new runtimes have emerged. Additionally, some of our libraries are being utilized by the community in other environments, such as browsers.

**What problem does it solve or avoid?**
Ambiguity and uncertainty for the community, alongside clear guidance for repository maintainers and contributors.

**Are there any existing issues/discussions/pull requests related to this?**
- https://github.com/pillarjs/router/issues/128
- https://github.com/expressjs/discussions/issues/297
- https://github.com/expressjs/express/discussions/6051
- https://github.com/jshttp/cookie/issues/211

## Decision

During the [working session](https://github.com/expressjs/discussions/issues/320), we had an in-depth discussion about this topic. After careful consideration, we concluded that we will not make a dedicated effort to export our libraries in the ESM format. Instead, we will continue exporting the libraries as we have done historically.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which packages export ESM today?

Copy link
Member

@blakeembrey blakeembrey Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


This decision is motivated by the lack of resources to maintain such an effort in the long term. It is also worth noting that Express is specifically designed to run with Node.js. While some of our libraries can be considered "isomorphic," this was unintended and can currently be classified as an "unofficial but functional feature." Consequently, our CI systems do not include browsers or other runtimes as part of their testing workflows.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@ctcpip ctcpip Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@blakeembrey blakeembrey Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


At present, our libraries function seamlessly in Node.js, supporting both CommonJS and ESM. Transitioning to support additional scenarios, such as direct ESM exports, would require significant changes to our CI systems and additional maintenance overhead.

**TL;DR**: Dedicated ESM exports will not be available for Express.js, PillarJS, or JSHTTP packages. PR with such a change will not be accepted.

**What will be done?**

Future issues can be closed with a link to this document.

## Rationale

CommonJS is the default syntax in Node.js. While the JavaScript ecosystem has increasingly moved toward ESM due to its compatibility with browsers, enhanced tree-shaking capabilities, and support for dynamic imports, there are still complexities and challenges associated with ESM.

Adopting ESM for our libraries would require a significant investment of time and resources to ensure proper implementation and long-term maintenance. While it is not impossible to achieve, it represents a considerable effort. Moreover, the majority of our users already utilize our libraries in their projects, relying on bundlers to handle the necessary transformations without issues.

- **Alternatives Considered:**
- Alternative 1: Add ESM export to our libraries. CommonJS format is accepted by all most popular bundlers.
- **Pros and Cons**: Outline the pros and cons of the chosen solution.
- **Why is this decision the best option?** Time and energy can be shifted to other topics.

## Consequences

- **Positive Impact**: It does not require to support another set of tools and one more major (or at least big) release.
- **Negative Impact**:
- Packages can't be used in deno projects and potentially in other future runtime engines for JavaScript that decide to not support commonjs. That can be a potential user miss
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 'may not work in browser environments.'

Copy link
Member Author

@kjugi kjugi Feb 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- OSS library authors that use our packages in ESM native libs might suffer from a lack of support
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@kjugi kjugi Feb 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- **Mitigations**: Potential decision update to support isomorphism for selected libraries (not specified yet) and exposing both types (CJS and ESM)

## References

Support for commonjs imports in ESM code is available in the node. Described in docs:
- https://nodejs.org/api/esm.html#interoperability-with-commonjs

Support for ESM modules imports in commonjs is available since node v20 behind the experimental flag and node v23 without a flag. Docs:
- https://nodejs.org/api/modules.html#loading-ecmascript-modules-using-require

## Changelog

Track changes or updates to this ADR over time. Include the date, author, and a brief description of each change.

- **[2025-01-15]**: [@kjugi] - document init
- **[2025-01-18]**: [@kjugi] - applied code review suggestions
Loading