Skip to content

Switch to pnpm for frontend development#73

Merged
rmunn merged 2 commits into
developfrom
chore/npm-to-pnpm
May 24, 2023
Merged

Switch to pnpm for frontend development#73
rmunn merged 2 commits into
developfrom
chore/npm-to-pnpm

Conversation

@rmunn

@rmunn rmunn commented May 24, 2023

Copy link
Copy Markdown
Contributor

pnpm is faster and more space-efficient than npm

Fixes #70.

rmunn added 2 commits May 24, 2023 15:18
pnpm is faster and more space-efficient than npm
This is necessary to continue building after switching to PNPM, because
some combination of outdated packages wasn't handling pnpm's node_modules
layout after the switch.

Everything builds and runs fine after this package upgrade.
@rmunn

rmunn commented May 24, 2023

Copy link
Copy Markdown
Contributor Author

Some combination of outdated packages, probably some Vite plugin for Svelte given the error message I was seeing, was failing to run after the PNPM switch. I assume it has to do with the new node_modules layout that pnpm uses (short version: npm flattens node_modules while pnpm keeps it organized in a hierarchy). Updating all packages with pnpm up fixed that, and it's probably a good idea to stay up-to-date on NPM packages anyway. The only drawback is that now this PR contains more than one change, but that's unavoidable in this case.

@rmunn
rmunn requested a review from myieye May 24, 2023 09:13
@rmunn

rmunn commented May 24, 2023

Copy link
Copy Markdown
Contributor Author

@myieye In #70 (comment), @hahn-kev mentioned that he's already using PNPM, so I'm sure he'd be in favor of this switch. So I'd like you to review this PR, if you would. That way I'll know that everyone's on board and it's ready to merge.

@hahn-kev hahn-kev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, let's wait for Tim to respond before we go ahead though.

Comment thread README.md
dotnet watch
```

If you don't have pnpm installed, run `npm install -g pnpm`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually it would be better to suggest using corepack to install it, like this corepack prepare pnpm@latest --activate as suggested by their documentation. Core pack is a feature in more recent node versions that help you manage other package managers.

Comment thread frontend/package.json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We could specify the package manager in this file like this: "packageManager": "pnpm@8.5.1"

I'm not sure, but I believe that will tell node which package manager we want to use, it should mean that we don't even need to instruct users to install pnpm manually and it should get installed automatically, but I need to check up on how that works exactly.

@myieye

myieye commented May 24, 2023

Copy link
Copy Markdown
Collaborator

@rmunn fine with me 👍

@rmunn

rmunn commented May 24, 2023

Copy link
Copy Markdown
Contributor Author

I'll look into how to specify pnpm via corepack, and maybe do it as a separate PR. Merging this one so that we can get moving.

@rmunn
rmunn merged commit dac1596 into develop May 24, 2023
@rmunn

rmunn commented May 24, 2023

Copy link
Copy Markdown
Contributor Author

Opened #75 to track remaining work.

@hahn-kev
hahn-kev deleted the chore/npm-to-pnpm branch July 9, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch to PNPM for the frontend

3 participants