expressjs / discussions

Public discussions for the Express.js organization
62 stars 14 forks source link

Module Format #202

Open wesleytodd opened 7 months ago

wesleytodd commented 7 months ago

The project really never took a stance on this AFAIK. Since everything was CJS and it wasn't releasing breaking changes meant we really didn't need to take a stance. That said, due to the fact that ESM only modules explicitly drop support for the majority of the ecosystem, I would propose that the project commit to one of two options for all the packages:

  1. Stick with CJS until the status changes (and pick some clear trip wire points to revisit the decision)
  2. Dual publish. This would require some build tooling which we traditionally didn't need, but would keep ecosystem support.

This came up because of this issue in multer: https://github.com/expressjs/multer/issues/1245

ljharb commented 7 months ago

There’s no value I’m aware of for dual publishing except for those who want to avoid a bundling process; all it does is add complexity. Since express runs on the server and browsers are irrelevant, i think option 1 makes the most sense.

jonchurch commented 7 months ago

No matter the choice, people will be upset we did not choose their preference. This is where goals and vision for the project become essential, as there isn't a clear purely technical answer to what is right. Grounding the choice in stated goals is not just responsible, but useful for when people do throw stones. We can point to the project's goals and let them decide if our goals align with theirs.

That said I see two big points of consideration:

  1. It is a fact that some modules we have in our dep tree have gone native esm in their majors over the past 5 years.
  2. It is my opinion that going native ESM will lock out many folks in the Node ecosystem from upgrading.

For point one, its only a problem if we dont go esm. If not we have a problem. Solving that problem means using a build step, or taking on maintainer burden. When problems are hard and the easy way around will break people, taking on maintainer burden makes sense (assuming breaking them in this way is a nongoal). We can rewrite things to allow for dynamic const thing = await import("foo") of dependencies where necessary. The work entailed in this is likely nontrivial, but I havent scoped it so ¯_(ツ)_/¯

For point 2, we can serve both cjs and esm style imports more easily if the API exposes only one entrypoint, which I believe is the fact of the matter today. We can utilize what Ive seen Matteo call the "infamous triplet", which is used in fastify. (direct github link).

/**
 * These export configurations enable JS and TS developers
 * to consumer fastify in whatever way best suits their needs.
 * Some examples of supported import syntax includes:
 * - `const fastify = require('fastify')`
 * - `const { fastify } = require('fastify')`
 * - `import * as Fastify from 'fastify'`
 * - `import { fastify, TSC_definition } from 'fastify'`
 * - `import fastify from 'fastify'`
 * - `import fastify, { TSC_definition } from 'fastify'`
 */
module.exports = fastify
module.exports.fastify = fastify
module.exports.default = fastify

My opinion, let's keep compat with as many people as we can. Let's pursue options to do so without dual publishing or a build step.

jonchurch commented 7 months ago

@ljharb

There’s no value I’m aware of for dual publishing except for those who want to avoid a bundling process; all it does is add complexity. Since express runs on the server and browsers are irrelevant, i think option 1 makes the most sense.

This is true for express, and less true for packages under the express umbrella which are depended upon widely in the rest of the ecosystem and end up in the browser. For most of those, consumers may be well served by a dual publish. (path-to-regexp can't be the only example that fits that definition of widely depended upon cross environment, surely?)

This thread is in the context of npm i express as I understand it. But under the guidance of their respective lead maintainers, it is very likely that some packages may want to go native ESM in the future, and some may already be dual publishing. Express's use of CJS may be the thing preventing some maintainers from going native ESM today.

Having a strategy to consume native ESM in express would be prudent, assuming it is possible without breaking a lot of things in express in crappy ways. Otherwise we paint oursevles into a corner where we have to depart from the ecosystem and fork deps ourselves to keep them CJS friendly, or even transpile and publish the output 🤮

ljharb commented 7 months ago

I'd love to hear more about the actual benefits of dual-publishing for anyone - beyond "you don't need a build process", which is an idealistic and illusory zeno's bridge.

mercmobily commented 7 months ago

[Edit: everybody watch mercmobily embarrass himself after forgetting how EMS/CJS interaction works, and trying to tell seasoned developers who serve a gazillion users how things are...! Lucky I started with "I might be missing something here", because I WAS...]

I might be missing something here... but.... My experience, as a developer who maintains both ESM nodules and CJS ones, is that moving a module to ESM is a step that changes basically nothing for the consumers. CJS projects can still require() ESM ones, without even realising that anything has changed.

Who exactly would be upset if Express were published as an ESM?

I think it would be much better to see the next version of Express using ESM, since that's where everybody else is going -- and frankly where they should be going.

ljharb commented 7 months ago

@mercmobily that's entirely false; CJS projects absolutely can not require ESM projects - ESM can only be consumed synchronously by ESM; it's CJS that's the universally available format.

mercmobily commented 7 months ago

@mercmobily that's entirely false; CJS projects absolutely can not require ESM projects - ESM can only be consumed synchronously by ESM; it's CJS that's the universally available format.

You are 150% correct and I am totally wrong.

ljharb commented 7 months ago

No worries, module formats and how that interacts with packages are confusing to virtually everyone.

mercmobily commented 7 months ago

@ljharb Jordan, what I meant was "for the consumers who move their projects to ESM".

I am sorry -- it's been so so so long since I wrote anything using CJS, or even dealt with CJS code, that I was basically affected by tunnel vision in terms of what was possible.

For what is worth, once you write projects using EJS, you can do pretty much anything. It doesn't matter if a module is ESM, CJS... everything just works. If your project is CJS, you are obviously not as lucky (something I had basically forgotten, since it's been years). However, at the same time, Express being something used by a gazillion users, there is a perspective change (something I am personally not used to).

Apologies.

ljharb commented 7 months ago

I totally agree - for applications, even I would only use native ESM at this point (unless i cared about performance or instrumentation/monitoring, but that will actually improve with time) - just not for packages.

jonchurch commented 7 months ago

@mercmobily

[Edit: everybody watch mercmobily embarrass himself after forgetting how EMS/CJS interaction works, and trying to tell seasoned developers who serve a gazillion users how things are...! Lucky I started with "I might be missing something here", because I WAS...]

No need to prostrate yourself. Plus I always have to check these details when speaking about it bc it is confusing as heck. The fact that most of us don't have to think about this regularly is due either to build tooling or maintainers taking on the burden to abstract it from consumers.

We're discussing which burden we want to take on 😂

Here's the maintainer of redux's account of being out of their element in this area and figuring it out painstakingly.

(this can be marked off topic but was worth saying)

kibertoad commented 7 months ago

Key problem that I've experienced with trying to go ESM is that TypeScript ESM app would fail to import TypeScript CommonJS lib. See https://github.com/privatenumber/tsx/issues/38 for more in-depth description of the problem.

Btw, any thoughts on using TypeScript or at least bundling the types?

ljharb commented 7 months ago

probably should be a separate issue for discussing types. I've started to publish some packages with handwritten d.ts files and jsdoc comments inline in the JS; it seems to work rather well without adding a build process.

kibertoad commented 7 months ago

Can you create an issue? +1 on handwritten .d.ts

ljharb commented 7 months ago

203. (express folks, please feel free to hide my type-related comments as off-topic)