-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
Make Node.js more permissive when libraries ship invalid ESM code #46074
Comments
I strongly disagree with the kind of framing you're trying to do here. Next.js has been around for over 6 years at this point and 6 years ago the ESM space looked significantly different from today. Over time webpack improved ESM support and added support for strict ESM resolving. We even shipped that initially but found that the overall ecosystem wasn't ready for it through a large amount of issues/feedback probably similar to what you've been receiving. We've always favored backwards compatibility so that the surface of breaking existing applications is small when upgrading between major versions. Many of these packages were published even before Next.js even existed or became a popular tool of choice. To further clarify: npm packages that are opted into ESM |
@timneutkens I meant no offense, and apologies in case you took it personally (I didn't mean to). I'm just speaking from the perspective of someone implementing a strict implementation (here the Node.js team) who may think "well, we've done a correct implementation and the permissive implementation should fix itself". I actually don't have any "problem" with Next.js/webpack being permissive here. There are other areas about Next.js I've a problelm with, but Next.js being permissive about ESM isn't one of them 😉. As things stand today, and regardless of why it came to be, the only option I see moving forward is for Node.js to become more permissive. |
I assume you're going to remove these mentions from the issue as they're both wrong? As said Next.js doesn't even handle packages that publish ESM any different and as such guides you towards publishing correct ESM packages while interop with the larger ecosystem is kept. |
@timneutkens The first sentence is actually correct (feel free to PM me if you want me to elaborate), and I've just edited the second sentence. Let's make this ticket about the technical aspects and not something personal. I truly mean no offense and my sole intention is to be constructive. |
There is nothing personal in this thread. You're trying to push two narratives that are untrue:
Packages are not being published to npm with non-standard ESM because Next.js exists. Packages published to npm using The edits that you've made clearly show you're trying to narrate that this is a problem Next.js introduced which is not the case as pointed out in my earlier replies.
You've edited it to "one can argue" which speaks for itself that you do not want to remove the sentence that you know is wrong. |
Discussing the reasons why npm packages are being published with invalid ESM isn't the goal of this ticket. (We can discuss this if you want, but I'd suggest to PM me or open another ticket instead.) The goal here is how we can make Node.js being able to cope with the many npm packages that contain invalid ESM. |
If the that is not your goal then remove the untrue mentions of Next.js because as you're pointing it out it's not relevant to the ticket. |
(It's untrue from your perspective, but I do believe it to be true and that's why I'm keeping it. I want to keep it to provide a little bit of insights of why there are npm packages out there with invalid ESM. I just PM'd you about why I believe Next.js plays a role in the current situation.) |
I see why you wanted to dm this as it proves even further that it's not a problem with Next.js but the larger ecosystem. This is what was shared in DM: aws-amplify/amplify-ui#3155 Looking at the package it's not marked as ESM, there is no The main difference between Next.js and Vite is that Next.js was created 6 years ago and already had an established userbase even before ESM became mainstream. Vite drew a line in the sand: All ESM. Next.js couldn't draw a line in the sand as it would break all existing applications (hundreds of thousands). Packages being published this way is not caused by Next.js, the majority of the packages you're referring to that have |
Chiming in, since SvelteKit was mentioned, to say that changing Node's behaviour would be a catastrophic decision. In 10 Things I Regret About Node.js, Ryan Dahl specifically mentions extension-less imports. As the creator of Rollup, I can empathise: allowing modules to import each other without specifying the It's unfortunate that people are currently shipping invalid ESM, during this transitional period when people are still getting used to it and haven't yet shaken off the bad habits that CommonJS and earlier design decisions encouraged. It's unfortunate — mea culpa — that tooling allows these errors to remain hidden. It's unfortunate, even, that VSCode defaults to omitting file extensions when automatically importing modules. But none of those problems are best solved by making Node itself behave in a non-standard way. Interfaces are never made better by making them less explicit. It would be ironic if, as a result of this issue, we ended up with packages that work in Node but not other JavaScript runtimes, just as the community is finally maturing to the point that portable code is a reality. |
@Rich-Harris While I agree with your point, I'm still left wondering: what can be done about the current situation? The status quo is that a lot of packages make Node.js choke. The situation is untenable. I actuallly really like that about Next.js: I wonder if there is a way to make Node.js permissive while fostering valid ESM. E.g. integrating something like https://publint.dev to npm while making Node.js permissive. Be liberal with what you accept and strict with what you send. |
Actually, I'm realizing that if npm validates npm packages before they are being published could do the trick: npm/rfcs#665. AFAICT we wouldn't need Node.js to be permissive anymore. (Soon enough, most npm packages will have valid ESM.) I'll be closing this ticket if no one comes up with an argument in favor of making Node.js permissive. |
cc @nodejs/loaders @nodejs/modules The short answer is that Node.js has no plans to adopt the loose “try to handle anything” attitude of many bundlers. That may be acceptable for a build tool where performance isn’t the top concern, but Node.js wants to run code as fast as possible and this means being a bit less permissive about what is acceptable. Especially since there are so many great bundlers that can convert sloppy code into performant, runnable code, we’d rather not slow down Node.js for everyone when either bundlers or loaders can handle those who want to wrote nonstandard JavaScript. |
In light of @GeoffreyBooth's comment I'll go ahead and close this. |
What is the problem this feature will solve?
I'm the author of vite-plugin-ssr and a lot of users are having problems because many libraries ship invalid ESM code, which makes Node.js choke.
(The reason why many libraries ship erronous ESM code is because frameworks like Next.js bundle the server-side code of libraries. Where Next.js is permissive and is able to process invalid ESM code, Node.js is strict and throws an error upon invalid ESM code.)
More information at https://vite-plugin-ssr.com/broken-npm-package#solution.
This is a concern not only for vite-plugin-ssr, but also for the whole Vite ecosystem such as Nuxt and SvelteKit (see SvelteKit FAQ - How do I fix the error I'm getting trying to include a package? which is, I quote, "issues most commonly affecting SvelteKit users").
The situation is quite bad and the feedback I get from users is quite vivid, to say the least.
While one could say that it's "Next.js's fault of being permissive" (webpack more precisely speaking), the situation is what it is and it's not going to change anytime soon. So I propose following practical solution.
What is the feature you are proposing to solve the problem?
Make Node.js more permissive.
For example, a widespread problem is npm packages having invalid ESM imports, e.g.
import './foo'
instead ofimport './foo.js'
, which makes Node.js choke. Another example is packages that ship ESM but don't havetype: "module"
in theirpackage.json
. Node.js should be able to execute npm packages that ship invalid code.I'd make Node.js permissive only for npm packages (i.e. code living in
node_modules
), while being more strict for the user land.What alternatives have you considered?
[Edit] An alternative is to make npm validate packages before they are published: npm/rfcs#665.
The text was updated successfully, but these errors were encountered: