nodejs / TSC

The Node.js Technical Steering Committee
575 stars 127 forks source link

When to make `--default-type=module` the Node.js default #1445

Closed GeoffreyBooth closed 11 months ago

GeoffreyBooth commented 11 months ago

Now that https://github.com/nodejs/node/pull/49869 has landed, and we need to figure out what to say about it for the 21.0.0 announcement, we should agree on a tentative plan for when the default value of --default-type flips from commonjs to module. That would be a semver-major change.

Obviously we can’t announce a certain date or version, since we don’t know what issues may arise during 21.x as people test out --experimental-default-type=module and when we might consider it stable, but assuming things go fairly smoothly then we could conceivably make the flip in 22.0.0 in April 2024. I think that would probably be preferable to 23.0.0 in October 2024, which would mean that the first LTS line with ESM by default wouldn’t begin until 24.x in April 2025, which is a long way off.

I think for the health of the project and the greater JavaScript ecosystem it’s important for Node.js to shift to an ESM-first mindset. We will continue to support CommonJS as a first-class module system, but it becomes opt-in rather than opt-out, conforming Node with other runtimes and modern tools.

This is primarily a change for new users. Most users learning JavaScript today are learning it with import/export syntax and get frustrated when Node doesn’t run their code right away without messing with configuration settings. I think it’s important that we prioritize the UX of these users. The users still writing CommonJS today are among our most sophisticated and experienced, and will be able to figure out how to opt into the old mode via --default-type or the "type" field.

GeoffreyBooth commented 11 months ago

For those wondering, this is what the migration paths look like:

Or, of course, they could refactor code from CommonJS to ESM, but I’m not sure that needs to be presented as an option. This is nothing like Python 2 to 3.

The other significant migration is of packages that rely on monkey-patching the CommonJS loader. The authors of major instrumentation libraries and of Yarn Plug-n-Play have been very active engaging with the loaders team in ensuring that the module customization hooks can be used to support their use cases. Those APIs are already in “release candidate” stage, just baking to see if anyone reports any bugs before we declare them stable. Regardless of the --default-type flip, though, it’s important for the maintainability of the modules subsystem that anyone who currently monkey-patches moves away from that pattern and onto public APIs. The need to support monkey-patching use cases has severely hampered our ability to do significant refactoring or improvements of the modules code. Substantial performance achievements, such as could be attained by moving parts of the modules code into C++, can only be done if we can break the already unsupported monkey-patching use cases. Having an official announcement that the ESM loader will become the default module loader in a future Node version will hopefully be what corporate teams need to prioritize the work to move away from monkey-patching.

ljharb commented 11 months ago

Doesn’t this mean that users writing a library would have different default behavior for themselves than for when they’re installed in someone else’s node_modules?

mcollina commented 11 months ago

Doesn’t this mean that users writing a library would have different default behavior for themselves than for when they’re installed in someone else’s node_modules?

Yes. I think changing that is going to a be a massive breaking change that would break everybody. Most modules would need to keep working with the flag.

Other alternatives are

  1. deprecate typeless packages outside of node_modules
  2. ask npm init and the like to include a type property by default

I would personally prefer to ship this in 23, having v24 the first LTS with the change, but I'm not opposed to an accelerated timeline.

If we decide not to flip the switch in v22, we could ship some form of warning in v22 for typeless packages outside of node_modules, taking a similar "iteration" approach that we used in the past.

Something to consider is the coordination with the TS team, as this will impact them and their resolution algorithms.

Of course, Loaders should be stable by then and no critical bugs should be there (or in ESM) to do the switch.

GeoffreyBooth commented 11 months ago

Yes. I think changing that is going to a be a massive breaking change that would break everybody. Most modules would need to keep working with the flag.

Let’s investigate this, shall we? Let’s go through some scenarios:

  1. ESM-only packages, like node-fetch: The flip has no effect, on either the package author or its consumers.

  2. CommonJS-only packages, like express, where the author is developing the package using a pre-flip version of Node and the consumer is running post-flip Node: The flip has no effect, on either the package author or its consumers.

  3. CommonJS-only packages, like express, where the author upgrades to the post-flip version of Node. Assume that the package uses .js extensions and the package.json file has no type field: The first time the author tries to develop their package after upgrading, it will fail to load because .js is ESM now. The error message will prompt the author to add "type": "commonjs" to their package.json. The author does so and is back to developing. No effect on consumers of the package.

  4. A new ESM-only package, where the author starts writing the library in .js files with a package.json file that has no "type" field: (Assume for the sake of evaluating worst case scenarios that we get no help from npm or build tools; they function as they do today.) The author publishes their package to npm, thinking all is fine, but any consumers of the package—both using pre- and post-flip versions of Node—see errors when they try to consume it, because in all versions of Node a type-less package under node_modules is interpreted as CommonJS. This published version of the package is effectively broken for all users, which would surely trickle back to the author, who would add "type": "module" to the package.json and publish a new version that would work for everyone. Because this is a new package, it wouldn’t have millions of consumers; the mistake would be rectified very early on, before it has a chance to grow its popularity.

So basically, package authors would need to learn to add the type field to their package.json files, and hopefully we get some help from npm or other tools reminding them to do so. But the consequences of them forgetting to do so are low: a package either works for all users or is broken for all users, which is really what we want, because broken versions won’t find any adoption. I don’t see any scenarios where either authors or consumers would use --default-type=commonjs; it’s easier and more permanent to just add "type": "commonjs". Are there any other plausible scenarios I haven’t thought of?

benjamingr commented 11 months ago

Excellent summary @GeoffreyBooth that really clarified how migration would look and it sounds very reasonable.

We should also contact npm and ask for:

@nodejs/npm does that sound reasonable?

Edit: opened an issue with npm: https://github.com/npm/cli/issues/6855

