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 all 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
85 changes: 85 additions & 0 deletions docs/adr/esm-support.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# 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.

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

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 was specifically designed to run with Node.js. While some of our libraries can run in other runtimes, this can currently be classified as an "unofficial and untested feature." Consequently, our CI systems do not include other runtimes as part of their testing workflows.

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 [expressjs](https://github.com/expressjs), [pillarjs](https://github.com/pillarjs), or [jshttp](https://github.com/jshttp) packages. PRs with such a change will not be accepted.
Copy link
Member Author

Choose a reason for hiding this comment

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

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"


**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 (coming from bundler tools), and support for dynamic imports, there are still complexities and challenges associated with ESM.
Copy link

Choose a reason for hiding this comment

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

If there is any document that supports the "default syntax" statement, it would be beneficial to link it.

It seems to me that in Node, CJS is the de facto standard because of its history but it's not set in stone. Especially with syntax detection enabled by default in all LTS versions after April 2025 (when 18 is gone) - Node 20.19 has it enabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

node’s own docs, the default value of the “type” field, the default value of node’s input-type arg… does it really require evidence when it’s so overwhelmingly the case?

Copy link

Choose a reason for hiding this comment

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

This says absolutely nothing about what the project considers the default. But you know why? Let's go down the rabbit hole.

Back when the toggle was first added it already had mjs put first into the Markdown and cjs below it. It still is the case to this day.

In author's own words:

Enable code example using both modern ESM syntax and legacy CJS syntax.

Want more examples?
nodejs/node#37170

This is done to put emphasis on the ESM and Promise model.

Commit message says:

Convert examples to esm to help promote that pattern with users

Now, this only shows intent from authors. But is much better case (in my opinion) than the toggle.

I'd rather this decision was based on evidence and not coincidence that some toggle defaults to this or that state. And this could as well be a "bug" to fix with the docs page.

Copy link
Contributor

Choose a reason for hiding this comment

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

The default is whatever happens without configuration with .js files, which is always CJS.

it is not a bug, it is an intentional decision that is unlikely to ever change because it would break too much of the ecosystem.

Copy link

Choose a reason for hiding this comment

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

The default is whatever happens without configuration with .js files, which is always CJS.

Let's do a test.

❯ mkdir testesm && cd testesm
❯ echo 'import {version} from "node:process"; console.log(version);' > test.js
❯ node test.js

Which will fail on node 18:

❯ /opt/homebrew/Cellar/node@18/18.20.8/bin/node test.js
(node:47428) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
/Users/user/Projects/testesm/test.js:1
import {version} from "node:process"; console.log(version);
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at internalCompileFunction (node:internal/vm:76:18)
    at wrapSafe (node:internal/modules/cjs/loader:1283:20)
    at Module._compile (node:internal/modules/cjs/loader:1328:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)
    at Module.load (node:internal/modules/cjs/loader:1203:32)
    at Module._load (node:internal/modules/cjs/loader:1019:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:128:12)
    at node:internal/main/run_main_module:28:49

Node.js v18.20.8

But will have ABSOLUTELY NO ISSUE on 20+.

❯ node test.js
v20.19.0

Therefore it's not always CJS.

Let's extend the test further:

❯ echo '{"name": "testesm"}' > package.json
❯ node test.js
v20.19.0
(node:48221) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///Users/user/Projects/testesm/test.js is not specified and it doesn't parse as CommonJS.
Reparsing as ES module because module syntax was detected. This incurs a performance overhead.
To eliminate this warning, add "type": "module" to /Users/user/Projects/testesm/package.json.
(Use `node --trace-warnings ...` to show where the warning was created)

Even with poorly packaged code your statement does not hold true and node.js does not default to CJS. Nothing breaks in the ecosystem.

The only actual evidence of "default" would be here: https://nodejs.org/docs/latest/api/packages.html#introduction

In particular, package authors should always include the "type" field in their package.json files, even in packages where all sources are CommonJS. Being explicit about the type of the package will future-proof the package in case the default type of Node.js ever changes

And the type documentation is also misleading, as it states:

import statements of .js files are treated as ES modules if the nearest parent package.json contains "type": "module"

Which is not true, because omitting type does not mean commonjs by default.

❯ echo '{"name": "testesm", "type": "commonjs"}' > package.json

which breaks ESM inside .js files.

This shows that no type != commonjs. If there's any "default", it's "syntax detection".

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, that's fair that the default is now "syntax detection" for files. However, for the REPL, it's still CJS, and i think this sentence remains correct.

Copy link
Member

Choose a reason for hiding this comment

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

@slagiewka, If you disagree with the wording, please suggest an alternative and we can decide if it is a better fit. Otherwise I think we can resolve this thread, because I don't think nitpicking this wording choice is worth it if the goal is not to pick other words. Especially since these were chosen intentionally and are technically correct enough.

Copy link

Choose a reason for hiding this comment

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

Especially since these were chosen intentionally and are technically correct enough.

I'm yet to see it be proven technically correct. There is no default standard in node.js and there is no suggested syntax for libraries to use. Granted, the state of lesser complexity is fairly new.

nitpicking this wording choice

This is the leading statement that serves as the rationale for the decision. And at the time of writing is already hard to sustain. It may skew the decision towards suboptimal choice. However, I sense that everyone's mind is already set anyway.

If you disagree with the wording, please suggest an alternative

Given the above, there are a couple of options:

  1. Remove what can't be supported
Suggested change
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 (coming from bundler tools), and support for dynamic imports, there are still complexities and challenges associated with ESM.
While the JavaScript ecosystem has increasingly moved toward ESM due to its compatibility with browsers, enhanced tree-shaking capabilities (coming from bundler tools), and support for dynamic imports, there are still complexities and challenges associated with ESM.
  1. Try describing node's limbo.
Suggested change
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 (coming from bundler tools), and support for dynamic imports, there are still complexities and challenges associated with ESM.
CommonJS is still the dominating syntax in Node.js userland. While the JavaScript ecosystem has increasingly moved toward ESM due to its compatibility with browsers, enhanced tree-shaking capabilities (coming from bundler tools), and support for dynamic imports, there are still complexities and challenges associated with ESM. Node.js has only recently gained enough support for libraries to transition to ESM without requiring users to move away from CommonJS.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I said, it's the default syntax in the repl, which should be sufficient to establish that it's the default syntax for node.

Also, node 18 isn 't EOL quite yet, and it will be years after that that libraries can painlessly transition to 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.
- **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**:
- No guarantee the packages work in browser environments.
- Potential community library fork (to make it ESM-friendly) might lack security updates over time
- OSS library authors that use our packages in ESM native libs might suffer from a lack of support
- **Mitigations**: Potential decision update 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:
Copy link

Choose a reason for hiding this comment

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

This is no longer true.
Node 20.19 - experimental, but no flag required: https://nodejs.org/en/blog/release/v20.19.0#requireesm-is-now-enabled-by-default
Node 22.12 - experimental, but no flag required: https://nodejs.org/en/blog/release/v22.12.0#requireesm-is-now-enabled-by-default

Adding this to LTS shows commitment to the feature and the following is an example:

This feature has been tested on v23.x and v22.x, and we are looking for user feedback from v20.x to make more final tweaks before fully stabilizing it.

Copy link
Member

@wesleytodd wesleytodd Apr 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
Support for ESM modules imports in commonjs is available since node v20 behind the experimental flag and node v23 without a flag. Docs:
Support for ESM modules imports in commonjs is available in recent LTS Node.js versions. Docs:

Lets just drop the moving target and reference the upstream 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
- **[2025-02-02]**: [@kjugi] - applied second part of code review suggestions