Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from yarn to pnpm #27299

Closed
wants to merge 5 commits into from
Closed

Switch from yarn to pnpm #27299

wants to merge 5 commits into from

Conversation

renchap
Copy link
Sponsor Member

@renchap renchap commented Oct 5, 2023

This is al alternative to #27073

Yarn 1 is no longer maintained and suffers from many bugs (including eslint/eslint#17215 (reply in thread) which prevents multiple package upgrades)

pnpm is a more modern package manager, which should be much faster and generate a more correct node_modules/

The two main changes are:

  • pnpm maintains a global package repository on your machine. Then it links those packages into node_modules/, which makes installation much more efficient and reduces the disk usage if you have many JS projects on your computer
  • only direct dependencies are linked into node_modules/. For example, tesseract.js-core was not defined as a dependency but was imported from a JS file, causing an error, so I added it as an explicit dependency.

As Webpacker relies on yarn, I monkey-patched the parts that called yarn directly to replace it with pnpm.

Our setup should now properly support corepack, so installing the package manager is as simple as corepack prepare --activate (and maybe corepack enable before, if it has never been ran on this machine and is not down when installing node). It will automatically install the correct pnpm version.

This PR is based on #27295 as we can no longer use capitrano-yarn, and Capistrano is not tested so I removed it.

@renchap renchap force-pushed the pnpm branch 3 times, most recently from dffc1f6 to c022d77 Compare October 5, 2023 21:10
@nschonni
Copy link
Contributor

nschonni commented Oct 5, 2023

Probably a good choice, but you'll have to look at the Dockerfile stuff, since the official images only ship with NPM and Yarn1. I know there are a few Docker related PRs active right now, that might move off the official NodeJS image

@renchap renchap marked this pull request as ready for review October 6, 2023 10:08
@renchap
Copy link
Sponsor Member Author

renchap commented Oct 6, 2023

Probably a good choice, but you'll have to look at the Dockerfile stuff, since the official images only ship with NPM and Yarn1. I know there are a few Docker related PRs active right now, that might move off the official NodeJS image

I switched the docker images to using corepack, which will handle installing the correct JS package manager automatically, using the packageManager field from package.json.

Gargron
Gargron previously approved these changes Oct 6, 2023
@vmstan
Copy link
Contributor

vmstan commented Oct 7, 2023

Probably a good choice, but you'll have to look at the Dockerfile stuff, since the official images only ship with NPM and Yarn1. I know there are a few Docker related PRs active right now, that might move off the official NodeJS image

I've been working on one of them, and have it setup to test this and worked with Renchap to get a successful build on it today.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

@ClearlyClaire
Copy link
Contributor

ClearlyClaire commented Oct 12, 2023

Switching from our current yarn version to anything else will require significant changes for admins (unless using docker), and the documentation will need to be updated.

I ran into multiple issues setting up corepack on my setups, but to be fair, I had other issues with yarn 1.

For some reason, on my ArchLinux-based development environment, the husky hook now somehow picks up the wrong bundle environment. I don't understand what is causing this nor how to fix it.
Manually running bundle exec rubocop --force-exclusion -a works without issues.
EDIT: This issue isn't specific to pnpm

Also, should we change something to the Renovate config?~~

@renchap
Copy link
Sponsor Member Author

renchap commented Oct 12, 2023

For some reason, on my ArchLinux-based development environment, the husky hook now somehow picks up the wrong bundle environment. I don't understand what is causing this nor how to fix it:

What node version manager are you using? This is very weird, I am not sure what is happening here 🤔

@ClearlyClaire
Copy link
Contributor

What node version manager are you using? This is very weird, I am not sure what is happening here 🤔

v18.18.0

@renchap
Copy link
Sponsor Member Author

renchap commented Oct 14, 2023

Also, should we change something to the Renovate config?~~

Nothing specific, we use matchManagers: ['npm'], which will work with every JS package manager.

The currently open PRs will probably need to be rebased, but Renovate should do it.

the documentation will need to be updated.

I did it for documentation in this repo, but as the main documentation is not versioned, I will probably need to add specific instructions for Mastodon >= 4.3 everywhere yarn is mentioned :(

Do you want me to open the PR for this now, or should we do it closer to the 4.3 release?

Yarn v1 is no longer maintained and is causing multiple issues.

pnpm is a more modern package manager, which should be much faster and generate a more correct `node_modules/`

I monkey-patched the parts of Webpacker that called `yarn` directly to replace it with `pnpm`.
Those are unused as the definitions are included in the base packages
@renchap
Copy link
Sponsor Member Author

renchap commented Oct 16, 2023

Rebased with latest main, if you want to merge it :)

@github-actions
Copy link
Contributor

This pull request has resolved merge conflicts and is ready for review.

@vmstan
Copy link
Contributor

vmstan commented Oct 16, 2023

Rebased with latest main, if you want to merge it :)

Compiled and running it in prod, no errors.

@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@renchap
Copy link
Sponsor Member Author

renchap commented Nov 8, 2023

Closing as #27073 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants