Skip to content

use a specific pnpm version#2555

Merged
anonrig merged 1 commit intomainfrom
yagiz/use-specific-pnpm-version
Aug 20, 2024
Merged

use a specific pnpm version#2555
anonrig merged 1 commit intomainfrom
yagiz/use-specific-pnpm-version

Conversation

@anonrig
Copy link
Copy Markdown
Contributor

@anonrig anonrig commented Aug 19, 2024 •
edited
Loading

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
npaun
npaun approved these changes Aug 19, 2024
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
Contributor 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 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 jasnell and mikea 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
Copy Markdown
Contributor 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
Copy Markdown
Contributor

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
@anonrig anonrig deleted the yagiz/use-specific-pnpm-version branch August 20, 2024 14:04
@harrishancock harrishancock mentioned this pull request Aug 20, 2024
Merged
@npaun npaun mentioned this pull request Aug 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@npaun npaun npaun approved these changes

@fhanau fhanau fhanau approved these changes

@jp4a50 jp4a50 Awaiting requested review from jp4a50 jp4a50 is a code owner automatically assigned from cloudflare/workers-runtime-1

@mikea mikea Awaiting requested review from mikea

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@anonrig @npaun @fhanau @jasnell