graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.08k stars 1.72k forks source link

Move to yarn 3 + pnp, pnpm or latest npm? #2185

Open acao opened 2 years ago

acao commented 2 years ago

Whatever we decide on is great, just the install time is atrocious for some users, many have complained, and I don't think vscode-graphql is about to help with that 😬

orta commented 2 years ago

PNP is a bad idea if you're using something like dependabot, you're basically going to create a massive git history instead of a large download (all tarballs for every npm package would end up in git history to download)

if perf is a problem, pnpm has been a good replacement for yarn to me - don't think there's much wins though a complex monorepo nearly always ends up a massive set of deps (the TS website has the same thing)

rxliuli commented 2 years ago

pnpm is the best known, especially the --filter option currently has no replacement support, I use it in production (about 100+ modules)

acao commented 2 years ago

@orta agreed, ideally trimming dependencies is the best way to speed things up

@rxliuli that would be cool, would love to see a PR if someone has time! I imagine we would need to add pnp support libraries for jest, cypress, webpack, vite, etc?

rxliuli commented 2 years ago

I can try to create a PR with yarn2 => pnpm if you guys agree. In addition, pnpm does not need to use yarn pnp, it uses soft links to achieve more efficient disk sharing and performance.

ref: https://pnpm.io/zh/motivation

rxliuli commented 2 years ago

Some notes when I migrate pnpm inside my team @acao

Why use pnpm

Performance comparison Avoid phantom dependencies

Actual performance comparison

comparison pnpm yarn3
no cache 1m49.128s 7m15.828s
with cache 0m37.468s 1m20.278s
Installing dependencies 0m2.754s 0m13.080s
not updated 0m4.034s 0m12.188s
initialize 0m26.013s 0m35.381s

pnpm migration guide

rm package-lock.json
npm i -g pnpm
cd matrix/
pnpm i/rm
pnpm dev/build
# Run the specified command on all modules
pnpm --filter . run initialize

Limitations

timsuchanek commented 2 years ago

Happy to move to pnpm! Are we good with that decision? @rxliuli do you want to take this one? Happy to support you, as I set up pnpm before in a big repo.

rxliuli commented 2 years ago

Happy to move to pnpm! Are we good with that decision? @rxliuli do you want to take this one? Happy to support you, as I set up pnpm before in a big repo.

Yes, although I'm not too familiar with the codebase

acao commented 2 years ago

@rxliuli creating a PR will test almost everything and provide deploy previews!

noting that a few examples are included in workspaces but not all of them. It was too slow to include CRA and rollup in our lockfile, but we left vite in because we used to push deploy previews of each example to test bundler and ts loader constraints. The plan is to move from webpack to vite entirely if that matters

rxliuli commented 2 years ago

Yes, I am testing the performance changes before and after the migration, this is a basic performance comparison

comparison yarn pnpm
no cache 3m52.352s 1m25.885s
with cache 0m53.640s 0m19.277s
not updated 0m1.304s 0m2.094s
Installing dependencies 0m19.957s 0m5.100s

install dep: rxjs

timsuchanek commented 2 years ago

Wow this is exciting, thanks for taking the time @rxliuli

acao commented 2 years ago

Fabulous! Love to see it, thanks @rxliuli !

acao commented 2 years ago

@rxliuli I took the liberty of converting your PR back to a draft, when the github actuions are passing and it's ready for review, you can update the status and we can give it a review!