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

Suggestion for dropping Yarn from Node 14 release #1238

Closed
nschonni opened this issue Apr 3, 2020 · 39 comments
Closed

Suggestion for dropping Yarn from Node 14 release #1238

nschonni opened this issue Apr 3, 2020 · 39 comments

Comments

@nschonni
Copy link
Member

nschonni commented Apr 3, 2020

Splitting off this discussion from #777 since there is around 3 weeks till Node 14 is supposed to be released. This represents a good time to remove the support from the image for v14 without requiring a breaking change later.

Why?

How?

  • Update move the Yarn templating to update.sh and only append it when Node < 14

This would still keep Yarn in the <14 images till v12 hits EOL in April 2022, and follows a similar approach as the "OnBuild" deprecation by not adding it to newer versions after 8.
This wouldn't preclude a separate image with Yarn being taken back up by the Yarn project as they did in the past, giving them more control over tagging of their version releases.

@nschonni nschonni changed the title Suggestion for drop Yarn from Node 14 release Suggestion for dropping Yarn from Node 14 release Apr 3, 2020
@PeterDaveHello
Copy link
Member

I'm good with it. Let's cc other team members @nodejs/docker

@PeterDaveHello PeterDaveHello pinned this issue Apr 8, 2020
@SimenB
Copy link
Member

SimenB commented Apr 8, 2020

I'm still hugely 👎 on removing yarn. AFAIK nothing has changed since the last time we discussed this. 30+% (as far as we know) use yarn, and we should keep supporting it unless TSC says no. This is the same argument that has been stated across multiple issues by multiple people.

Issues like #1237 get raised for CVEs for Yarn, that we wouldn't patch like issues shipped with the bundled version of NPM in Node #1235

I don't see how this counts "against" yarn. We cannot patch issues with npm either, we say "wait for next node release".

Yarn 2 #1180 changed there deployment method to vendor in the binary into repos, so newer repos using Yarn shouldn't need the same global install

You still need a global yarn binary to launch the install, so it's still very much useful for us to keep shipping it.

@pesho
Copy link
Contributor

pesho commented Apr 10, 2020

Echoing what @SimenB said.

@Starefossen
Copy link
Member

I am on the fence as well.

@sam-github
Copy link
Contributor

My 2cents(CA), for what its worth.

Ideally docker-node would build images that contain only what the node.js project releases, plus the build essentials for binary addons (and -slim without it). That makes the images match the nodejs releases, as would the image update schedule.

Then, in addition, there would be images derived from it, perhaps labelled +yarn and/r +yarn2 that include yarn, rebuilt when yarn releases (and derived from the nodejs base images).

But... that's a lot of work, and someone has to do it... There is something to be said in terms of maintenance ease for simply putting the yarns into one image, and releasing only when node.js releases, but it does kind of fly in the face of docker layering.

Perhaps the yarn project should maintain yarn images themselves? Or have members who join the docker-node WG? If the image building was so automated the work was no effort, that would make it easier to maintain multiple image variants, but then the automation has to be built, which itself takes effort.

If the automation was so complete that image release was trivial, perhaps the images could even be a by-product of the nodejs release team's releases, so they would all be released at the same time from the same toolset and build process (though adding more work into the current release system might not be so attractive, still, its worth considering).

cc: @nodejs/releasers @nodejs/build

@mcollina
Copy link
Member

I am +1 in removing yarn. Given the recent turn of events between yarn and yarn2, and the fact that yarn2 is significantly incompatible with a non negligible amount of modules, I do not think we should include either in the official image.

@mmarchini
Copy link

+1 as well, having yarn on the docker image but not in our official tar might confuse users on what is distributed by us (a while back I thought we started distributing yarn on our tar because it was available in the Docker image).

@cjihrig
Copy link

cjihrig commented Apr 13, 2020

I'm +1 to removing yarn as well.

@rvagg
Copy link
Member

rvagg commented Apr 14, 2020

What's the base problem here? I must be out of the loop on recent happenings and haven't heard about this yarn2 business.

Due to demand, particularly by frontenders who started relying on yarn way more than npm:

So are there reasons we should be doing a more uniform strategy shift here?

@mcollina
Copy link
Member