GeoffreyBooth commented 11 months ago

We should also contact npm

For reference, adding type to npm init was proposed and rejected in 2019, then proposed and rejected again in 2021, then proposed again in 2022 (still open) and again in 2023. Of course, all of those prior discussions happened before the proposal of Node changing its default, so I am optimistic that a new outcome might be possible once we make an announcement on our side.

wraithgar commented 11 months ago

Am I following this correctly, that every package that's currently published will stop working once this goes into effect, and only newer versions of those packages that have this field defined will work?

benjamingr commented 11 months ago

@wraithgar no, the issue is with the default changing outside node_modules, the issue is when people publish new versions of packages without specifying type and relying on the default being esm - which is why we want the small npm enhancement to let people know their published code requires "type" to work as it did before publishing.

joyeecheung commented 11 months ago

I think this means if you have a simple script like this:

// test.js
require('fs')

This would stop working once the flag is flipped, unless you change the file name or add a package.json. I believe there is an enormous amount of utility scripts and tutorial examples that would be affected, now the question is do we want to create this havoc in all those scripts already out there and create a python2-like situation...

joyeecheung commented 11 months ago

Also, wouldn't this break most of our tests unless we do something? (and to that extent, it could probably break a lot of other tests in other projects that are run in a similar "loose" fashion). It could also force all projects not yet migrated to do something, while we already know that there are a lot of simple projects that are not actively maintained but are still heavily depended upon in the ecosystem - or the Node.js ecosystem once favored very simple packages like this, so if an author has tons of very simple packages that do not need much maintenance since Node.js v0.x days (because who would've thought that Node.js is going to break such simple use cases), they will now be forced to update all these small packages. And many of the old reproductions of confirmed bugs would also need updating...

GeoffreyBooth commented 11 months ago

Also, wouldn’t this break most of our tests unless we do something? (and to that extent, it could probably break a lot of other tests in other projects that are run in a similar “loose” fashion). It could also force all projects not yet migrated to do something,

The “do something” is what I wrote above as the migration path for CommonJS projects with no type field: add "type": "commonjs" to package.json, or create a package.json containing only { "type": "commonjs" }. That’s it.

People seem to keep pointing at Python 2-3 as the only example of a migration, but this is nothing like that: that required everyone to refactor their code, whereas this requires no one to refactor their code. They can if they want to, but this is a configuration change not a syntax change.

Yes the “shell scripts” case is the most awkward, because people often lack package.json files for those. But as I’ve written elsewhere, those scripts aren’t our primary use case and they shouldn’t be prioritized over application developers. Per https://nodejs.org/en/about, “Node.js is designed to build scalable network applications.” Just about every application framework has instructions in ESM syntax. TypeScript has exclusively supported ESM syntax since its inception, if I’m remembering correctly. We’re more out of sync with the ecosystem by requiring ESM to be opt-in than opt-out; while there will be some old tutorials and examples that become outdated after this change, there is far more that’s the reverse. Having that code work by default in Node.js will improve the new user experience and bring us into conformity with the broader ecosystem.

joyeecheung commented 11 months ago

that required everyone to refactor their code, whereas this requires no one to refactor their code

Requiring everyone to add a new file or change file name is no less a problem, I would say, especially considering:

  1. A lot of tools that run scripts would need to take this into account
  2. git history problems and 404-like problems caused by bulk file renames
  3. It really doesn't matter what needs to be changed. A lot of the times people run into issues in their dependencies and they just have no idea how to deal with them and do not want to invest the time. They only want simple things to keep working without breakages i.e. stability. Any change that affects what the hello world everywhere tells you to do is disruptive.

Per https://nodejs.org/en/about, “Node.js is designed to build scalable network applications.”

First of all I don't think whatever we put down on that web page is regarded as our principal. There are also a lot of things on that web page is out of sync with how Node.js is used in the wild. Also on that note, the hello world on that page is still using ComomnJS.

Second of all, claiming that "Node.js is designed to build scalable network applications." does not mean Node.js is only designed for that and we should disregard other use cases, for example, tooling. Also, can one lose the ability to build a scalable network application just because the default entry type stays the way it is since >10 years ago? I doubt how much difference the default entry type makes if that's the only thing we ever care about.

But as I’ve written elsewhere, those scripts aren’t our primary use case and they shouldn’t be prioritized over application developers.

I think above all Node.js prioritizes stability and not breaking existing code unless absolutely necessary.

while there will be some old tutorials and examples that become outdated after this change

Running a .js file that allows you to use require() to load builtins is something that Node.js supports since >10 years ago, it's everywhere in the tutorials and blog posts and books, and it has also been in our hello world examples since the initial days of Node.js. There are very few things that can be even more stable than this in the Node.js API contract. I don't think they are just "some old tutorials", personally I'd be convinced that > 80% of the Node.js tutorials out there still teach you to do this and maybe only 10% of them can ever be updated. There would be countless people counting on this to continue to work. I don't think this should be simply dismissed as "not something worth to be prioritized".

joyeecheung commented 11 months ago

Out of curiosity, I googled "get started with Node.js", and the top results all rely on being able to run a .js file that require():

  1. https://nodejs.org/en/docs/guides/getting-started-guide
  2. https://www.w3schools.com/nodejs/nodejs_get_started.asp
  3. https://www.freecodecamp.org/news/introduction-to-nodejs/
  4. https://www.simplilearn.com/tutorials/nodejs-tutorial/getting-started-with-nodejs
  5. https://nodejs.dev/en/learn/
  6. https://www.pluralsight.com/guides/getting-started-with-nodejs
  7. https://code.visualstudio.com/docs/nodejs/nodejs-tutorial (assumed in screenshots)
  8. https://levelup.gitconnected.com/the-ultimate-guide-to-get-started-with-node-js-4ce54579ceb7
  9. https://cloud.google.com/nodejs/getting-started (it's actually started by google cloud, I think cloud providers need to go through the hurdle of upgrading their infra and documentations for that change too, it would be even messier factoring in support for multiple Node.js versions)
  10. https://developer.ibm.com/learningpaths/get-started-nodejs/explore-nodejs-concepts/
  11. https://developer.mozilla.org/en-US/docs/Learn/Server-side/Express_Nodejs/development_environment

I've stopped looking after glancing over the top few and none of them is teaching people to write the entry type as ESM (I didn't scroll to bottom or click around, but at least CommonJS is always the first thing being taught). And this is just the English version...

GeoffreyBooth commented 11 months ago

Yes, many guides to Node.js were written a long time ago. Most people don’t develop “Node apps” per se anymore, they develop in frameworks that build on Node, or with things like TypeScript that use Node. If you look at those docs, most use ESM syntax. @mhdawson do we have any survey data to put some numbers on how people use Node, or what they use it with, or what their preferences are regarding modules?

Here’s what happens if you use ESM in a CommonJS context today:

mkdir test && cd test
echo 'import "fs"' > entry.js
node entry.js
(node:51228) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

This would be the inverse experience when we flip the default, in that the message would say something like “Warning: To load a CommonJS module, set "type": "commonjs" in the package.json or use the .cjs extension.” It’s to the point and tells you how to fix your problem. It’s not like the code runs but there are silent errors or unexpected behaviors. We should probably improve this by adding a link to a nodejs.org page explaining in more detail, but even the brief version tells you what to do.

A lot of the times people run into issues in their dependencies and they just have no idea how to deal with them and do not want to invest the time. They only want simple things to keep working without breakages i.e. stability. Any change that affects what the hello world everywhere tells you to do is disruptive.

Because the new mode doesn’t affect anything under node_modules, any dependencies installed through common methods like npm install would be unaffected by the flip.

joyeecheung commented 11 months ago

Most people don’t develop “Node apps” per se anymore, they develop in frameworks that build on Node, or with things like TypeScript that use Node.

I don't think we should base our decision of such a wide-spread breakage on pure impressions. We need evidence that this is not going to break, say, more than 5% of Node.js users (no matter they are writing code in a framework, or just using Node.js as a runner/tool, or using Node.js inside another environment e.g. cloud provider, desktop environment, or using Node.js to run mod/plugin script), before we can break an API contract that has been by design for >10 years.

Because the new mode doesn’t affect anything under node_modules, any dependencies installed through common methods like npm install would be unaffected by the flip.

By dependencies, I mean that the Node.js application is used as a runner/build tool/a component as part of the project, while developers/users do not necessarily know how to deal with it (e.g. our configure.py kind of use case).

GeoffreyBooth commented 11 months ago

I found the data: https://github.com/nodejs/next-10/tree/main/surveys/2023-04

image image image image image

And in https://github.com/nodejs/next-10/blob/main/surveys/2023-04/Node.js%20Survey%20open%20ended%20answers.pdf, question 18 is “If you wish to use ESM in an existing application, what have been the pain points or blockers preventing you from doing so (if any)?” and there are hundreds of responses so it’s not really summarizable, but you can get a sense from the responses of what people are trying to do.

benjamingr commented 11 months ago

I don't think the comparison to Python is fair at all. This is like Python 2/3 if Python 2 code kept working.

Namely:

Python 2->3 would have been if we removed CJS entirely, removed all the non-promise callback APIs, removed all deprecated APIs and made breaking changes to promise ones and then created an incompatible Node.js binary effectively making "two Nodes". This on the other hand is just changing the default outside node_modules

joyeecheung commented 11 months ago

I found the data: https://github.com/nodejs/next-10/tree/main/surveys/2023-04

I do not think the survey is created in a way that gathers data about how a widespread breakage in the existing code matter for users. It is true that people may want and do work on transitions when they have time, but it is also true that widespread breakages are disruptive and must be done with care. The right questions should be:

  1. Are you still maintaining CommonJS code?
  2. Would you accept a change that breaks running node hello.js when hello.js is a CommonJS entry point that require() without a package.json, in order to speed up adoption of ESM?
  3. Would you accept other breakages in existing CommonJS applications in order to speed up adoption of ESM?
  4. When do you think is the right time to make ESM the default entry type by breaking the existing way of running CommonJS applications?

Even in the survey, you can see that more than 30% of the respondents do not plan to migrate to ESM in the near future. I doubt how many of our users would be happy to trade a breakage in a widespread way of running CommonJS in order to speed up migration to ESM. It's not impossible to do forever, but I think we are still very far away from that point. If this only breaks running loose CommonJS files, and if that was really just a niche use case as you described, how much value would this actually bring to the speedup of ESM migration and why is it so urgent that it need to be done in a year? Either this is very widespread and moving on would be an important milestone, or this is just a niche use case so this is one of the last thing that needs to be taken care of. I don't think it can be both niche and an important milestone.

joyeecheung commented 11 months ago

Yes, many guides to Node.js were written a long time ago.

If you check the search results, many of them are either written or updated within the last two years.

Most people don’t develop “Node apps” per se anymore, they develop in frameworks that build on Node, or with things like TypeScript that use Node.

If that's really the case, why would it matter how we run the loose scripts by default? The frameworks should take care of this for them and how the entry point is interpreted by default is the last thing they need to care about. This is also contradicting what the OP says:

This is primarily a change for new users. Most users learning JavaScript today are learning it with import/export syntax and get frustrated when Node doesn’t run their code right away without messing with configuration settings.

Either most users get started by running a loose script and need to configure something to make it run as ESM by default, or they get started by writing code using some framework and the configuration should be done by the framework. I very much doubt how many JavaScript tutorials would teach readers to run ESM code in Node.js without any extra configuration and let them assume that it would just work, but we have evidence that most top Node.js tutorials are still explicitly teaching people to write a CommonJS entry point and run it without any additional configuration. I don't think we should invalidate existing working tutorials to prioritize non-working tutorials that may or may not even exist. Following a tutorial and finding out that what it says can run does not actually run is much more frustrating than trying out code in an environmnet that the tutorials do not teach (because it's in fact not runnable) and finding out that it doesn't work.

GeoffreyBooth commented 11 months ago

If you want to, you can make any conclusions you want from that survey data. You could look at the 65-35 split of users already using ESM and say that more than two-thirds of our users are already using ESM, why shouldn’t it be the default? Or you could look at the 35% and say but this is still a large group, too large to inconvenience, etc.

I think one fair conclusion that can be drawn is that this data does prove that far and away our users are application developers. They may run some loose shell scripts, but by and large I think it’s safe to assume that the vast majority of our users are probably doing most of their Node development in apps with a package.json file and dependencies in node_modules. These users either won’t notice the default flip (if they’re in the 65-70% of ESM users) or they’ll have to add "type": "commonjs" to package.json.

I do not think the survey is created in a way that gathers data about how a widespread breakage in the existing code matter for users.

I think the framing of “widespread breakage” or “Would you accept a change that breaks” or “Would you accept other breakages” is very leading. Not all breaks are created equal. As @benjamingr wrote, there’s a big difference between adding "type": "commonjs" to package.json and refactoring from Python 2 to 3. And besides, we’re not in a position to run another 1000-plus-person survey again anytime soon. This is the data we have.

Every breaking change, like any change, has costs and benefits. The question for us is whether the benefits of this one outweigh its costs. I think they do: I think it really matters that Node join the rest of the ecosystem in prioritizing the standard module format, that we provide the best experience for users in that mode, and that we make it easier to write standards-compliant JavaScript. I think the requirement of a one-line JSON file change, or adding a flag, is a very minimal cost to impose on a minority of users in order to realize these benefits.

If you check the search results, many of them are either written or updated within the last two years.

Well then that would imply that they’re still maintained, and if we announce that this change is coming, they’ll be updated before April 2024. Regardless, we can add a link to our own tutorial in the error message that someone would get when they run CommonJS syntax in ESM mode. I don’t view this as a blocker.

joyeecheung commented 11 months ago

And besides, we’re not in a position to run another 1000-plus-person survey again anytime soon. This is the data we have.

I don't think "we are not going to do a large-scale survey" is a good response for the lack of data to support an important breakage like this. If we can't run a survey anytime soon, then we should wait until we are ready to run another survey, gather data about how breaking the breakages are, before we make a decision to break something that's been working for more than 10 years and in most top Node.js tutorials out there.

I think the requirement of a one-line JSON file change, or adding a flag, is a very minimal cost to impose on a minority of users in order to realize these benefits.

If the weight of this breakage is enough in the prioritization, then it should be a very commonly used thing that would break a lot of people, hence a huge cost. If it only breaks a minority of users, then it should not have much weight in the prioritization and it's not really worth breaking. I don't think it can only have a minimal cost while also bring a lot of weight in the migration.

joyeecheung commented 11 months ago

Well then that would imply that they’re still maintained, and if we announce that this change is coming, they’ll be updated before April 2024. Regardless, we can add a link to our own tutorial in the error message that someone would get when they run CommonJS syntax in ESM mode. I don’t view this as a blocker.

I think this is a blocker. We need to at least make sure that the top Node.js tutorials out there are teaching people to write ESM by default before we create a sense of emergency, and for example, >90% of our users are ready for the flip. If that doesn't happen, then we should postpone the flip. It's fine to announce that a change is coming, but I think a one-year notice is just too short for breaking something that's been working and assumed everywhere for more than 10 years.

GeoffreyBooth commented 11 months ago

I think what you’re saying is that this is a big deal because it affects so many users; up to a third of our user base. What I’m saying is that it’s a small deal because all it takes is a minute of effort on those users’ part to update their projects to work in the new mode. Both things are true.

If there’s a way to run another survey, sure, that would be great. @mhdawson? But I think we need to write the 21 announcement soon, so I find it hard to imagine we’ll have results in time to inform what we write. Even if we wanted to I don’t think we can promise anything in particular, because no one can predict what PRs get blocked, but I think we can say that we intend to propose flipping the default of --default-type in 22 assuming that the flag is stable by then and issues people report have been resolved. The stronger the statement we make, the more feedback we’ll get; we can even include a survey with the announcement, and that might give us data to make an informed decision by the time 22 is getting ready. And no one will hold us to our intentions; we can always decide that 22 is too early, whatever, and let it slide to 23 or 24.

joyeecheung commented 11 months ago

I think "we'll flip in 22" is a message that shows how little we care about the stability of this software. This is not a niche API that only a fraction of users know about. It's something that has been working and assumed everywhere for more than 10 years, and it's something that almost all Node.js beginner tutorials out there has been teaching. A one-year notice for that is just too hasty. The message we should give is that "we'll flip when there's sufficient evidence that less than X percent of our users would be affected by the breakage" and ideally send a survey gathering data about the breakage (basically, "if you run the Node.js code you publish or maintain with --default-type=module, would it break?" and "how many dependent of your code would you expect to be broken by this flag?" if the answer is yes).

GeoffreyBooth commented 11 months ago

The message we should give is that “we’ll flip when there’s sufficient evidence that less than X percent of our users would be affected by the breakage”

This kind of statement has no acknowledgement that we’re just changing a default configuration setting, and that it’s easy to continue working in the previous mode if desired. I read phrases like “affected by the breakage” as implying that affected users are just irreparably broken, like they can never upgrade. This is not that.

