DEV: add script to switch from yarn to pnpm #11
Conversation
tyb-talks
force-pushed
the
dev-add-script-to-switch-from-yarn-to-pnpm
branch
from
dc2341c
to
b81b050
Compare
tyb-talks
force-pushed
the
dev-add-script-to-switch-from-yarn-to-pnpm
branch
from
b81b050
to
a9c88e7
Compare
| fi | ||
|
|
||
| echo "Replacing 'yarn' with 'pnpm' in package.json scripts..." | ||
| jq 'if has("scripts") then .scripts |= with_entries(.value |= gsub("yarn "; "pnpm ")) else . end' package.json > temp.json && mv temp.json package.json |
There was a problem hiding this comment.
this is a little clunky, but my experiment with using process substitution cp package.json <(jq 'some filter' package.json) errored out, so I think the temp file way is a more consistent across machines.
| }' package.json > temp.json && mv temp.json package.json | ||
|
|
||
| echo "Installing dependencies with pnpm..." | ||
| pnpm install |
There was a problem hiding this comment.
Do we need a cleanup script for existing setups, like we have in core?
https://github.com/discourse/discourse/blob/main/.pnpmfile.cjs#L6C1-L19C1
That is: will pnpm i work in dev envs that have yarn-created node_modules dirs?
There was a problem hiding this comment.
I tried without the cleanup script, pnpm tries to move the old modules into a .ignored folder, but I think for a cleaner slate, it's better to add this anyway.
There was a problem hiding this comment.
The reason I added it to core was because pnpm's auto cleanup doesn't work in workspaces. Themes/plugins are much simpler, so I think it's probably fine to just let pnpm do its thing, and avoid duplicating the .pnpmfile hack in every single repo.
What do you think @CvX @tyb-talks ?
There was a problem hiding this comment.
pnpm's auto cleanup doesn't work in workspaces
Ah I see.. in that case, I'm ok to let pnpm do its thing.. worst case, devs can rm their local node_modules and reinstall with pnpm.
|
|
||
| if ! file_exists .npmrc; then | ||
| echo "Creating .npmrc..." | ||
| echo -e "engine-strict = true\nauto-install-peers = false" > .npmrc |
There was a problem hiding this comment.
this is the same npmrc settings we have for core - would be needed for ensuring running pnpm install locally on a theme/plugin repo behaves the same way.
| command -v "$1" >/dev/null 2>&1 | ||
| } | ||
|
|
||
| if ! command_exists jq; then |
There was a problem hiding this comment.
Feel free to ignore but... I wonder if we should just bite the bullet and write this script in Ruby? I made the mistake of writing the original ones in bash cos I thought they would be simple... but complexity has ballooned over time 😢
There was a problem hiding this comment.
Maybe for a future one? I'm still finding the current use cases ok for bash. (of course, without adding tools like jq in bash, it becomes quite hairy to handle stuff like json 😅 )
There was a problem hiding this comment.
in Ruby
some time ago I started redoing existing scripts in node js 😅 we could revive that effort, but it's low priority. though we could at least land the support for js scripts (w/o actually finishing the rewrite of all of the existing stuff)
There was a problem hiding this comment.
land the support for js scripts
It should 'just work', right? As long as you change the shebang at the top of the file to something like
#!/usr/bin/env node
or for ruby
#!/usr/bin/env ruby
?
There was a problem hiding this comment.
Oh, yeah, I suppose that would work too. I was thinking about "native" support, without shelling out, but I guess that would make more sense when all scripts are already in js.
This script will be used to convert all officially supported plugins & themes to use pnpm as a package manager.
See https://meta.discourse.org/t/324521 for more detail on this change.
How to run:
--
Example PRs:
discourse/discourse-gifs#74 - CI runs should be passing now that discourse/.github#152 is merged.