Yarn 1.22 will be released next week. Once done, the 1.x branch will officially enter maintenance mode - meaning that it won't receive further releases from me except when absolutely required to patch vulnerabilities

From https://dev.to/arcanis/introducing-yarn-2-4eh1. @rvagg this article is a good read on the topic and a significant part of the dev community was not enthusiastic about yarn2.

I think the key question is if the yarn team can commit to support yarn classic (1.x) for a full LTS cycle. Considering what I’ve read so far I would not bet on it, so by vendoring yarn 1.x we will be risking breaking folks using our lts line in a few years time.

@SimenB
Copy link
Member

SimenB commented Apr 14, 2020

That blog post is somewhat out of date, more recent info is here: #1180 (comment). From my understanding it's a non-goal for yarn@2 to ever replace the global binary of yarn@1, so LTS compat shouldn't be much if an issue.

Maintenance wise there's not really any work here (or rather, that work has already been done) - the same script that updates node versions also update yarn versions.

@mcollina
Copy link
Member

That blog post is somewhat out of date, more recent info is here: #1180 (comment). From my understanding it's a non-goal for yarn@2 to ever replace the global binary of yarn@1, so LTS compat shouldn't be much if an issue.

It would be good to have some level of commitment from the yarn team.
By putting something in maintainance, I would assume that they would like/plan to retire that at some point.
I'm good with keeping yarn classic in as long as they plan to keep the maintainance up for our full LTS cycle of v14.

@arcanis
Copy link

arcanis commented Apr 14, 2020

(Answers to off-topic Yarn 2 subjects)

the fact that yarn2 is significantly incompatible with a non negligible amount of modules

That's incorrect. Yarn 2 is perfectly able to install projects using the node_modules layout if asked to do so. Even the PnP kernel is just a stricter set of rules that merely highlight preexisting problems except in very, very specific cases.

and a significant part of the dev community was not enthusiastic about yarn2.

And a significant part is, but doesn't feel like entering the drama on social networks. Don't watch your timeline, watch the activity surrounding the relevant projects.


Yarn 2 #1180 changed there deployment method to vendor in the binary into repos, so newer repos using Yarn shouldn't need the same global install

That Yarn 2 switched to a local install model doesn't change anything - in both cases a good user experience requires to have a global yarn binary, if only to shell out to the local one. Removing Yarn from the Docker image would have the same negative impact on Yarn 1 and Yarn 2 users as they would need to still install a global binary.

Regardless I don't think changing the status quo here will do any good, especially not when there's an important topic on the subject being actively discussed in nodejs/node. The timing couldn't be any more wrong.

@pesho
Copy link
Contributor

pesho commented Apr 14, 2020

Ideally docker-node would build images that contain only what the node.js project releases, plus the build essentials for binary addons (and -slim without it).

Note that if we follow that logic strictly, we should also remove the -alpine variants. Which are still not officially supported (despite the awesome unofficial builds maintained by @rvagg).

@Trott
Copy link
Member

Trott commented Apr 14, 2020

Would be curious to know if any of the discussion so far has changed @nschonni's opinion on this.

@mcollina
Copy link
Member

@arcanis is there any official LTS documentation for yarn? I could not easily find one.
It would it be good to know what the long term plan for yarn classic is, and if it will be supported for the entirety of our LTS cycle , e.g. April 2020? I think that is a key defining factor.

@sam-github
Copy link
Contributor

If yarn@1 won't be supported for the LTS life of 14.x, then it'd be dropped from the 14.x docker images during the lifetime of 14.x, which could be pretty disturbing to the community. Node.js itself has a note on this kind of eventuality:

Node.js does not support a platform version if a vendor has expired support for it. In other words, Node.js does not support running on End-of-Life (EoL) platforms. This is true regardless of entries in the table below.

Despite that we have an escape hatch, we try not to release a LTS line claiming support for something if we already know that that thing is going EOL during the node LTS period.

I don't know if docker-node has needed such a formal policy.

@mcollina
Copy link
Member

The classic documentation is here: https://classic.yarnpkg.com/lang/en/. It's linked in the main domain name header, and old links redirect to the new location.

I still cannot see anything regarding an LTS schedule for it, e.g. how long it would be supported. I

