Skip to content
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.

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

ESM support #13319

Closed
1 task done
andylacko opened this issue Mar 13, 2024 · 11 comments
Closed
1 task done

ESM support #13319

andylacko opened this issue Mar 13, 2024 · 11 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@andylacko
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

Some modules does not support old commonjs modules. We are stuck on older versions.

Describe the solution you'd like

Move to ESM, it is 2024

Teachability, documentation, adoption, migration strategy

Flawlessly

What is the motivation / use case for changing the behavior?

Standardize as many things among the world. Less diversity in questions that are almost the same leads to simplicity and less learning, kind of optimization. This is similar problem like in some countries people don't use standardized measuring system and there is different time date format for every country.

IMG_1294

@andylacko andylacko added needs triage This issue has not been looked into type: enhancement 🐺 labels Mar 13, 2024
@micalevisk
Copy link
Member

Move to ESM, it is 2024

that's not a good argument tho.

We are stuck on older versions.

you can use ESM-only packages in CommonJS projects just fine. It's a nodejs feature, not related with nestjs.


See #8736

@KostyaTretyak

This comment was marked as off-topic.

@magicdawn
Copy link

https://github.com/wooorm/npm-esm-vs-cjs

more and more are moving to ESM, inconvenient to use more and more import()

@Meierschlumpf
Copy link

What about migrating to esm is so hard?
It does not hurt anybody when you support both and improves developer experience with nestjs and esm modules immensely.

I mean the way has already been paved with:
#8736

Benefits of esm modules are:

  • standard way of doing modules
  • supported in browsers and server side runtimes like deno and node
  • allow asynchronous loading
  • exports without globals
  • support for more packages

For me and my team it's a dealbreaker if there is no support for esm with nestjs as we don't want to work our way arround this package or monkey patch everything.

Thanks for the effort and in advance

@kamilmysliwiec
Copy link
Member

support for more packages

More packages use commonjs than esm atm. You can still use async import() if you really need esm modules in the latest version of Node you can now statically require(esm) package (see below).

supported in browsers and server side runtimes like deno and node

Using Nest in the browser makes zero sense

standard way of doing modules

CJS was the standard way of doing modules for 10 years and it's not going anywhere

allow asynchronous loading

What use case does it cover which is not already covered by Nest?

It does not hurt anybody when you support both

It hurts us as it adds a significant maintenance burden. Maintaining both is a huge waste of time and we can't yet drop support for CJS as it would hurt lots of NestJS users all around the globe.

more and more are moving to ESM, inconvenient to use more and more import()

@magicdawn this graph isn't accurate, see wooorm/npm-esm-vs-cjs#9

Also, in the latest version of Node.js (should be backportable to v20) you can require(esm) package with the --experimental-require-module flag nodejs/node#51977. I hope that (almost) full interoperability between ESM and CJS will be eventually implemented - breaking what the ecosystem has been doing all along with no graceful migration path made very little sense from the get-go.

@DarkPurple141
Copy link

Completely understand that this is not going to change in the short term and the arguments made by @kamilmysliwiec are very valid.

Given this is a 'batteries-included' framework though there is a slow erosion of the framework's simple config premise occurring with more and more community packages migrating. Whatever the exact statistics of the state of the current ESM / CJS split - anecdotally at least - it seems to me packages that are actively maintained and thus more likely to be consumed are migrating or considering migrating in increasing numbers.

Eg. A little while back I a added markdown parsing service via unified but it requires a compile step to work as the package is all esm.

So adding newer packages; be it to get new APIs, patch vulns, meet node compat requirements whatever will push more packages down this route.

Perhaps the answer is to simply lean on node's require(esm) behaviour but it feels like the kind of paper cut that is going to get more and more annoying for new users.

@kamilmysliwiec
Copy link
Member

I'm not saying we will never migrate - I'm just trying to emphasize it doesn't seem reasonable atm.

@elovin
Copy link

elovin commented Mar 29, 2024

@kamilmysliwiec

I hope that (almost) full interoperability between ESM and CJS will be eventually implemented - breaking what the ecosystem has been doing all along with no graceful migration path made very little sense from the get-go.

I think that has already been implemented in node 21.1.0 --experimental-detect-module, it has also been backported to 20.10.0.

Okay, never mind, it is just one more step in the right direction I guess.

@GerbenRampaart
Copy link

denoland/deno#20619

@Diluka
Copy link
Contributor

Diluka commented May 9, 2024

you can use ESM-only packages in CommonJS projects just fine. It's a nodejs feature, not related with nestjs.

const axios_curlirize_1 = tslib_1.__importDefault(require("axios-curlirize"));
                                                  ^
Error [ERR_REQUIRE_ESM]: require() of ES Module /node_modules/axios-curlirize/src/main.js from service.js not supported.
Instead change the require of main.js in service.js to a dynamic import() which is available in all CommonJS modules.

nestjs version: 10.3.8

@luskin
Copy link

luskin commented May 31, 2024

We are about to abandon NestJS for lack of ESM support. More and more packages are ESM only (lucia, superjson) and they simply do not work, even with hacks. It's unfortunate really, nestjs is a great framework but becoming more of a headache than it's worth.

@nestjs nestjs locked and limited conversation to collaborators Jun 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests