yarn → pnpm
#9476
Merged
yarn → pnpm
#9476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sndrs
force-pushed
the
sndrs/pnpm
branch
2 times, most recently
from
dc6ad84 to
eea6ff0
Compare
|
Size Change: -259 kB (-26%) 🎉 Total Size: 742 kB
ℹ️ View Unchanged
|
sndrs
force-pushed
the
sndrs/pnpm
branch
4 times, most recently
from
c475c61 to
e513b26
Compare
mxdvl
reviewed
Dec 14, 2023
mxdvl
reviewed
Dec 14, 2023
sndrs
force-pushed
the
sndrs/pnpm
branch
5 times, most recently
from
56a211b to
ac7cb12
Compare
This was referenced Dec 15, 2023
sndrs
force-pushed
the
sndrs/pnpm
branch
2 times, most recently
from
ad87a6a to
cd503d1
Compare
sndrs
force-pushed
the
sndrs/pnpm
branch
2 times, most recently
from
12144f5 to
0026f61
Compare
sndrs
force-pushed
the
sndrs/pnpm
branch
2 times, most recently
from
741d462 to
889b57a
Compare
mxdvl
reviewed
Jan 2, 2024
mxdvl
reviewed
Jan 2, 2024
mxdvl
reviewed
Jan 2, 2024
mxdvl
approved these changes
Jan 2, 2024
arelra
approved these changes
Jan 2, 2024
There was a problem hiding this comment.
🚀 Very nice performance bump and the culmination of lots of prior work.
Just as a precaution could we deploy this to CODE pls to make sure the prod build and start is not affected. I will raise a ticket to add a GHA check to run the prod task so this is automated in future.
DCR deployed to CODE here https://riffraff.gutools.co.uk/deployment/view/0d3751e5-0a93-4d50-8961-5fb5c7cd0ec3 🟢 AR here https://riffraff.gutools.co.uk/deployment/view/0b31d225-9883-49cb-82b3-35a821c1bf18 🟢 |
Merged
mxdvl
reviewed
Jan 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change?
Swaps
yarnforpnpmpnpmis much faster to install, much lighter on disk, and because of that it is able isolate dependencies so that no transitive deps are available.Updating the apps to get this working has surfaced real bugs and instability.
Using
pnpmshould hopefully prevent this class of bug recurring, while keeping a useable, predictablenode_modulesdirs around (unlike, for example, yarn's pnp mode, which solves the problem in a more non-standard way).Note
To get the build working, I had to create the
pnpm-lock.yamlfrom scratch – importing the currentyarn.lockresulted in various issues with webpack types (and i think others too) which starting clean resolved.This means that dependencies will have been bumped to the latest available versions in the specified ranges. This is probably a good thing, but it does mean that build output is not going to be identical to
main. I think that's ok?node_modulesmake install(no changes)./.github/actions/setup-node-env(with a cache hit)Removes the
preinstallcorepack checkOnly yarn runs the
preinstallhook before installing (see here and here), so there's not a lot of point in keeping this around.