It does imply that we strongly prefer stability and backward compatibility, which we do, and I think that’s one of Node’s strengths. But a statement that says that we need to wait years to catch up with the ecosystem, to embrace a standard that was released in 2015, makes Node seem hopelessly stagnant and stuck in the past. I’d like to think that we’re a more forward-looking and faster-moving project than that.

I think we can draft a statement along the lines of this:

In 21.0.0 we ship a new flag --experimental-default-type that does {explanation here}. We encourage feedback about the new --experimental-default-type=module mode, and whether it should be made the default in a future major version of Node.js. For users not already writing ES module syntax, the migration paths look like {explanation here}. The ESM mode might become the default as early as 22.0.0 if users don’t report too many issues with the flag and it can go stable by then, and if survey responses indicate that the user base would welcome such a change. Please take this survey to let us know what you think: {link}

joyeecheung commented 11 months ago

But a statement that says that we need to wait years to catch up with the ecosystem, to embrace a standard that was released in 2015, makes Node seem hopelessly stagnant and stuck in the past.

Even browsers do not support import in a <script> tag without type="module", I would not blame a JS runtime for following what browsers do by default for stability. I think this is just the unfortunate result of the design of the standard, which should've considered the impact before getting released. This is the just reality that we have to live with. I don't think making Node.js less stable by breaking something that's still been widely assumed everywhere is the right answer.

I think we can draft a statement along the lines of this

I would say this statement only looks appropriate when the flag is no longer experimental. It still looks careless to release a timeline for such a widespread breakage based on an experimental feature that just gets out and is not yet even tested in the wild.

benjamingr commented 11 months ago

@joyeecheung to understand your position, are you concerned with changing the default at all or do you think v22 is too soon?

If the concern is about the breakage I honestly don’t understand since the breakage is super minimal, doesn’t break npm packages and only requires you to either pass a flag or edit a config. I’m not sure what survey data will get us here that will help us decide? Like, we know CommonJS is an important part of Node (for all of us hopefully). I maintain several packages that will “break” because of this (like sinon/fake-timers, tmp-promise etc) and I am not concerned about migration at all since it’s just adding a package json field at which point my code runs correctly on all Node versions.

If on the other hand you think there are missing features or design issues making you consider CJS better from a user’s point of view let’s discuss them.

I wanna make it clear I consider you a very smart and very reasonable member of Node.js and I’m not dismissing what you’re saying - I’m just confused and trying to understand the concern.

ovflowd commented 11 months ago

First of all I don't think whatever we put down on that web page is regarded as our principal. There are also a lot of things on that web page is out of sync with how Node.js is used in the wild. Also on that note, the hello world on that page is still using ComomnJS.

Id just like to chime in that we are in the process of replacing all the pages with new ones. And we will remove the old guides by new ones, and those are already with ESM by default.

ovflowd commented 11 months ago

I think one fair conclusion that can be drawn is that this data does prove that far and away our users are application developers. They may run some loose shell scripts, but by and large I think it’s safe to assume that the vast majority of our users are probably doing most of their Node development in apps

As someone from Next-10, I just wanted to mention that the audience of this survey is not necessarily a statistically significant survey. Due to the size of the test.

Not saying you're wrong or right, let's just not make decisions based on a survey, as it might or might not necessarily reflect to the broader audience/end-users/maintainers/etc.

I do agree with Joyee here that if we want to make decisions based on surveys we need a more focused survey.

ovflowd commented 11 months ago

And besides, we’re not in a position to run another 1000-plus-person survey again anytime soon. This is the data we have.

I don't think this is true. If the TSC needs another survey, we'd (Next-10) be more than happy to run another survey 🙈🙇

ovflowd commented 11 months ago

Out of curiosity question:

Could we create a smoke-test repository where we could test a number of popular scenarios and validate that the claimed behaviours of flag switching would work?

Because I do share concerns that this change has the potential to draw a lot of drama and fire towards the Node.js project as it was the potential to break a lot of things.

I share Geoffrey's enthusiasm, but I also share Joyee's concerns that being overly cautious to the extreme is the best we can do here.

I believe that at some point we should switch the default behaviour from CJS default and ESM opt-in to ESM default and CJS opt-in. But that needs to be done with a lot of warnings, prior announcements, enough time for the broader audience to proactively add the type on their packages.json;

Giving people a long period of time of adoption ensures somewhat that once the switch is done, users cannot claim that we did not give enough time or guarantees for this.

I would even wonder if we could have an adoption period where by default it is ESM but if it detects CJS without the proper flags set, instead of breaking it runs CJS normally but firing WARNS; So in theory (for a period) we would be kinda supporting CJS during the default ESM runtime if no (type module/type cjs) were defined (ie purely default mode)

That would definitely allow even end users to report these warnings to maintainers of libraries. Because even with enough announcements and baking periods, I still foresee people just not getting to know about this...

MoLow commented 11 months ago

Could we create a smoke-test repository where we could test a number of popular scenarios and validate that the claimed behaviours of flag switching would work?

Isn't that what https://github.com/nodejs/citgm is all about?

ovflowd commented 11 months ago

Could we create a smoke-test repository where we could test a number of popular scenarios and validate that the claimed behaviours of flag switching would work?

Isn't that what nodejs/citgm is all about?

Ty! I had no idea this existed 🙇

ovflowd commented 11 months ago

Also if I haven't made it clear, I am 100% in support with this, just have a few concerns 🙈

benjamingr commented 11 months ago

I believe that at some point we should switch the default behaviour from CJS default and ESM opt-in to ESM default and CJS opt-in. But that needs to be done with a lot of warnings, prior announcements, enough time for the broader audience to proactively add the type on their packages.json;

Note that even if users don't add the type field to their package.json nothing bad happens, the package is still published to npm and Node still treats it as CommonJS.

