chore: migrate from yarn to pnpm#420
Conversation
📝 WalkthroughWalkthroughThis PR migrates the repository from Yarn to PNPM: CI/workflows, docs, Makefile, wrangler build commands, and package metadata are updated to use PNPM, node engine requirement raised to >=20, pnpm-workspace.yaml added, and wrangler dev dependency bumped. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
9da9e77 to
5239a3f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
Makefile (1)
19-19: Add--frozen-lockfileto pnpm install in lint/format targets for reproducibility.The
fmtandmd-linttargets usepnpm iwithout--frozen-lockfile, which can mutate the lockfile if dependencies drift. Frozen installs ensure consistent behavior across runs.Suggested change
- corepack enable pnpm && pnpm i && pnpm md-fmt + corepack enable pnpm && pnpm i --frozen-lockfile && pnpm md-fmt- corepack enable pnpm && pnpm i && pnpm md-check + corepack enable pnpm && pnpm i --frozen-lockfile && pnpm md-check🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 19, Update the Makefile's format/lint targets to use a frozen install so the lockfile isn't mutated: replace the plain `pnpm i` invocation (seen in the command `corepack enable pnpm && pnpm i && pnpm md-fmt`) with `pnpm i --frozen-lockfile` (and do the same for the md-lint/other lint targets that call `pnpm i`) so `fmt` and `md-lint` targets perform reproducible installs..github/workflows/deploy.yml (1)
50-52: Pin PNPM version viapackageManagerfield to ensure consistent CI runs.The
packageManagerfield is not set inpackage.json. Whileengines.pnpm: >=10exists, this is only a version constraint, not a pin—it allows any version ≥10, leaving the CI vulnerable to drift. Usingcorepack enable pnpmwithout an explicit pinnedpackageManagerinpackage.json(e.g.,"packageManager": "pnpm@10.0.0") will resolve to unpredictable pnpm versions across CI runs. Updatepackage.jsonto include a pinnedpackageManagerfield for deterministic behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy.yml around lines 50 - 52, CI can pick different pnpm versions because package.json lacks a pinned packageManager; add a "packageManager" field to package.json (e.g., "packageManager":"pnpm@10.0.0") to pin the pnpm version, keeping the existing engines.pnpm entry if desired, then commit that change so corepack enable pnpm and pnpm i --frozen-lockfile in the workflow use the pinned version consistently..github/workflows/e2e.yml (1)
58-68: Consider usingactions/setup-nodewithcache: pnpmfor better dependency caching.The workflow has migrated to pnpm but continues to cache
~/.npm(npm's cache directory), which won't effectively cache pnpm's store. Useactions/setup-node@v4(or later) withcache: pnpmto leverage pnpm-aware caching, or manually cache the pnpm store directory for improved CI performance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yml around lines 58 - 68, The "Cache dependencies" workflow step currently uses actions/cache@v5 and caches ~/.npm (npm cache) which is ineffective for pnpm; update the workflow's "Cache dependencies" step (the step with name "Cache dependencies" and uses: actions/cache@v5) to either: 1) replace that step with actions/setup-node@v4 and set cache: 'pnpm' and cache-dependency-path: '**/pnpm-lock.yaml' (and ensure node-version is set), or 2) if keeping actions/cache, stop caching ~/.npm and instead cache the pnpm store directory (e.g. ~/.pnpm-store or the distro-specific pnpm store path) and update the key to include pnpm-lock.yaml; make the change where the current key uses hashFiles('**/pnpm-lock.yaml','**/Cargo.lock') and remove ~/.npm from the path list.package.json (1)
5-6: AddingpackageManagerfield is optional, not required for reproducibility.The repo already uses
corepack enable pnpmin CI combined withpnpm-lock.yamland--frozen-lockfile, which ensures reproducible builds. Adding"packageManager": "pnpm@<exact-version>"would only provide explicit local consistency for developers; it's not necessary for CI reproducibility. If pursuing this enhancement, first determine the exact pnpm version used in your CI environment and pin it accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 5 - 6, The package.json currently lists engine constraints ("node": ">=20", "pnpm": ">=10") but omits a packageManager field; if you want explicit local reproducibility for developers add a "packageManager": "pnpm@<exact-version>" entry to package.json (pin the exact pnpm version used in CI), otherwise leave as-is because CI already guarantees reproducible installs via corepack enable pnpm, pnpm-lock.yaml and --frozen-lockfile; to implement, determine the exact pnpm version from your CI environment and add the packageManager field with that pinned version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/deploy.yml:
- Around line 50-52: CI can pick different pnpm versions because package.json
lacks a pinned packageManager; add a "packageManager" field to package.json
(e.g., "packageManager":"pnpm@10.0.0") to pin the pnpm version, keeping the
existing engines.pnpm entry if desired, then commit that change so corepack
enable pnpm and pnpm i --frozen-lockfile in the workflow use the pinned version
consistently.
In @.github/workflows/e2e.yml:
- Around line 58-68: The "Cache dependencies" workflow step currently uses
actions/cache@v5 and caches ~/.npm (npm cache) which is ineffective for pnpm;
update the workflow's "Cache dependencies" step (the step with name "Cache
dependencies" and uses: actions/cache@v5) to either: 1) replace that step with
actions/setup-node@v4 and set cache: 'pnpm' and cache-dependency-path:
'**/pnpm-lock.yaml' (and ensure node-version is set), or 2) if keeping
actions/cache, stop caching ~/.npm and instead cache the pnpm store directory
(e.g. ~/.pnpm-store or the distro-specific pnpm store path) and update the key
to include pnpm-lock.yaml; make the change where the current key uses
hashFiles('**/pnpm-lock.yaml','**/Cargo.lock') and remove ~/.npm from the path
list.
In `@Makefile`:
- Line 19: Update the Makefile's format/lint targets to use a frozen install so
the lockfile isn't mutated: replace the plain `pnpm i` invocation (seen in the
command `corepack enable pnpm && pnpm i && pnpm md-fmt`) with `pnpm i
--frozen-lockfile` (and do the same for the md-lint/other lint targets that call
`pnpm i`) so `fmt` and `md-lint` targets perform reproducible installs.
In `@package.json`:
- Around line 5-6: The package.json currently lists engine constraints ("node":
">=20", "pnpm": ">=10") but omits a packageManager field; if you want explicit
local reproducibility for developers add a "packageManager":
"pnpm@<exact-version>" entry to package.json (pin the exact pnpm version used in
CI), otherwise leave as-is because CI already guarantees reproducible installs
via corepack enable pnpm, pnpm-lock.yaml and --frozen-lockfile; to implement,
determine the exact pnpm version from your CI environment and add the
packageManager field with that pinned version.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
.github/workflows/deploy.yml.github/workflows/e2e.ymlMakefileREADME.mddocs/cors_testing_guide.mddocs/e2e_api_testing_guide.mdpackage.jsonpnpm-workspace.yamlwrangler.toml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #420 +/- ##
=======================================
Coverage 41.11% 41.11%
=======================================
Files 40 40
Lines 2668 2668
=======================================
Hits 1097 1097
Misses 1571 1571 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/e2e.yml (2)
107-107: Pin PNPM version in CI to avoid toolchain drift.Line 107 enables pnpm, but the workflow step itself does not pin a concrete pnpm version. Consider explicit pinning (unless already enforced via
packageManagerinpackage.json).♻️ Suggested change
- corepack enable pnpm + corepack enable + corepack prepare pnpm@<pinned-version> --activate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yml at line 107, The CI currently runs the unpinned command "corepack enable pnpm", which can cause toolchain drift; replace or supplement that step by preparing and activating a specific pnpm release (e.g., use "corepack prepare pnpm@<version> --activate") so the workflow consistently uses a pinned pnpm version, or alternatively ensure the concrete version is enforced via the packageManager field in package.json and reference that version when calling corepack; update the step that contains "corepack enable pnpm" to call corepack prepare with the chosen semantically pinned version.
67-68: Cache PNPM store, not npm cache, for this workflow.Line 67 still caches
~/.npm; with pnpm installs, caching the pnpm store is usually the meaningful speed-up path.♻️ Suggested change
path: | ~/.cargo/bin/ ~/.cargo/registry/index/ ~/.cargo/registry/cache/ ~/.cargo/git/db/ target/ - ~/.npm + ~/.pnpm-store🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yml around lines 67 - 68, Replace the npm cache path with the pnpm store path in the workflow cache step: change the cache path from "~/.npm" to the pnpm store directory (e.g., "~/.pnpm-store" or the distro-appropriate pnpm store such as "~/.local/share/pnpm") so the cache actually stores pnpm artifacts; keep the existing cache key expression (key: ${{ runner.os }}-deps-${{ hashFiles('**/pnpm-lock.yaml', '**/Cargo.lock') }}).Makefile (1)
19-19: Prefer frozen installs in automation-facing lint/format targets.Line 19 and Line 24 use
pnpm i, which can rewritepnpm-lock.yaml. Use frozen lockfile installs to keep runs deterministic.♻️ Suggested change
- corepack enable pnpm && pnpm i && pnpm md-fmt + corepack enable pnpm && pnpm install --frozen-lockfile && pnpm md-fmt ... - corepack enable pnpm && pnpm i && pnpm md-check + corepack enable pnpm && pnpm install --frozen-lockfile && pnpm md-checkAlso applies to: 24-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 19, The Makefile uses non-frozen installs ("pnpm i") in automation-facing targets; replace each occurrence of "pnpm i" with a frozen-lockfile install (e.g., "pnpm install --frozen-lockfile" or "pnpm --frozen-lockfile install") so runs are deterministic and fail when pnpm-lock.yaml would be rewritten—update the command instances that currently read "corepack enable pnpm && pnpm i && pnpm md-fmt" (and the similar occurrence later) to use the frozen-lockfile flag instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/e2e.yml:
- Line 107: The CI currently runs the unpinned command "corepack enable pnpm",
which can cause toolchain drift; replace or supplement that step by preparing
and activating a specific pnpm release (e.g., use "corepack prepare
pnpm@<version> --activate") so the workflow consistently uses a pinned pnpm
version, or alternatively ensure the concrete version is enforced via the
packageManager field in package.json and reference that version when calling
corepack; update the step that contains "corepack enable pnpm" to call corepack
prepare with the chosen semantically pinned version.
- Around line 67-68: Replace the npm cache path with the pnpm store path in the
workflow cache step: change the cache path from "~/.npm" to the pnpm store
directory (e.g., "~/.pnpm-store" or the distro-appropriate pnpm store such as
"~/.local/share/pnpm") so the cache actually stores pnpm artifacts; keep the
existing cache key expression (key: ${{ runner.os }}-deps-${{
hashFiles('**/pnpm-lock.yaml', '**/Cargo.lock') }}).
In `@Makefile`:
- Line 19: The Makefile uses non-frozen installs ("pnpm i") in automation-facing
targets; replace each occurrence of "pnpm i" with a frozen-lockfile install
(e.g., "pnpm install --frozen-lockfile" or "pnpm --frozen-lockfile install") so
runs are deterministic and fail when pnpm-lock.yaml would be rewritten—update
the command instances that currently read "corepack enable pnpm && pnpm i &&
pnpm md-fmt" (and the similar occurrence later) to use the frozen-lockfile flag
instead.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.github/workflows/deploy.yml.github/workflows/e2e.yml.prettierignoreMakefileREADME.mddocs/cors_testing_guide.mddocs/e2e_api_testing_guide.mdpackage.jsonpnpm-workspace.yamlwrangler.toml
🚧 Files skipped from review as they are similar to previous changes (6)
- package.json
- .github/workflows/deploy.yml
- docs/cors_testing_guide.md
- README.md
- wrangler.toml
- docs/e2e_api_testing_guide.md
Summary of changes
follow up on ChainSafe/forest#6586
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
adheres to the team's
documentation standards,
(if possible),
should be reflected in this document.
Summary by CodeRabbit
Chores
Documentation