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

chore: Switch to pnpm. #2387

Merged
merged 1 commit into from
Apr 13, 2022
Merged

chore: Switch to pnpm. #2387

merged 1 commit into from
Apr 13, 2022

Conversation

cpojer
Copy link
Contributor

@cpojer cpojer commented Apr 11, 2022

Summary

pnpm is fast and extremely solid. I've been using it for all my projects recently, and I see adoption growing where previously Yarn/npm were used. I propose switching to it, but I realize it might be bikeshedding so feel free to close if it doesn't align with your plans. Happy to spend more time cleaning dependency things up in follow-up PRs.

Note: I did not switch vscode/editor because vsce does not seem to currently support pnpm yet. See microsoft/vscode-vsce#421.

Test Plan

  • Tried out the website → works.
  • Tried out the playground → works.
  • Tried out the website/api folder, works besides not having a database set up.
  • playground/workers-site seems like a mostly empty template, so probably fine.

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

There are few changes that are missing in this PR:

  • update the "scripts" field of the playground/package.json to use pnpm instead of npm
  • update the "scripts" field of the website/package.json to use pnpm instead of npm. Here we also use npm-run-all, I think we can use this https://pnpm.io/cli/run#--parallel as replacement
  • update the "engines" field of playground/package.json to use the latest version of pnpm. Ideally 6.*, as 7 is still in pre release

@ematipico
Copy link
Contributor

I believe the PR is not up to date with the latest version of the code, that's why there are failing tests.

@cpojer
Copy link
Contributor Author

cpojer commented Apr 12, 2022

Sorry for the delay here.

Updates:

  • Rebased.
  • Updated with all your recommendations. The engines one definitely makes sense, the other ones previously worked as well but you are right we should only use pnpm in the end so I changed it all.

@ematipico ematipico merged commit c18323d into rome:main Apr 13, 2022
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.

2 participants