The only potential ecosystem issue is for new packages when Node defaults to esm and then they publish it to npm which does CJS by default - where they add a type field to their package.json , this can be validated by package managers like other package.json fields so we can just make sure npm does that when we ship a flpped version of Node.js

Additionally existing users of CJS projects just run with a flag or add "type": to their package.json, none of their dependencies or code break.

I would even wonder if we could have an adoption period where by default it is ESM but if it detects CJS without the proper flags set, instead of breaking it runs CJS normally but firing WARNS; So in theory (for a period) we would be kinda supporting CJS during the default ESM runtime if no (type module/type cjs) were defined (ie purely default mode)

I don't think that's technically feasible given Node's choice of two loaders for the different runtimes unlike Bun's or Webpack's "everything is both" approach in order to make it easier to write universal JavaScript. I think "file that is treated as esm unless it looks like cjs at which point it's parsed as cjs" wouldn't really work? (Interesting idea nontheless)

That would definitely allow even end users to report these warnings to maintainers of libraries. Because even with enough announcements and baking periods, I still foresee people just not getting to know about this...

We can release a version of Node that warns on no type in the package field but honestly that would probably be more annoying to users?

ovflowd commented 11 months ago

Note that even if users don't add the type field to their package.json nothing bad happens, the package is still published to npm and Node still treats it as CommonJS.

That's great, more wondering about their CI's starting to fail out of nowhere if this flip happens on an LTS version.

The only potential ecosystem issue is for new packages when Node defaults to esm and then they publish it to npm which does CJS by default - where they add a type field to their package.json , this can be validated by package managers like other package.json fields so we can just make sure npm does that when we ship a flpped version of Node.js

We might also want to reach out yarn and pnpm maintainers.

We can release a version of Node that warns on no type in the package field but honestly that would probably be more annoying to users?

We already have a few experimental warnings, they might be slightly annoying (people can easily shut them) but yet better than people getting random errors, getting frustrated, not understanding what's going on and pointing fingers towards us.

benjamingr commented 11 months ago

We already have a few experimental warnings, they might be slightly annoying (people can easily shut them) but yet better than people getting random errors, getting frustrated, not understanding what's going on and pointing fingers towards us.

Yes but they're usually annoying when:

This is annoying on the other hand because:

I think from a migration point the actual migration shouldn't be too scary - I think it's worth making sure we have consensus on ESM being the default at all (if we play the "assume migration cost is 0" game).


Do collaborators and the TSC collectively prefer ESM to CJS? If not - what's still missing? Do we want to revisit the overall approach of web compatible code?

For example, here's an alternative* we may want to have the new default for .js files work in hybrid mode and load both esm and CJS and tell people who want to entirely write web-compatible code to use .mjs or .cjs. That would add no requirement to add the field to pacakge.json but some stuff will break.

* it's an example, not a proposal

ovflowd commented 11 months ago

Yes but they're usually annoying when:

Gotcha, sorry I wanted to ratify that the warning would only apply if they're on the new default mode of ESM and using CJS within, which is actually (I think) what @GeoffreyBooth suggested. Please correct me if I am wrong.

New code based on old tutorials that use CJS may (will probably) be broken. This is because ESM tutorials will guide users on how to enable ESM and CJS tutorials will not. We can and should work around this printing a helpful error message saying "change this one setting"

I agree our warning should be as meaningful as possible and even probably have a hotlink attached with some simple guides on how people can migrate to the new thing.

Also, our guide/hotlink should mentioned the most common CJS APIs that can be replaced by ESM and even point to shiny new things ESM does (such as import.meta.url in place of __dirname etc etc

I think from a migration point the actual migration shouldn't be too scary - I think it's worth making sure we have consensus on ESM being the default at all (if we play the "assume migration cost is 0" game).

Yes, I think that's not the case here (for what I got from all the comments), what is scary is nailing that we do it with enough advertisement, warnings and in the less breakable possible way.

Do collaborators and the TSC collectively prefer ESM to CJS? If not - what's still missing? Do we want to revisit the overall approach of web compatible code?

That maybe requires a survey, but at least for the website team I feel we're all in hands with ESM.

For example, here's an alternative* we may want to have the new default for .js files work in hybrid mode and load both esm and CJS and tell people who want to entirely write web-compatible code to use .mjs or .cjs. That would add no requirement to add the field to pacakge.json but some stuff will break.

I'm 100% in favour of this, and that's what I was sorta referring before. Having a temporary compatibility mode only for .js but when the compat mode is invoked having a warning, for me is probably the less breaking way out there.

mhdawson commented 11 months ago

My 2 cents is that we should be careful along the lines of what @joyeecheung said:

I don't think "we are not going to do a large-scale survey" is a good response for the lack of data to support an important breakage like this. If we can't run a survey anytime soon, then we should wait until we are ready to run another survey, gather data about how breaking the breakages are, before we make a decision to break something that's been working for more than 10 years and in most top Node.js tutorials out there.

From my perspective there has not been a substantial discussion around making the switch for the default until this thread. What I remember earlier was along the lines of "we'll add this flag and maybe in the future we'll flip the default". I don't think we should try to rush the discussion and due diligence which is needed before considering a change like that just to make the Node.js 21 release announcement.

joyeecheung commented 11 months ago

to understand your position, are you concerned with changing the default at all or do you think v22 is too soon?

As I said before, I think changing the default is acceptable when we have sufficient evidence that this is not going to break, say, >5% of our users. I don't think the project is in a position to pile work on people to upgrade when it's not some kind of 0-day situation and more like "we think this is the future and we want you to upgrade, we don't care how many snippets/scripts/packages you have that needs this upgrade or how much time it would take you to upgrade them all, you either spend the time and do what we say or we'll break you soon". I think it is fair to say ">95% of the ecosystem have upgraded, follow the crowd and do it pal", but it's not fair to say "The Node.js TSC wants everyone to upgrade before Oct 2024, do it pal". Node.js claims to be stable at this point and we should prioritize stability. It's also upsetting that when we have evidence that the top Node.js tutorials are going to break because of this change yet it's dismissed as not worth being cared about. If even most of the hello world examples of top tutorials out there would be broken by this flip, we need to wait until they don't before we flip it. I am not convinced that we should shrug it off when Node.js beginners everywhere start to see errors when literally running a basic hello world that has been run in the same way for more than 10 years.

Also TBH I think the narrative that "breaking loose scripts is important in the migration" and ""loose scripts are a very niche use case" are conflicting. If they are only used by very few people, why do they matter in the migration, and why can't we just leave them alone? If they are only a niche use case, I think it's very fair for the final situation to be "Just like you need type="module" to use import statements in <script> tags in browsers, in Node.js you also need to pass --default-type="module" to the node command". If they are not a niche use case, then we should have a longer notice for the migration and wait until the potential breakages drops.

mcollina commented 11 months ago

How about we add an environment variable for this? It sounds like something people might want to change system wide, knowing they are doing so. Flags won't cut in this case.

It's a bit of a chicken and egg situation. I also think we should ask (again) npm to switch the default for new modules etc.

benjamingr commented 11 months ago

Thank you for clarifying @joyeecheung 🙏

By design this change "breaks" many users - I suspect more than 5% is very likely and in particular people using TypeScript who likely think they'e using esm but most of them are likely using cjs with transpilation. I thought that was "common knowledge", in retrospect maybe we need to make sure we can get consensus on that.

I'm not sure I agree with the term broken to describe the whole thing though since the flow for a tutorial from several years ago would become:

Another question if I may, do you find it unacceptable for Node.js to make those users edit their package.json file to "unbreak" or is this more about making sure we communicate it well and consider it carefully?

benjamingr commented 11 months ago

(And to be clear I believe Joyee's and others' points about making sure we do this rollout in a way that serves our community is important and I agree with the importance of these concerns)

joyeecheung commented 11 months ago

How about we add an environment variable for this? It sounds like something people might want to change system wide, knowing they are doing so. Flags won't cut in this case.

Doesn't NODE_OPTIONS automatically include all flags?

ovflowd commented 11 months ago

How about we add an environment variable for this? It sounds like something people might want to change system wide, knowing they are doing so. Flags won't cut in this case.

Doesn't NODE_OPTIONS automatically include all flags?

Was going to say that

joyeecheung commented 11 months ago

User runs code snippet and gets an error asking them to define a "type" field in their package.json or to add a flag User does either of those things and completes the tutorial successfully

I think this is overestimating beginners' ability to trouble shoot problems. First of all if they are doing a basic hello world, package.json is not something that they immediately know of. Second of all they may not even have a good idea about how command line flags work (yes command line flags can be scary for beginners, and Node.js has become popular enough that this can be the first real command line tool that beginners use for learning server development or writing a tool or just programming in general). It's just frustrating that a hello world example does not even run without troubleshooting. Factor in that we are still lagging behind in translation of error messages, the more complex the error message is, the more frustrating the experience it is for beginners who do not speak English as their first language and are following a non-English tutorial.

GeoffreyBooth commented 11 months ago

Doesn't NODE_OPTIONS automatically include all flags?

No, but if I neglected to add --default-type it should be added. I think --import might have been mistakenly excluded too.

joyeecheung commented 11 months ago

By the way, I don't think "breaking node index.js where index.js has require()" is necessarily tied to the recently introduced and not yet released --experimental-default-type flag. There are surely many other solutions to keep the old hello world working. Just throwing ideas around - we can even consider exposing require() that only accepts builtin IDs to ESM (and throw errors once it ends up not in the list of builtins, or we can be evil and allow user modules if we really don't want to break anyone). Sure, it's not nice that we pollute the ESM namesapce, but we already pollute it with a bunch of other globals anyway, and the pros of not breaking hello worlds may be worth the pollution. But anyway that's just an idea, we can come up with more ideas that makes the transition nicer. My point is we don't really need to die on the hill of this unreleased flag alone. We can say in the release announcement that this is just one way to check if your applications are future-proof. And we are working on other solutions to make the transition smoother.

benjamingr commented 11 months ago

@joyeecheung thanks, I think I see your point about beginners. How do you feel about what bun is doing for example where require is exposed to ESM natively with the only limitation is that you can't require a module with top-level-await?

I know this is entirely different than what we current have (web compatible ESM mode and Node only CommonJS mode) but I've heard pretty good things about it from people who tried it and it's arguably how most developers work anyway (with bundlers and transpilers that do this).

Node can do this if no type field is specified but if either esm or commonjs are specified the current behavior is still preserved.

It's the only option I have that meets the "tutorial example" you had other than Node literally giving beginners a button/option to re-run the code with the flag automatically or something similar.

GeoffreyBooth commented 11 months ago

Regarding tutorials, it makes sense that the “hello world,” start-here tutorials for Node would use CommonJS syntax. To use ESM they would have to explain first that there are two module systems/syntaxes, and then how to opt in to ESM. That’s a lot to stuff in before getting to “hello world.” I don’t really expect tutorials to update until after we make the switch. They would say something like “For Node < 22, click here” or something, to link to the old version; or just drop it, since anyone doing a tutorial is on the latest version. So saying that we can’t update until tutorials update is a bit of a catch-22. Though two of the top ten tutorials are ours, on nodejs.org and nodejs.dev, so we could update those as part of the flip.

