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

Separate variant including yarn #777

Open
caub opened this issue Jun 4, 2018 · 21 comments · May be fixed by #1768
Open

Separate variant including yarn #777

caub opened this issue Jun 4, 2018 · 21 comments · May be fixed by #1768
Labels

Comments

@caub
Copy link

caub commented Jun 4, 2018

I think the nodejs image should'nt contain yarn, for projects and developers not using yarn, which is very likely the majority

related: #243

yarn adds 4436kB to the image

@xemasiv
Copy link

xemasiv commented Jun 6, 2018

I think that'll be hurting since:

  1. node-yarn has been deprecated @ https://hub.docker.com/r/yarnpkg/node-yarn/, hence a good fraction of the community already have the assumption that this image comes with yarn
  2. yarn is quite stable already @ v1.x.x and peforms quite on-par with npm these days
  3. there are quite some issues with npm that aren't present when using yarn - personally experienced this before, which made me switch to yarn
  4. yarn binaries are truly really small, like come on man including yarn in this image is one of the greatest gifts ever please don't take it back haha

@chorrell
Copy link
Contributor

chorrell commented Jun 6, 2018

FWIW, we're looking into creating new variant for use with multi-stage builds that does not include yarn or npm.

@SimenB
Copy link
Member

SimenB commented Jun 18, 2018

Yeah, I think it makes more sense to have a version without either of the package managers rather than excluding one of them. Multi-stage builds should be good enough when we get to it (or just do rm -rf /opt/yarn-v* if you wanna remove it now).

~40% of all npm registry downloads goes through yarn, so it's not like it's a niche use case. (https://twitter.com/cpojer/status/988955951301570560 as source, https://stats.yarnpkg.com/ seems down)

@caub
Copy link
Author

caub commented Oct 27, 2018

I made my own fork https://github.com/caub/docker-images#node, since I don't think we can agree about having npm and not yarn in the main node images

19% size gain on the slim variant

@ljharb
Copy link
Member

ljharb commented Oct 28, 2018

fwiw i agree; the official node docker image should map to what node officially ships, which by default is npm and not yarn. Whether it’s niche or not is irrelevant; if it’s too niche to ship with node, it’s too niche to be on the official node docker image.

@daveisfera
Copy link

daveisfera commented Oct 23, 2019

As @SimenB, yarn is used by a significant portion of the node ecosystem, and it only adds 5.5MB to the docker image (less than 10% of the alpine image size and even less for the debian bases), so I think that cost is well worth the convenience it provides.
I'm all for a variant that is "node only" (i.e. doesn't have npm or yarn), but until multi-stage builds are supported for official images, I think that will be tough to get the full benefits from (see #404 )

@ljharb
Copy link
Member

ljharb commented Oct 23, 2019

The size is not the issue nor the convenience. The default node docker image must not ship a package manager that neither the OS nor node ships - it’s wildly inappropriate for that decision to be made unilaterally for a single “official” node distribution (and not for all of them).

If anything, the variant should be the one with yarn, as that is the one that deviates from node’s official default.

@nschonni nschonni mentioned this issue Jan 18, 2020
@jjangga0214
Copy link

jjangga0214 commented Jan 19, 2020

Why don't we just make some tags then?
Like node:12-npm, node:lastest-yarn, node:13-alpine-pnpm, node:lts-base(no package managers).

CC: @nschonni

@LaurentGoderre
Copy link
Member

@tianon is it possible for the build to use multi-stage build? One way we could do this is have the current images which are the full ones and then create other images that just copy the part of those images a la

FROM alpine3.10

COPY --from=node:12-alpine3.10 /path/to/node

@caub
Copy link
Author

caub commented Jan 20, 2020

@LaurentGoderre
Copy link
Member

@caub I know it's possible in general but not necessarily with our setup.

@daveisfera
Copy link

Just keep in mind that multi-stage builds are still early in their support and have some caveats:
https://github.com/docker-library/faq#multi-stage-builds

@LaurentGoderre
Copy link
Member

@daveisfera my example falls into category number 2!

@nschonni
Copy link
Member

I think even if the multi-stage stuff works here, it would make sense for the Yarn org to have a repo for the images that builds on top of these images. EX: that would give them control over their own versioning and patching of the images, vs. the current policy of just updating the Yarn version here when the NodeJS release is cut

@LaurentGoderre
Copy link
Member

I'm not sure we are ready to take on another of these breaking changes

@nschonni
Copy link
Member

Yeah, if it happens, I think it would need to be like the OnBuild deprecation that lived until the 8 release line end

@SimenB
Copy link
Member

SimenB commented Jan 27, 2020

As far as I know, nothing has changed since we added it to the image almost 3 years ago, and I don't see why anything needs to change now. It's (still) by far our most requested feature, together with an alpine image. There's no real maintenance burden for us at this point, so I don't understand why we'd want to disrupt almost half of our users by changing this.

Node doesn't officially support running on alpine either, yet we have that image. As long as the TSC (or something like it) doesn't come and say "hey, we don't want this is as an official distribution" I'm strongly against removing yarn (or alpine) at this point.


I still think the correct solution here is distributing a version without either npm or yarn, and recommending multi stage builds.

@DRoet
Copy link

DRoet commented Apr 28, 2022

Would it be an idea to corepack enable by default instead of bundling w/ yarn? I feel like this would resolve the initial complaint about the image size, no? from my understanding Corepack will only download the yarn binary when you actively use it?

This would also provide easier usage for pnpm/yarn v2+ users

@SimenB
Copy link
Member

SimenB commented Apr 28, 2022

corepack enable might make sense! I see now that it gives 1.22.15 instead of 1.22.18, but that's probably not a huge issue. Would simplify our scripts as well. One could also say "to use yarn, run corepack enable first" and not do it automatically (that would however be a breaking change - but so could running enable automatically be as well, as we then also add pnpm. another breaking change is that YARN_VERSION env var would be wrong).

Lastly, corepack is experimental - would it still be OK to use it?

/cc @arcanis thoughts?

@arcanis
Copy link

arcanis commented Apr 28, 2022

A few points:

  • I think using Corepack would be a great idea! I'd also give us some practical insight as to how easy it is to use Corepack inside a Docker image, which would be valuable once we get to move it outside of experimental.

  • I'd still suggest to run corepack prepare during the image bundling, so that the default package managers are inside the image cache and can be used without network access. It doesn't improve the size aspect, but it's closer from what currently happens and thus a slightly less risky first step.

  • I feel like corepack enable should be called by default. The main reason it isn't by default in the regular Node distribution is to avoid potential breakages on systems where Yarn could already be installed another way, so in the case of the Docker image (where yarn is already provided) there's little reason to have it disabled.

  • "I see now that it gives 1.22.15 instead of 1.22.18" - I want to auto-release updates when package managers are updated, but haven't had time to look into that yet. There's also some push from the other Node team members to use dynamic versions rather than pin them in each Node release, so that'd solve that as well.

dunglas added a commit to dunglas/docker-node that referenced this issue Sep 7, 2022
This patch enables Corepack, allowing to use pnpm directly
and unbundling yarn from the default image.

Removing yarn also simplifies the image and the maintainance.

This change as been discussed in
nodejs#777.

Closes nodejs#777, nodejs#1645, nodejs#1755.
dunglas added a commit to dunglas/docker-node that referenced this issue Sep 7, 2022
This patch enables Corepack, allowing to use pnpm directly
and unbundling yarn from the default image.

Removing yarn also simplifies the image and the maintenance.

This change has been discussed in
nodejs#777.

Closes nodejs#777, nodejs#1645, nodejs#1755.
@dunglas dunglas linked a pull request Sep 7, 2022 that will close this issue
12 tasks
dunglas added a commit to dunglas/docker-node that referenced this issue Sep 7, 2022
This patch enables Corepack, allowing to use pnpm directly
and unbundling yarn from the default image.

Removing yarn also simplifies the image and the maintenance.

This change has been discussed in
nodejs#777.

Closes nodejs#777, nodejs#1645, nodejs#1755.
@dunglas
Copy link

dunglas commented Sep 7, 2022

I proposed a patch enabling Corepack and unbundling yarn in #1768.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

14 participants