feat: pnpm migration#520
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughRepository migration from npm to pnpm: workflows, docs, and package metadata updated to use pnpm@10.5.2; CI jobs switch to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 16 minutes and 26 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
Migrates the repo’s JavaScript tooling from npm to pnpm, aligning local developer docs and GitHub Actions workflows with the new package manager and lockfile.
Changes:
- Declare pnpm as the repo package manager and update script chaining to use pnpm.
- Update GitHub Actions workflows (CI, crawl, Sanity deploy) to install and execute via pnpm.
- Add ESLint dependency-checking rules and update developer docs for pnpm commands.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Sets packageManager, updates build script chaining, adds pnpm config and dependency changes. |
| eslint.config.mjs | Adds import/no-extraneous-dependencies configuration for dependency hygiene. |
| docs/plans/2026-04-28-pnpm-migration.md | Documents migration plan/status and checklist. |
| README.md | Updates local setup instructions to pnpm. |
| AGENTS.md | Updates contributor command references from npm to pnpm. |
| .github/workflows/deploy-sanity.yml | Switches dependency install and deploy commands to pnpm. |
| .github/workflows/crawl.yml | Switches crawl/sync jobs and cache key to pnpm + pnpm lockfile. |
| .github/workflows/ci.yml | Switches CI install/build/lint/typecheck/format/e2e steps to pnpm. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@sanity/vision": "5.19.0", | ||
| "@sentry/nextjs": "10.47.0", | ||
| "@tanstack/react-query": "5.96.1", | ||
| "@tanstack/react-query-devtools": "^5.96.1", |
There was a problem hiding this comment.
@tanstack/react-query-devtools is only loaded in non-production via a NODE_ENV !== 'production' conditional (dynamic import). Keeping it in dependencies increases the production install surface unnecessarily; it should remain in devDependencies unless it’s required at runtime in production.
| "eslint-import-resolver-typescript": "^4.4.4", | ||
| "eslint-plugin-import": "^2.32.0", |
There was a problem hiding this comment.
These newly added ESLint packages use version ranges (^...) whereas most devDependencies are pinned. Consider pinning eslint-import-resolver-typescript and eslint-plugin-import to exact versions to keep dependency updates explicit and consistent.
| "eslint-import-resolver-typescript": "^4.4.4", | |
| "eslint-plugin-import": "^2.32.0", | |
| "eslint-import-resolver-typescript": "4.4.4", | |
| "eslint-plugin-import": "2.32.0", |
| rules: { | ||
| 'import/no-extraneous-dependencies': [ | ||
| 'error', |
There was a problem hiding this comment.
import/no-extraneous-dependencies is configured, but the flat config doesn’t register eslint-plugin-import under plugins. With the current file, ESLint will fail with “Definition for rule 'import/no-extraneous-dependencies' was not found” unless another config happens to register it. Import eslint-plugin-import and add it to this config object’s plugins map (e.g., plugins: { import: importPlugin }) before using import/* rules.
| "@sanity/vision": "5.19.0", | ||
| "@sentry/nextjs": "10.47.0", | ||
| "@tanstack/react-query": "5.96.1", | ||
| "@tanstack/react-query-devtools": "^5.96.1", |
There was a problem hiding this comment.
New dependencies are added with version ranges (^...) while the rest of this package.json pins exact versions. Using ranges here makes installs more likely to drift over time and is inconsistent with the repo’s current versioning approach; consider pinning these to exact versions as well.
| "@tanstack/react-query-devtools": "^5.96.1", | |
| "@tanstack/react-query-devtools": "5.96.1", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/plans/2026-04-28-pnpm-migration.md (1)
35-45:⚠️ Potential issue | 🟡 MinorAdd unresolved questions at the end of the plan.
The plan currently ends with
## Files; it should end with an unresolved-questions section (even if empty).Suggested patch
- `README.md`: local setup and schema typegen commands updated to pnpm - `AGENTS.md`: workflow command references updated to pnpm + +## Unresolved questions + +- NoneAs per coding guidelines:
docs/plans/**/*.md: At end of each plan, list unresolved questions if any.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-04-28-pnpm-migration.md` around lines 35 - 45, The plan file docs/plans/2026-04-28-pnpm-migration.md currently ends with the "## Files" section but is missing the required unresolved-questions section; append a final "## Unresolved questions" (or "## Unresolved Questions") heading at the end of the document and include a short placeholder line (e.g., "None" or leave empty) so the plan complies with the docs/plans/**/*.md guideline and makes any outstanding items explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 50-51: Update the vague Node requirement string "Node.js 24
installed" to the exact pinned runtime "Node.js 24.12.0 installed (managed by
Volta)" so the README matches the repository's runtime pin; find and replace the
current line containing "Node.js 24 installed" and, if present, add or adjust
the Volta note to indicate Volta will manage Node.js 24.12.0.
---
Outside diff comments:
In `@docs/plans/2026-04-28-pnpm-migration.md`:
- Around line 35-45: The plan file docs/plans/2026-04-28-pnpm-migration.md
currently ends with the "## Files" section but is missing the required
unresolved-questions section; append a final "## Unresolved questions" (or "##
Unresolved Questions") heading at the end of the document and include a short
placeholder line (e.g., "None" or leave empty) so the plan complies with the
docs/plans/**/*.md guideline and makes any outstanding items explicit.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 56728f64-7fe2-42f2-8914-736f9898f4c5
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/workflows/ci.yml.github/workflows/crawl.yml.github/workflows/deploy-sanity.ymlAGENTS.mdREADME.mddocs/plans/2026-04-28-pnpm-migration.mdeslint.config.mjspackage.json
| - Node.js 24 installed | ||
| - pnpm 10.5.2 installed |
There was a problem hiding this comment.
Specify exact Node version to avoid setup drift.
Line 50 still uses a broad version (Node.js 24 installed), which conflicts with the repository’s documented runtime pin.
Suggested doc fix
-- Node.js 24 installed
+- Node.js 24.12.0 installed (managed by Volta)Based on learnings: Node version must be 24.12.0, managed by Volta.
📝 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.
| - Node.js 24 installed | |
| - pnpm 10.5.2 installed | |
| - Node.js 24.12.0 installed (managed by Volta) | |
| - pnpm 10.5.2 installed |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 50 - 51, Update the vague Node requirement string
"Node.js 24 installed" to the exact pinned runtime "Node.js 24.12.0 installed
(managed by Volta)" so the README matches the repository's runtime pin; find and
replace the current line containing "Node.js 24 installed" and, if present, add
or adjust the Volta note to indicate Volta will manage Node.js 24.12.0.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - **IMPORTANT**: Dev server runs on port 3003 (not 3000): `npm run dev` | ||
| - **IMPORTANT**: Dev server runs on port 3003 (not 3000): `pnpm run dev` | ||
| - Node version: 24.12.0 (managed by Volta) |
There was a problem hiding this comment.
This doc says Node is “managed by Volta”, but the repo is configured to manage Node via Mise (.mise.toml) and CI uses jdx/mise-action. Update this line to avoid onboarding/setup confusion.
| - Node version: 24.12.0 (managed by Volta) | |
| - Node version: 24.12.0 (managed by Mise) |
| - uses: actions/checkout@v6 | ||
| - uses: jdx/mise-action@v4 | ||
| - uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 10.5.2 |
There was a problem hiding this comment.
actions/checkout is pinned to v6 here, while other workflows in the repo use actions/checkout@v4 (e.g. crawl.yml). It’s safer to align on a single major version across workflows to avoid unexpected behavior differences and simplify maintenance.
|
|
||
| - uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 10.5.2 |
There was a problem hiding this comment.
actions/checkout is pinned to v6 here, while other workflows in the repo use actions/checkout@v4 (e.g. crawl.yml). Consider aligning the major version across workflows to reduce the chance of workflow drift and hard-to-debug CI differences.
| const dependencyCheckConfigValue = { | ||
| files: ['**/*.{js,mjs,cjs,jsx,ts,tsx}'], | ||
| settings: { | ||
| 'import/internal-regex': '^(@/|@data/)', | ||
| 'import/resolver': { | ||
| typescript: { | ||
| project: './tsconfig.json' | ||
| } | ||
| } | ||
| }, | ||
| rules: { | ||
| 'import/no-extraneous-dependencies': [ | ||
| 'error', |
There was a problem hiding this comment.
import/no-extraneous-dependencies is configured, but the flat config does not register eslint-plugin-import under plugins. Without that, ESLint will fail with an unknown rule. Import eslint-plugin-import and add plugins: { import: importPlugin } (and keep the existing settings/rules).
| "@sanity/vision": "5.19.0", | ||
| "@sentry/nextjs": "10.47.0", | ||
| "@tanstack/react-query": "5.96.1", | ||
| "@tanstack/react-query-devtools": "^5.96.1", |
There was a problem hiding this comment.
New dependencies are using semver ranges (^...) while the rest of this repo’s package.json pins exact versions. To keep installs reproducible (especially after the pnpm migration), pin these newly added deps to exact versions as well.
| "@tanstack/react-query-devtools": "^5.96.1", | |
| "@tanstack/react-query-devtools": "5.96.1", |
| @@ -68,6 +73,8 @@ | |||
| "eslint": "9.39.4", | |||
| "eslint-config-next": "16.2.2", | |||
| "eslint-config-prettier": "10.1.8", | |||
| "eslint-import-resolver-typescript": "^4.4.4", | |||
| "eslint-plugin-import": "^2.32.0", | |||
There was a problem hiding this comment.
dotenv-cli, eslint-import-resolver-typescript, and eslint-plugin-import were added with ^ ranges, which breaks the repo’s convention of exact version pinning. Consider switching these to exact versions to avoid lockfile drift.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/plans/2026-04-28-pnpm-migration.md (1)
6-45: Plan can be tightened further to match repo plan-style rules.Condense Purpose/Requirements/Todo/Files into shorter fragments to keep this extremely concise.
As per coding guidelines: "
docs/plans/*.md: Make plan files extremely concise - sacrifice grammar for concision".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-04-28-pnpm-migration.md` around lines 6 - 45, Condense this plan by merging Purpose/Requirements/Todo/Files into a single ultra-compact summary: replace the multi-section prose under "Purpose", "Requirements", "Todo" and "Files" with a 3–5 line bulletless fragment that states the objective (standardize on pnpm), the key actions (add packageManager in package.json, commit pnpm-lock.yaml, remove package-lock.json, update workflows and READMEs, run lint/build/test), and the remaining blocking item (failing e2e test tests/search.spec.ts); keep references to unique artifacts (package.json, pnpm-lock.yaml, .github/workflows/ci.yml, crawl.yml, deploy-sanity.yml, README.md, AGENTS.md) but compress them into a single short comma-separated list and strip explanatory sentences so the file matches the repo's concise docs/plans/* style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Line 27: Change the unpinned pnpm dlx invocation that runs "pnpm dlx vercel
env pull .env.local --token=${{ secrets.VERCEL_TOKEN }} --yes" to a pinned
vercel release, e.g. "pnpm dlx vercel@<version> env pull .env.local --token=${{
secrets.VERCEL_TOKEN }} --yes", and apply the same change to the other identical
dlx invocation (the second occurrence of the pnpm dlx vercel command) so CI uses
a deterministic vercel version.
---
Nitpick comments:
In `@docs/plans/2026-04-28-pnpm-migration.md`:
- Around line 6-45: Condense this plan by merging
Purpose/Requirements/Todo/Files into a single ultra-compact summary: replace the
multi-section prose under "Purpose", "Requirements", "Todo" and "Files" with a
3–5 line bulletless fragment that states the objective (standardize on pnpm),
the key actions (add packageManager in package.json, commit pnpm-lock.yaml,
remove package-lock.json, update workflows and READMEs, run lint/build/test),
and the remaining blocking item (failing e2e test tests/search.spec.ts); keep
references to unique artifacts (package.json, pnpm-lock.yaml,
.github/workflows/ci.yml, crawl.yml, deploy-sanity.yml, README.md, AGENTS.md)
but compress them into a single short comma-separated list and strip explanatory
sentences so the file matches the repo's concise docs/plans/* style.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3958cb40-9c48-436f-b5f5-9a6561932510
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/deploy-sanity.yml.prettierignoreAGENTS.mdREADME.mddocs/plans/2026-04-28-pnpm-migration.mdpackage.json
✅ Files skipped from review due to trivial changes (1)
- .prettierignore
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/deploy-sanity.yml
- README.md
- AGENTS.md
| run: pnpm install --frozen-lockfile | ||
| - name: Pull environment variables from Vercel | ||
| run: npx vercel env pull .env.local --token=${{ secrets.VERCEL_TOKEN }} --yes | ||
| run: pnpm dlx vercel env pull .env.local --token=${{ secrets.VERCEL_TOKEN }} --yes |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all unpinned Vercel dlx usages in workflows
rg -nP 'pnpm dlx vercel(?!@)' .github/workflowsRepository: dejanvasic85/williamstownsc
Length of output: 309
Pin vercel in pnpm dlx for deterministic CI runs.
Unpinned dlx commands download the latest available version at execution time, which can vary across runs and compromise CI reproducibility. Use pnpm dlx vercel@<pinned-version> instead.
Suggested update
- run: pnpm dlx vercel env pull .env.local --token=${{ secrets.VERCEL_TOKEN }} --yes
+ run: pnpm dlx vercel@<pinned-version> env pull .env.local --token=${{ secrets.VERCEL_TOKEN }} --yesAlso applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml at line 27, Change the unpinned pnpm dlx invocation
that runs "pnpm dlx vercel env pull .env.local --token=${{ secrets.VERCEL_TOKEN
}} --yes" to a pinned vercel release, e.g. "pnpm dlx vercel@<version> env pull
.env.local --token=${{ secrets.VERCEL_TOKEN }} --yes", and apply the same change
to the other identical dlx invocation (the second occurrence of the pnpm dlx
vercel command) so CI uses a deterministic vercel version.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Prerequisites | ||
|
|
||
| - Node.js 24 installed | ||
| - Node.js installed (managed by Mise) |
There was a problem hiding this comment.
README prerequisites now mention Node.js is managed by Mise, but the pinned Node version isn’t stated. Consider including the exact version from .mise.toml (24.12.0) to avoid local/CI drift.
| - Node.js installed (managed by Mise) | |
| - Node.js 24.12.0 installed (managed by Mise) |
| "name": "williamstownsc", | ||
| "version": "1.0.0", | ||
| "private": true, | ||
| "packageManager": "pnpm@10.33.2", |
There was a problem hiding this comment.
PR description mentions migrating to pnpm v10.5.2, but the repo is pinned to pnpm@10.33.2 via the packageManager field (and workflows also install 10.33.2). Please align the PR description/release notes with the actual pinned pnpm version (or change the pin if 10.5.2 is intended).
Summary by CodeRabbit
Chores
Documentation