I intend to keep supporting Classic as far as security vulnerabilities are concerned for the duration of the LTS, yes.

Good to hear! Then I think we should keep bundling yarn classic in the docker image and re-evaluate for v16 then.

@nschonni
Copy link
Member Author

nschonni commented Apr 14, 2020

Would be curious to know if any of the discussion so far has changed @nschonni's opinion on this.

No, it's less about the LTS support of Yarn, and more about shipping another package manager that isn't included as part of the Node release. I'd be similarly -1 on shipping PNPM as part of the official image.

I think the base work here could be used to setup on official Yarn Docker image, where they can choose what platforms they want to support. That would be outside of this discussion of the direction for the Node official image though.

nschonni added a commit to nschonni/docker-node that referenced this issue Apr 15, 2020
@jasnell
Copy link
Member

jasnell commented Apr 15, 2020

To this point, the TSC has never taken a position with regards to official support of Yarn (1 or 2) in any official distribution. Similar to a comment that I believe @sam-github makes above, what I'd like to see are separated distributions with different LTS classifications:

That is...

  • One docker-node that is just Node.js with no npm or yarn
  • docker-node + npm (npm covered LTS)
  • docker-node + yarn1 (yarn not covered by LTS guarantees)

Given the issues around yarn2, I do not think we should have an official distribution that bundles it at all.

@SimenB
Copy link
Member

SimenB commented Apr 15, 2020

I like that idea. It's essentially what #808 tried, except that was images with package managers or not, not separate npm and yarn ones. Since #808 we've also gotten the unofficial musl builds, so in theory it'll be simpler to support for alpine as well than it was at the time.

Given the issues around yarn2, I do not think we should have an official distribution that bundles it at all.

No-one is suggesting that at this time

@gengjiawen
Copy link
Member

I am -1 on removing yarn. It's has been widely used as npm. It will break too many things rely on this, especially in CI.

@nschonni
Copy link
Member Author

It's has been widely used as npm.

@gengjiawen what are you basing that statement on?

It will break too many things rely on this, especially in CI.

Not unless they are using the "node" versionless tag. That is why it is important to make this change for 14-only before it is released

@gengjiawen
Copy link
Member

@gengjiawen what are you basing that statement on?

Single on npm 1m weekly download https://www.npmjs.com/package/yarn, let alone via os package manager.

From my experience, node:latest is very common to use, I don't see much benifit removing it other than causing inconvenience.

@nschonni
Copy link
Member Author

1M weekly downloads isn't that high of a number for an NPM package.

From my experience, node:latest is very common to use, I don't see much benifit removing it other than causing inconvenience.

Because it's not something that the Node.js project maintains or includes with it's installers at this time.

I've created an Issue over on the Yarn side to see if they'll reboot their own official image, which they used to maintain till #337

@SimenB
Copy link
Member

SimenB commented Apr 21, 2020

1M weekly downloads isn't that high of a number for an NPM package.

It's quite high for a package which isn't installed as a dependency, but as a global tool. You can't compare it to glob or something like that

Yarn is the 24th most downloaded package from homebrew the last 365 days: https://formulae.brew.sh/analytics/install/365d/

It's not a niche tool.

@arcanis
Copy link

arcanis commented Apr 21, 2020

Because it's not something that the Node.js project maintains or includes with it's installers at this time.

I've found this type of arguments a bit lopsided in the past:

  • They often originate from people who don't often use alternate package managers as part of their workflows. There's nothing wrong with that by itself, but I feel like extra care should then be put into understanding the impact for people that do use them, which I don't find here.

    As an example, the original post doesn't mention at all Yarn's usage rate - and thus which portion of the ecosystem will become second-class citizen. It's not easy to find it, but I feel like it's a critical part of the context that shouldn't have been omitted (polls show it a consistent ~30%-40%).

  • Second they typically address the problem in very focused ways, ignoring other options. @nschonni If the problem is that Yarn isn't acknowledged by Node, why do you jump to the conclusion that its support should be removed? Why not instead suggest to properly support it at the foundation level? Which is exactly what we're doing at [Feature] Discussion around default package manager choice node#15244.

In its current state, I personally don't find the orginal proposal very appealing:

And of course it would let the Yarn team know once again that we're second-class citizen, pushed back behind the glorious npm. Which is frankly tiring, not that anyone cares.

