-
Notifications
You must be signed in to change notification settings - Fork 63
Use pnpm (and fix Storybook issues as a result) #525
Conversation
Oops, I actually pushed a commit with some config suggestions to your PR. Please review, as they're minor anyway! Everything is working flawlessly for me, tested with:
rm -rf node_modules
volta install pnpm
pnpm i
npm run dev
docker compose up
pnpm run storybook
pnpm run test:e2e
pnpm run test |
@zackkrida just curious, why do we need to explicitly use |
@sarayourfriend We don't, I just think it's clearer for anyone checking, for example, documentation: I generally lean towards being verbose |
Gotcha. I was following the style the recommend in their |
5d20ff6
to
5abfa7e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with eliminating run
usage @sarayourfriend, since the pnpm docs actively encourage not writing out run
it might actually be clearer without it. So glad this was such a win, and thanks for finding everywhere npm needed to be replaced 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested the docker build but locally it is amazingly fast and Storybook mdx files all work for me. Thank you for improving devex, @sarayourfriend !
One small thing that isn't related but would reduce the number of warnings when running Storybook is to replace "license/VLicence" with "License/VLicense" (first letter is upper-case L). I added a tiny PR : #527
Just a note: Webstorm integration with pnpm is not working great. I used the GUI (green 'run' triangles) to run the scripts from |
@obulat Maybe these could help, it seems that it's supposed to work! https://stackoverflow.com/questions/63875255/how-to-set-up-pnpm-in-run-debug-in-webstorm |
Fixes
Fixes #483 by @krysal
Description
Replaces
npm
withpnpm
.pnpm
has some significant advantages compared tonpm
, including super fast installs thanks to its local dependency cache. Dependencies are sym-linked intonode_modules
which makes usingpnpm
for many projects ultimately result in smaller disk usage. It also makes dependency re-installation super fast as it just has to create the sym-links. Similarly, deletingnode_modules
is much faster as you're just removing sym-links.To support Vue we need to use the
shamefully-hoist
option as noted in this issue: pnpm/pnpm#831 (note that theshamefully-flatten
option has been renamed toshamefully-hoist
)Tagging @rbadillap specifically for review on this one as it touches the newly minted Dockerfile 🙂
Marking this as high priority as not being able to properly use Storybook is not good for redesign development which we're currently moving quickly on.
Testing Instructions
Checkout this branch and remove
node_modules
(for examplerm -rf node_modules
). Follow the instructions in the readme to installpnpm
(volta install pnpm
) then runpnpm install
. Next runpnpm storybook
andpnpm dev
to ensure that they work 🎉 Also runpnpm build
followed bypnpm start
.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin