[RRFC] Validate ESM before npm publish

[brillout]

Motivation ("The Why")

Many npm packages contain invalid ESM, which makes Node.js choke.

The situation is quite bad, as described in nodejs/node#46074.

Example

For example, the npm package @aws-amplify/ui-react contains invalid ESM, see aws-amplify/amplify-ui#3155.

How

Current Behaviour

Npm packages can be published while containing invalid ESM.

Desired Behaviour

Validate npm packages before they are published: npm should reject any npm publish that publishes invalid ESM.

References

For reference, https://publint.dev is a project that validates whether an npm package is published correctly.

[ljharb]

npm just wraps up a tarball and publishes it - it doesn't validate CJS packages, why would it validate ESM ones? What about packages that support multiple node versions - how would npm validate "not the current node version"?

[brillout]

@ljharb The bottom line is that a lot of npm packages are being published with invalid ESM that make Node.js choke. The question here is: what can we do about it? As discussed in nodejs/node#46074, we don't want to make Node.js permissive if we don't strictly need to. So that leaves us with the only option of making npm strict about the code it hosts. Or maybe you're seeing another solution I'm not seeing?

The situation is untenable and we really need a solution. (I'm the author of vite-plugin-ssr and the, by far, number 1 issue users are reporting is Node.js chocking on npm packages having invalid ESM.)

What about packages that support multiple node versions - how would npm validate "not the current node version"?

The validation doesn't have to cover everything. Only validating basics things like the following would make a big impact already:

a widespread problem is npm packages having invalid ESM imports, e.g. import './foo' instead of import './foo.js', which makes Node.js choke. Another example is packages that ship ESM but don't have type: "module" in their package.json. Node.js should be able to execute npm packages that ship invalid code.

[ljharb]

npm can't and shouldn't be strict about what it hosts, and i very much agree node can't and shouldn't be permissive - so the only solution is education. Very few packages even need to be ESM (or should be!) and that's part of education too.

[brillout]

I understand npm should be agnostic. But I'm thinking the situation is so bad that it warrants an exception.

As much as I'd like education to be the solution, I'm afraid it doesn't cut it. I already have documentation about invalid ESM on https://vite-plugin-ssr.com but it doesn't make much of a difference. I'm afraid nothing will happen if we close this ticket. On the other hand, if npm validates ESM, then that would completely resolve the situation. That'd be a godsend for a lot of users. Also a godsend for library maintainers (e.g. I've already reached the Apollo GraphQL team 3 times because they repeatedly shipped invalid ESM code).

To quote a user hitting a Node.js issue because an npm package has invalid ESM:

Now there is some cryptic node bug that showed up out of nowhere
JS libraries are a special kind of punishment

The situation is painful.

I'm thinking that the (only?) way forward here is to make npm validate ESM.

[ljharb]

Based on the discussion in that node thread, it seems like the reason it's painful it's vite's decision to "draw a line in the sand". It doesn't seem problematic with other bundlers - am i missing something?

[brillout]

Vite transpiles only what is strictly necessary (that's why it's so much faster). This means that with Vite, Node.js directly executes code (whereas Next.js transpiles the code before Node.js executes it). That's why the situation is worse in the Vite ecosystem.

It also affects folks equally badly who directly use Node.js (without a bundler like Next.js/webpack).

[ljharb]

right, but the pain you're describing seems like it is the very necessity that needs more transpilation.

[bnb]

-1 on having npm check this. This should be tooling built by the ecosystem, not something Node.js nor npm maintain. Also, having npm do this would not actually solve the problem - there are other package managers 😅

[brillout]

One of Vite's foundational innovation is "lazy-transpiling": only transpile what is strictly necessary. Transpiling all npm packages only because some contain invalid ESM is a massive performance degradation. Vite values performance a lot and this isn't going to change.

I think everyone can agree that we should foster valid ESM, and that it would be a good thing if npm hosts ESM that is valid. (It doens't have to be 100% valid: a few basic checks can get us a long way.)

It's also nice for folks who don't use bundlers (e.g. Node.js servers without frontend) to be able to directly use npm packages without the need for a transpiler.

Making npm validate ESM is a very effective way of achieving this.

I understand the general principle of keeping npm agnostic but I strongly believe we should do an exception as I think the pros outweigth the cons, by far.

The most important here: it's a net win for everyone, starting from the users who suffer from this. Also including library authors who will be relieved to know that they shipped valid ESM. (I can imagine that the Apollo GraphQL would love this.)

[ljharb]

That's fine - but a package using the "module" field, and not the "exports" field, is one that it is necessary to transpile. Similarly, if a package is published with syntax that isn't supported by the target environments, the only choices are to transpile it, or, not use that package - "use it as-is" isn't a viable choice.

[ruyadorno]

@brillout a good place to start might be having package authors opting into some kind of validation (like publint that you mentioned), e.g:

$ npm pkg set "scripts.prepublishOnly=npx publint"

This might be a simple patch to submit to a few projects/teams (like in your example with the Apollo GraphQL team) to try and see if you can get some meaningful improvement to the ecosystem prior to having it builtin to package managers.

[brillout]

The way I see it is this: it's either pain for (Vite / Node.js server) users, or npm validates ESM.

I'd love an alternative as much as you do, but I don't see any.

The unfortunate truth is that users are suffering, and I wonder what we can we do about it.

[brillout]

@ruyadorno Sounds nice, although it's virtually impossible to educate the entire ecosystem to do this. That's why I believe in npm validating ESM, and so by default.