meilisearch / meilisearch-js

JavaScript client for the Meilisearch API
https://www.meilisearch.com
MIT License
703 stars 85 forks source link

Switch package manager from `yarn` to `pnpm` #1662

Closed flevi29 closed 1 month ago

flevi29 commented 1 month ago

Pull Request

Related issue

Fixes #1660

What does this PR do?

flevi29 commented 1 month ago

How do these work, how can I see what is hidden in this action, what does cache do here exactly?

https://github.com/meilisearch/meilisearch-js/blob/6cd64e9d3b7e70fd96b17809890d677bbb961c45/.github/workflows/tests.yml#L69-L73

I need to know this, it might need adjustments.

EDIT: I'm guessing it sets up yarn if it's not already cached, and maybe something else. This requires changes. All it has to do is corepack enable instead of installing yarn.

image

EDIT: I thought this might require changes in instant-meilisearch as well as that uses actions/setup-node@v4 too, but by default corepack will install the latest version of Yarn 1.x as soon as you type yarn in the command line and there isn't any packageManager specified in the package.json of that repository.

brunoocasali commented 1 month ago

Hi @flevi29, although I see the pnpm is faster than yarn, I don't think we should move away from the current package manager because we have yarn set up in the other js apps/libs across the org. Changing that to a different one will be a pain 😅.

flevi29 commented 1 month ago

Okay, fair enough. However my suggestion would make it so that nothing would have to be changed in all the other repositories.

Corepack lets you use Yarn, npm, and pnpm without having to install them. (https://github.com/nodejs/corepack)

We could adopt corepack, and manage package manager versions via the "packageManager" entry instead of globally.

flevi29 commented 1 month ago

Closing as not planned.