chore: migrate package manager from bun to pnpm#191
Merged
Conversation
- pnpm workspace + lockfile; packageManager pnpm@11.1.1 (catalogs moved to pnpm-workspace.yaml, native build scripts allowlisted) - replace bumpp + tag-triggered bun publish with changesets (publishConfig.directory: dist keeps the existing dist build) - convert CI workflows and Vercel configs to pnpm - keep bun as a runtime for apps/demo scripts and the CLI package-manager detection
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR migrates the repository from Bun to pnpm, introduces Changesets for versioning and releases, adds a pnpm workspace, updates CI/CD workflows and app manifests to pnpm, and adds a publish helper script to publish built package dists. ChangesPackage Manager & Release Pipeline Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
31-31: ⚖️ Poor tradeoffConsider pinning GitHub Actions to commit SHAs.
The new
pnpm/action-setup@v4references use version tags. For enhanced security and reproducibility, consider pinning to specific commit SHAs (e.g.,pnpm/action-setup@fe1e16c3126dd1225ad154e1a3f6a2c1ab0da622). This prevents potential supply-chain attacks if a version tag is moved.Also applies to: 60-60, 87-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml at line 31, Replace the loose tag usage of the GitHub Action pnpm/action-setup@v4 with a pinned commit SHA to improve security and reproducibility; update the occurrence shown (pnpm/action-setup@v4) and the other two occurrences of pnpm/action-setup in the workflow to use a specific commit ref (e.g., pnpm/action-setup@<commit-sha>) so each action reference is pinned to an immutable SHA instead of a mutable tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Line 22: Replace tag-pinned GitHub Actions with their corresponding full
commit SHAs: locate every uses: entry such as actions/checkout@v4,
pnpm/action-setup@v4, actions/setup-node@v4 in .github/workflows/release.yml
(and similarly in .github/workflows/ci.yml, e2e.yml, npm-dist-tag.yml) and
update each to the exact commit SHA for that action (e.g.,
actions/checkout@<full-sha>) so the workflow references a fixed commit instead
of a floating tag.
In `@packages/polar/package.json`:
- Around line 13-15: The package.json for `@paykitjs/polar` is configured to
publish from "dist" but main/types/exports point to ./src/* causing pnpm pack to
fail; fix by either (A) removing the "publishConfig.directory" entry so
publishing uses the package root package.json (and keep main/types/exports as
./src/*), or (B) emit/copy a complete package.json into dist/ with updated
"main", "types", and "exports" that reference the built files inside dist (e.g.,
dist/index.js, dist/index.d.ts), or (C) update the package root's
"main"/"types"/"exports" to point to the built outputs under dist and ensure the
build produces those files; modify the package.json fields ("publishConfig",
"main", "types", "exports") accordingly and ensure the chosen option results in
a package.json present at the publish root that correctly references the
published file layout.
In `@packages/stripe/package.json`:
- Around line 13-15: The package.json currently sets publishConfig.directory to
"dist", which causes pnpm pack to fail because dist/package.json is missing;
either remove the publishConfig.directory entry from
packages/stripe/package.json and ensure the built output aligns with existing
main/types/exports pointing at the repo root (so packing uses the top-level
manifest), or keep publishConfig.directory but change the build to emit a
dist/package.json and update main/types/exports in packages/stripe/package.json
to reference compiled files inside dist (not ./src/index.ts); pick one approach
and apply consistently so pnpm pack can find a package.json in the publish
directory.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 31: Replace the loose tag usage of the GitHub Action pnpm/action-setup@v4
with a pinned commit SHA to improve security and reproducibility; update the
occurrence shown (pnpm/action-setup@v4) and the other two occurrences of
pnpm/action-setup in the workflow to use a specific commit ref (e.g.,
pnpm/action-setup@<commit-sha>) so each action reference is pinned to an
immutable SHA instead of a mutable tag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 19c725c1-b90c-40ff-bd14-cfe4d955551a
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.changeset/README.md.changeset/config.json.github/workflows/ci.yml.github/workflows/e2e.yml.github/workflows/release.yml.superset/config.jsonCONTRIBUTING.mdapps/demo/package.jsonapps/demo/vercel.jsonapps/web/lighthouserc.jsonapps/web/package.jsonapps/web/vercel.jsonapps/wh/drizzle.config.tsapps/wh/package.jsonbump.config.tspackage.jsonpackages/dash/src/plugin/html.tspackages/paykit/package.jsonpackages/polar/package.jsonpackages/stripe/package.jsonpnpm-workspace.yamlscripts/worktree-setup.sh
💤 Files with no reviewable changes (2)
- bump.config.ts
- apps/demo/package.json
- drop publishConfig.directory (it made pnpm link workspace deps to dist, breaking paykit-source -> src resolution when dist isn't built) - publish built dist via scripts/publish-dist.mjs in ci:release instead - disable no-underscore-dangle / consistent-function-scoping (intentional Node-internal + sentinel names) so oxlint --deny-warnings passes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/publish-dist.mjs`:
- Around line 15-20: The try/catch around execSync(`npm view ${spec} version`)
treats all errors as "not published"; change the catch to capture the thrown
error (catch (err)) and inspect err.stderr / err.message / err.status to detect
the genuine "version not found" case (e.g., 404/E404 or npm's "No matching
version" text) and only set alreadyPublished = false in that case; for any other
error (network/auth/registry) rethrow or exit non-zero so transient failures
fail fast. Ensure you reference the same execSync call, the spec variable, and
the alreadyPublished flag when making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 67d3b033-6dba-45b8-9bfb-e31fe0aa5ec2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.oxlintrc.jsonpackage.jsonscripts/publish-dist.mjs
✅ Files skipped from review due to trivial changes (1)
- .oxlintrc.json
Comment on lines
+15
to
+20
| try { | ||
| execSync(`npm view ${spec} version`, { stdio: "ignore" }); | ||
| alreadyPublished = true; | ||
| } catch { | ||
| // `npm view` exits non-zero when the version does not exist yet. | ||
| } |
There was a problem hiding this comment.
Handle only “version not found” as a skip condition.
The catch-all here treats any npm view failure as “not published yet”. Transient registry/auth/network errors should fail fast instead of attempting publish.
Suggested fix
let alreadyPublished = false;
try {
execSync(`npm view ${spec} version`, { stdio: "ignore" });
alreadyPublished = true;
- } catch {
- // `npm view` exits non-zero when the version does not exist yet.
+ } catch (error) {
+ const message = String(error?.stderr || error?.message || "");
+ // npm returns E404 when a specific version does not exist yet.
+ if (message.includes("E404")) {
+ alreadyPublished = false;
+ } else {
+ throw error;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| execSync(`npm view ${spec} version`, { stdio: "ignore" }); | |
| alreadyPublished = true; | |
| } catch { | |
| // `npm view` exits non-zero when the version does not exist yet. | |
| } | |
| try { | |
| execSync(`npm view ${spec} version`, { stdio: "ignore" }); | |
| alreadyPublished = true; | |
| } catch (error) { | |
| const message = String(error?.stderr || error?.message || ""); | |
| // npm returns E404 when a specific version does not exist yet. | |
| if (message.includes("E404")) { | |
| alreadyPublished = false; | |
| } else { | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/publish-dist.mjs` around lines 15 - 20, The try/catch around
execSync(`npm view ${spec} version`) treats all errors as "not published";
change the catch to capture the thrown error (catch (err)) and inspect
err.stderr / err.message / err.status to detect the genuine "version not found"
case (e.g., 404/E404 or npm's "No matching version" text) and only set
alreadyPublished = false in that case; for any other error
(network/auth/registry) rethrow or exit non-zero so transient failures fail
fast. Ensure you reference the same execSync call, the spec variable, and the
alreadyPublished flag when making this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the monorepo from bun to pnpm, and from bumpp + the tag-triggered
bun publishto changesets.What changed
pnpm-workspace.yaml(+ catalogs moved here frompackage.json, native build scripts allowlisted),packageManager: pnpm@11.1.1,pnpm-lock.yaml; removedbun.lock..changeset/,ci:version/ci:releasescripts,publishConfig.directory: diston the 3 public packages (keeps the existingdistbuild). Removedbumpp+bump.config.ts;release.ymlnow runschangesets/actionon push tomain.vercel.jsonswitched to pnpm.apps/demodev scripts (bun as a TS runtime) and the CLI's package-manager detection / docs — these support paykit users on bun.Verified
pnpm install,pnpm turbo build typecheck --filter ./packages/*(8/8 pass —paykit-sourceresolves through pnpm symlinks),pnpm packshipsdist(notsrc).Release behavior
Merging this to
maintriggers the Release workflow. With no pending changesets and0.0.6not yet on npm, it runspnpm ci:release→ builds +changeset publish→ publishespaykitjs/@paykitjs/polar/@paykitjs/stripe@0.0.6aslatest. Requires a validNPM_TOKENsecret.Summary by CodeRabbit