what I'd like to see are separated distributions with different LTS classifications:

That would be reasonable, although I think it depends on the outcome of nodejs/node#15244. If a jumper system is agreed upon, there will be no need for different images.

Generally, what I'm worried about is to change the usage instructions ("don't use node, use node-yarn") to change them back a release later ("don't use node-yarn, use node"). Because then, why not just directly go to the second step?

@mcollina
Copy link
Member

Given that v14 is going to be releases today and there is no agreement on this issue, I would recommend yarn1 to be included in the v14 image and wait for the decision of nodejs/node#15244.

@MylesBorins
Copy link
Contributor

I would like to second what @mcollina has brought up. Do we have a timeline on the release of docker images for 14?

@PeterDaveHello
Copy link
Member

As it's still under discussion and unlikely to remove yarn@1, I'll still do the same thing on v14 release here.

@SimenB
Copy link
Member

SimenB commented Apr 21, 2020

I've opened up a v14 PR now, keeping the status quo (i.e. yarn@1 is still included). Would love to investigate the "no package manager" thing regardless of nodejs/node#15244 since I think it makes sense no matter if we keep yarn around or not, especially with multi-stage builds.

@nschonni
Copy link
Member Author

@SimenB and @Starefossen have landed the v14 PR already, so there is no point of further discussion here since the window for making this change has been closed

@ljharb
Copy link
Member

ljharb commented Apr 23, 2020

@nschonni there's still a point, because it can still be made for v15 - and it should be discussed now, not in a year when the window is tight.

@nschonni
Copy link
Member Author

@ljharb this one could be re-opened, but there are already existing discussion around splitting out Yarn in issues like #777. I opened this one to get a targeted decision for v14 which didn't work out.

@ljharb
Copy link
Member

ljharb commented Apr 24, 2020

Can we get a targeted decision for v15, far in advance of its release?

@Trott
Copy link
Member

Trott commented Apr 26, 2020

Can we get a targeted decision for v15, far in advance of its release?

@ljharb If that's desirable, then perhaps the thing to do is open a new issue proposing it. The problem with getting a decision this far out is that (a) there doesn't really seem to be consensus around whether or not this should be done and (b) six months out is a long time and a lot can change. If there's anything evolving in the yarn project in particular, it could affect (b) potentially.

However, if a discussion happens, and an impasse is arrived at, then the conversation can be escalated to the TSC again, and this time maybe a deadline for a decision can be more carefully observed by those of us on the TSC.

That said, I do suspect that the TSC will be very reluctant to overrule the opinions of the people actually doing the releases. (At least, that is my personal position as a TSC member. The case for doing so would have to be overwhelmingly compelling. Not impossible, but the bar is very high.) So gauging chances of success ahead of time could involve figuring out who is actually doing the work to create these releases, and then seeing where they personally stand on the matter.

@Trott
Copy link
Member

Trott commented Apr 26, 2020

However, if a discussion happens, and an impasse is arrived at, then the conversation can be escalated to the TSC again, and this time maybe a deadline for a decision can be more carefully observed by those of us on the TSC.

That said, I do suspect that the TSC will be very reluctant to overrule the opinions of the people actually doing the releases. (At least, that is my personal position as a TSC member. The case for doing so would have to be overwhelmingly compelling. Not impossible, but the bar is very high.) So gauging chances of success ahead of time could involve figuring out who is actually doing the work to create these releases, and then seeing where they personally stand on the matter.

Big oops on my part! The TSC can offer its opinion, but the Docker Working Group is officially chartered and the TSC cannot overrule it on the areas of its chartered responsibility which include "Decide and implement image improvements and/or fixes.", which I would interpret to include "to include yarn or not to include yarn" decisions. Docker Working Group would have to escalate to TSC themselves or something. The only way the TSC can stop a decision it doesn't like is to de-charter the Working Group entirely, which I think we are exceedingly unlikely to do over an issue like this.

@Trott
Copy link
Member

Trott commented Apr 26, 2020

By the way, if the working group isn't really active anymore and it's now just a GitHub team that does the releases but doesn't have any formal governance process that it actually follows or anything like that, de-chartering might make sense. So if that's the case, that might be the first step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment