Skip to content
New issue

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

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

Already on GitHub? # to your account

use a specific pnpm version #2555

Merged
merged 1 commit into from
Aug 20, 2024
Merged

use a specific pnpm version #2555

merged 1 commit into from
Aug 20, 2024

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Aug 19, 2024

Currently, we don't have any way of understanding which version of pnpm we are using.

This pull-request:

  • Adds packageManager field that is used both by corepack and pnpm which automatically downloads the correct pnpm version.
  • Updates pnpm to latest version
  • Updates 2 bazel plugins in order to update the pnpm version. Unfortunately, the existing versions doesn't support pnpm 9.

@anonrig anonrig requested review from a team as code owners August 19, 2024 20:35
@anonrig anonrig requested review from fhanau and jp4a50 August 19, 2024 20:35
Copy link
Member

@npaun npaun left a comment

Choose a reason for hiding this comment

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

Just this morning I thought to myself "huh this project isn't using corepack for some reason". And now it's solved.

@anonrig anonrig force-pushed the yagiz/use-specific-pnpm-version branch from 10d17fa to 5f39d65 Compare August 19, 2024 20:41
@anonrig
Copy link
Member Author

anonrig commented Aug 19, 2024

Just this morning I thought to myself "huh this project isn't using corepack for some reason". And now it's solved.

My goal wasn't to add corepack since it is experimental and might be removed. Thankfully, pnpm has recently added a support for auto-installing the correct version depending on the packageManager value.

@npaun
Copy link
Member

npaun commented Aug 19, 2024

So the preferred workflow is to install any old version of pnpm, then get pnpm to install the right one for us?

@anonrig anonrig force-pushed the yagiz/use-specific-pnpm-version branch from 5f39d65 to d68f6b1 Compare August 19, 2024 20:54
@anonrig
Copy link
Member Author

anonrig commented Aug 19, 2024

So the preferred workflow is to install any old version of pnpm, then get pnpm to install the right one for us?

Preferably, pnpm install pnpm@latest which will handle the correct version for us, so there won't be any unnecessary diff in pnpm-lock.yml

@anonrig anonrig force-pushed the yagiz/use-specific-pnpm-version branch 3 times, most recently from 7f85c93 to e31dbb3 Compare August 19, 2024 22:01
@anonrig anonrig requested a review from a team as a code owner August 19, 2024 22:01
@anonrig anonrig force-pushed the yagiz/use-specific-pnpm-version branch from e31dbb3 to 08992d8 Compare August 19, 2024 22:14
@anonrig anonrig requested review from mikea and jasnell August 19, 2024 22:16
@anonrig anonrig force-pushed the yagiz/use-specific-pnpm-version branch from 08992d8 to f5fac52 Compare August 20, 2024 03:24
@anonrig
Copy link
Member Author

anonrig commented Aug 20, 2024

the syntax for .npmrc was wrong. i force pushed. @fhanau can you update the internal pull-request if you don't mind?

@fhanau
Copy link
Collaborator

fhanau commented Aug 20, 2024

Internal CI has passed. Note that the internal build will not be accessing the workerd .npmrc file, it's just the TS and tsconfig changes that are relevant to the internal build, so the last CI run is sufficient. This is ready to merge.

@anonrig anonrig merged commit 770c95b into main Aug 20, 2024
9 of 10 checks passed
@anonrig anonrig deleted the yagiz/use-specific-pnpm-version branch August 20, 2024 14:04
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants