Skip to content

chore: pnpm migration[KHCP-21042]#890

Open
sharmaadityaHQ wants to merge 6 commits into
mainfrom
chore/pnpm-migration
Open

chore: pnpm migration[KHCP-21042]#890
sharmaadityaHQ wants to merge 6 commits into
mainfrom
chore/pnpm-migration

Conversation

@sharmaadityaHQ

@sharmaadityaHQ sharmaadityaHQ commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR aims to address pnpm migration from v10 to v11 by following the migration guide here - https://pnpm.io/migration.

pnpm v11 compatibility fixes in CI

  1. Removed --amend from pnpm version. the flag was dropped in pnpm v11 and caused the step to error out.
  2. Fixed pnpm publish output parsing as pnpm v11 no longer prints + @Package@version to stdout, so the old grep "+ " always returned empty and falsely triggered Error publishing package. Now we check the exit code for failure detection instead.
  3. Simplified npm_instructions construction. Instead of parsing the publish output to extract the package name, reads it directly from package.json using jq.
  4. vitest, form-data audit fixes to unblock audit in CI.

Jira - https://konghq.atlassian.net/browse/KHCP-21042

Consuming app PR - https://github.com/kong-konnect/portal/pull/2582

Comment thread .github/workflows/CI.yaml
Comment on lines -91 to +93
pkg=$(pnpm publish --no-git-checks --access public --report-summary --tag "${tag}" | grep "+ "| sed 's/+ //')
pnpm publish --no-git-checks --access public --tag "${tag}"

if [[ -z "${pkg}" ]]; then
if [ $? -ne 0 ]; then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this refactor necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, pnpm v11 changed the publish output format. the CI would fail with Error publishing package without this change.

@adamdehaven adamdehaven Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you leave some more context on the pull request; otherwise, we're just assuming what the changes are.

Please leave some context in the areas that were changed

Comment thread .github/workflows/CI.yaml
Comment on lines -98 to +99
npm_instructions="@$(echo ${pkg}|cut -d'@' -f2)@${tag}"
package_name=$(jq -r ".name" package.json)
npm_instructions="${package_name}@${tag}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this refactor necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, pkg is removed in the above pnpm publish step and its simpler to read package name from package.json just like we read version on line 80.

Comment thread pnpm-workspace.yaml Outdated
yaml@>=2.0.0 <2.8.3: '>=2.8.4'
vite@>=7.0.0 <=7.3.1: '>=7.3.3'
strictPeerDependencies: false
autoInstallPeers: false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

issue: why are we not auto-installing peerDependencies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is migrated as is from .npmrc via pnpm codemod - https://github.com/Kong/spec-renderer/pull/890/changes#diff-e813096d69c49812e40e00be9ac8fa14d77f0ec1f97ab4c45cb096b3bedd35deL2. i see that in our case vue is listed in peer dependencies as well as dev dependencies. should i be changing it in this PR or is it fine as is?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see why we would not want to auto-install peers in most cases

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the repository’s package-manager setup from pnpm v10 to v11, moving pnpm configuration into pnpm-workspace.yaml, updating the pinned pnpm version, and adjusting CI publishing steps to align with the newer pnpm behavior.

Changes:

  • Moved pnpm overrides/build-allowlist and peer settings out of package.json / .npmrc into pnpm-workspace.yaml.
  • Updated Volta’s pinned pnpm version to 11.5.3.
  • Simplified CI preview publishing logic for pnpm v11 (version bump + publish flow).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
pnpm-workspace.yaml Centralizes pnpm v11 workspace config (overrides, peer settings, build approvals).
package.json Removes embedded pnpm config and bumps Volta pnpm version to v11.
.npmrc Drops settings now represented in the workspace config.
.github/workflows/CI.yaml Updates preview publish steps to match pnpm v11 behavior/output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/CI.yaml Outdated
Comment on lines +77 to +78
# Use yarn to bump the version for the prerelease
pnpm version prerelease --preid ${preid} --no-git-tag-version --yes --amend
pnpm version prerelease --preid ${preid} --no-git-tag-version --yes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's fix comments

Comment thread .github/workflows/CI.yaml
Comment on lines +91 to 95
pnpm publish --no-git-checks --access public --tag "${tag}"

if [[ -z "${pkg}" ]]; then
if [ $? -ne 0 ]; then
echo "Error publishing package"
exit -1
@sharmaadityaHQ sharmaadityaHQ force-pushed the chore/pnpm-migration branch from 2f56f08 to b181bda Compare July 6, 2026 08:57
@kongponents-bot

Copy link
Copy Markdown
Collaborator

Install the preview package from this PR

@kong/spec-renderer@pr-890

Comment thread .github/workflows/CI.yaml
# Use yarn to bump the version for the prerelease
pnpm version prerelease --preid ${preid} --no-git-tag-version --yes --amend
# Use pnpm to bump the version for the prerelease
pnpm version prerelease --preid ${preid} --no-git-tag-version --yes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is --amend removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

amend flag was droppend in pnpm v11. was leading to ERROR: Unknown option: 'amend'. no-git-tag-version already helps ensure no git commit or tag is created here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

seems like amend wasn't there in previous versions either. v11 has a native publish flow as mentioned in the highlights section point 6 here - https://pnpm.io/blog/releases/11.0. earlier it used to delegate to the npm CLI where passing extra flags was not erroring.

@sharmaadityaHQ sharmaadityaHQ Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

reference to the above comment - https://pnpm.io/pnpm-cli#differences-vs-npm. "Unlike npm, pnpm validates all options"

@sharmaadityaHQ sharmaadityaHQ requested a review from ValeryG July 6, 2026 17:58
@kongponents-bot

Copy link
Copy Markdown
Collaborator

🔴 PR audit failed. 🔴

🔥 Renovate Security PRs detected.

There are 1 open renovate security PRs older than 3 days.

This PR cannot be merged until all renovate security PRs created more than 3 days ago are resolved.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants