Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

Replaces "Yarn PnP" by "Yarn 2" #41

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Replaces "Yarn PnP" by "Yarn 2" #41

wants to merge 4 commits into from

Conversation

arcanis
Copy link
Collaborator

@arcanis arcanis commented Feb 11, 2020

Fixes #31

I've also enabled the Yarn 2 benchmark for the parts that were previously n/a by removing the .pnp.js where the node_modules folder would have been removed otherwise.

@arcanis arcanis requested a review from zkochan February 11, 2020 18:23
@arcanis
Copy link
Collaborator Author

arcanis commented Feb 11, 2020

Now that I think about it, now that the cache and the node_modules are effectively the same, the results aren't as easy to interpret 🤔 Hmm maybe the "n/a" approach was more sensible ...

@@ -20,15 +20,15 @@ The app's `package.json` [here](./fixtures/react-app/package.json)

| action | cache | lockfile | node_modules| npm | pnpm | Yarn | Yarn PnP |
Copy link
Member

@zkochan zkochan Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might make sense to rename the columns to Yarn 1 and Yarn 2. But to be honest, why not just run the performance tests for Yarn 2 and name it "Yarn"? Why do we need to include Yarn 1 in the benchmarks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's still valuable given that many people will only migrate over time. Plus, since 1.x and 2.x are quite different implementations with different tradeoffs, it gives a nice way to see the impact of the choices we made, one way or another.

@arcanis
Copy link
Collaborator Author

arcanis commented Feb 11, 2020

I'll restore the n/a, use a different color for Yarn 2.x, add the "1.x" label to the column, and maybe sort the table so that all the "caches" / "no caches" are put together - it really highlights where is 2.x faster or slower.

Btw @zkochan, what's your secret sauce for "cache / no lockfile / n_m"? I'd expect it to take at least the time needed to fetch the metadata from the registry 🤔

@zkochan
Copy link
Member

zkochan commented Feb 11, 2020

I think that case is so fast because even though there is no lockfile near the package.json, there is still a lockfile duplicate inside node_modules/.pnpm/lock.yaml.

pnpm has two lockfiles. They have the same format. One is in the same folder in which package.json and the other one inside node_modules. This is great because after a branch switch, for instance, we just make a diff of the two lockfiles and we know what to remove or add.

@zkochan
Copy link
Member

zkochan commented May 24, 2020

It would be nice to update the benchmarks to use Yarn 2. I'd love to see how pnpm v5 performs compared to it.

@SgtPooki
Copy link

Anyone working on getting this merged?

@zkochan
Copy link
Member

zkochan commented Jan 29, 2021

I don't think so because Yarn now maintains its own benchmarks: https://yarnpkg.com/benchmarks

I think I would prefer to keep only benchmarks that compare non-PnP package managers in this repo: npm7, pnpm, and Yarn2 with node-linker=node-modules

and we can add a link to Yarn benchmarks to see how PnP performs compared to pnpm.

Base automatically changed from master to main February 5, 2021 01:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Yarn PnP w/ Yarn 2
3 participants