I read through the 325 responses to the last question of https://github.com/nodejs/next-10/blob/main/surveys/2023-04/Node.js%20Survey%20open%20ended%20answers.pdf, “Q19: Do you have any recurring issues when using Node.js?”. I filtered out the ones that were unrelated to the modules system (and I excluded complaints about npm or other package managers as out of scope) and I was left with 88 responses, or 27% of all responses. (If you’re curious, my guess at the other most popular topics are debugging and performance.) These are the module-related complaints:

* “type”: “module” should be the default when running npm init. Libraries should not use CJS by default * A lot of time spent configuring Typescript with Node.js for full custom projects * Backward compatibility with non ESM packages * Basic typescript types, cold start times * Breaking changes in JSON Z imports between minor releases * CJS * CJS must die * Commonjs not supported any more with esm * Debugging, better typescript support. * Dependencies In ESM * Direct typescript execution * error handling is terrible & no built in typescript support * errors wont appear untill runtime (in JS) other than in TS * es6 features doesn’t work OK * ESM * ESM in general when using third party packages * ESM support is infuriating. * ESM support, non-descriptive error traces (specially for async code and chain of Promises) * ESM vs CJS * Honestly, eslint and typescript should be basic features at this point. I can’t imagine building things without them. * https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along- * I would like to have native support of typescript in Node.js * I’d love if package.json could add another object for global dependencies. * If node could strip type annotations and run TS directly that would be slay * Im a purist, I want JS, some people around me push for TS… they can all move to zimbabwe and have a nice life away from me * Import and require issue * import/require: incompatibilities with packages * Importing commonjs modules from esm * importmap, native websocket, create and assert snapshot in nodejs test runner * In a reflection on my screen, I see a silhouette of Ryan Dahl shaking his head in disapproval whenever I use package.json for custom metadata. * Incompatible library versions * Incompatiblity with some libraries when updating to the latest version. Had problem with nuxt at some time after nodejs version upgrade * Interoperability between ESM & CommonJS * issues with typescript support issues with ESM modules still hard to work with unexpected exceptions in runtime still hard to detect which user blocked the execution and close it (fail fast) * It’s hard to use it with Typescript * It’s related to different packages not updated with old version of node js * Lack of out-of-the-box support for TypeScript * Lack of types * Lack of TypeScript and enterprise language features * Libs * Memory leaks are hard to track down in production - First-class typescript support would be huge * Minors issues related to external modules * Missing or complicated support of ESM * More ecosystem related. If you’re not staying on the edge of the trends, tech debt accumulates (example: when request package was deprecated). * Mostly libraries incompatibilities, node_modules dependency resolution issues and non reproducible builds. * Native Typescript support should be enabled out of the box. - Nodejs should improve the way external packages are imported and the scope those have in the ecosystem. Even if you import one package you end up with 50 and it’s hard to keep track of how legit those in-depth packages are. * Need to change Node versions due to tools not being compatible with latest versions. For frontend development I have to keep Node 16 due to issues with Babel version used in my project… * Needs typescript support out of the box * No native TypeScript support * No other than ESM (import syntax) configuration and using typescript. E.g I’d like to be able to just run a .ts file like `node index.ts` ts-node is supposed to help this but maybe I’m just dumb because I often run into issues with that and module syntax. (Even when using —esm flag) * Overwhelming complexity of configuring packages for publishing (especially when using native ESM) * People often use global node packages in npm scripts, which means you have to install them before installing project dependencies. I have seen people use typescript's 'tsc' as a global module, when it should really be used as a dev dependency. * Please help push to integrate the Types as Comments, so we can finally be rid of Typescript and the horrible build step and non-standard feature implementations. * Problem with incorrect module versions, and not being able to run Typescript directly. * random errors mainly related to CJS/ESM mismatch of libraries/applications * Regular npm install / update / ci confusion by junior developers. Typescript boilerplate compiling/ts-node'ing/etc because of lack of support in node * Remove common js * security, design patterns/structure of the project node_modules wtf?(a lot dependencies) education for high-quality code TYPES * Setting up a project for a library vs an app is a bit confusing * Should improve support for ES6 modules * Size and complexity of dependencies, libraries insisting on cjs, lack on package manager interop (eg pnpm requires it's own lockfile) * Some older modules may not be used * Some problems with node_modules folder and npm-packages. From time to time strange things happen, when some module does not work or works but not correctly. And sometimes the solution is a complete reinstallation of Node.js by deleting the node_modules folder. I want global node_modules folder like in pnpm or Golang modules. * Startup time, no built-in TypeScript support, mediocre docs * The ESM transition has been our painful python 2->3 moment. Maintaining async state can be messy. * The thing that missing is full integration with typescript. Or a guidance of how no JS runs with typescript. Maybe this is more applicable for typescript side * TS support * types * Types * Types * Types should have first class support. * Types, I need at least basic typing support because it’s hard to maintain large applications. * Typescript + ESM situation is a hell * Typescript and performances * Typescript compilation * typescript getting in the way. * Typescript should be embedded or at least be enabled as first class citizen. * TypeScript support still seems patchy. Error messages can be archaic. ●U * updating Node.js version while keeping all global deps (via NVM) slow startup time for CLI commands (esp. if they are written in TS and need –loader ts-node/esm/transpile-only * Want to forget the commonjs fro forever * wishing to have typescript “out of the box” * working with commonjs and ESM always gives the error “cannot use import” Also, sometimes node processes die in production for lack of memory and there isnt a clear error message for explaing what happened Monorepo tooling should have first class support * Would really appreciate naive typescript support and easier project setup * Yep, especially when figuring out how to work with dozens of module import/export approaches. Mixing them together when migrating to TypeScript or just to a new module system. I wish there were an ultimate consolidated solution to all these problems. * Yes, importing CommonJS from ESM * Yes, when using packages there is poor understanding of the mode in which node.js will run my script. If we could have .noderc configuration to be more specific about runtime configuration and ways to process files - would be way more easier

I draw a few conclusions from this: