Skip to content

yarn -> pnpm, volta -> vite+ (BL-16115)#611

Open
andrew-polk wants to merge 2 commits into
masterfrom
BL_harden_deps_pnpm
Open

yarn -> pnpm, volta -> vite+ (BL-16115)#611
andrew-polk wants to merge 2 commits into
masterfrom
BL_harden_deps_pnpm

Conversation

@andrew-polk

@andrew-polk andrew-polk commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

This change is Reviewable

Summary by CodeRabbit

  • Chores
    • Migrated build and development infrastructure to Vite+ (vp) and pnpm package manager
    • Updated Node.js version requirement to 22.22.3
    • Converted dependency version specifications to exact pins for improved stability
    • Implemented supply-chain security hardening measures in package management

andrew-polk and others added 2 commits June 9, 2026 14:16
Use the Vite+ (vp) CLI to manage the Node.js runtime and the pnpm package
manager instead of Volta. vp reads the Node version from .node-version and
the package manager from package.json's packageManager field.

Bump Node 20.18.0 -> 22.22.3: pnpm 11.5.2, when run via Vite+'s bundled
pnpm, requires Node >= 22.13 (it uses the node:sqlite builtin). Align
@types/node to the 22.x line accordingly.

CI now uses voidzero-dev/setup-vp@v1 (pinned to vp 0.1.24) plus
`vp install --frozen-lockfile` and `vp run <script>`. The build/test
scripts still run the pinned Vite 5 / vitest; routing build/dev/test
through `vp build|dev|test` is a follow-up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR replaces the Yarn-based build system with pnpm and the Vite+ (vp) task runner, pins all dependency versions exactly, adds supply-chain security hardening, updates Node to 22.22.3, and upgrades CI/CD workflows and developer documentation to use the new toolchain.

Changes

Package Manager & CI/CD Toolchain Migration

Layer / File(s) Summary
Package Manager Configuration & Dependency Pinning
package.json, pnpm-workspace.yaml, .node-version
Scripts and dependencies migrate from Yarn to pnpm with exact version pinning. pnpm-workspace.yaml adds release-age delays (7d, excluding bloom-player), blocks exotic transitive sources, enforces script allowlists, and pins @types/react@17.0.83. Node updates to 22.22.3. Volta and resolutions blocks removed; packageManager set to pnpm@11.5.2.
CI/CD Workflow Automation
.github/workflows/build-and-deploy.yml
Workflow adds voidzero-dev/setup-vp@v1 (v0.1.24) to manage vp toolchain and Node from .node-version, enabling action caching. Dependency install becomes vp install --frozen-lockfile. Crowdin, build (master: build:ci:alpha, others: build:ci), and test steps all convert to vp run equivalents with BLOOM_CROWDIN_TOKEN preserved.
Local Hooks & Dependency Cleanup
.husky/pre-commit, .gitignore
Pre-commit hook calls node_modules/.bin/pretty-quick --staged and node_modules/.bin/lint-staged instead of yarn wrappers. .gitignore removes .pnp, .pnp.js, .yarn/*, .yarnrc.yml, and yarn debug patterns; adds pnpm-debug.log*.
Documentation & Command References
README.md, index.html, src/translations/README.md, vite.config.ts
Developer setup instructions, HTML template comments, Crowdin command docs, and config comments all updated to reference vp install, vp run dev/storybook/test/build/crowdin-download and pnpm link instead of yarn equivalents.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'yarn -> pnpm, volta -> vite+ (BL-16115)' accurately and concisely summarizes the main changes: migrating from yarn to pnpm and from volta to vite+.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR migrates the project from yarn + volta to pnpm + Vite+ (vp), updates the GitHub Actions CI workflow to use the voidzero-dev/setup-vp action, and adds a pnpm-workspace.yaml with supply-chain hardening settings (minimumReleaseAge, strictDepBuilds, allowBuilds).

  • All yarn invocations across scripts, CI, docs, and hooks are replaced with pnpm/vp run equivalents; the bloom-player@alpha upgrade on master is ported to pnpm add -D bloom-player@alpha inside build:ci:alpha.
  • pnpm-workspace.yaml introduces security controls (7-day release-age gate, exotic-subdep blocking, an explicit build-script allowlist) with bloom-player excluded from the delay to keep the alpha-build workflow functional.
  • yarn.lock is deleted and replaced with pnpm-lock.yaml; all dependency versions are pinned exactly (no ^/~) to match the savePrefix: \"\" workspace setting.

Important Files Changed

Filename Overview
.github/workflows/build-and-deploy.yml Migrated from yarn/volta to vp (Vite+); install/build/test commands updated correctly. Uses mutable setup-vp@v1 tag instead of a pinned SHA.
pnpm-workspace.yaml New file adding supply-chain hardening: allowBuilds allowlist, blockExoticSubdeps, and a 7-day minimumReleaseAge. The minimumReleaseAgeExclude for bloom-player only covers the package itself, not its transitive deps, which could cause master alpha-build failures in strict mode.
package.json Switched packageManager to pnpm@11.5.2, removed volta config and yarn resolutions, pinned all dependencies to exact versions, and updated scripts from yarn to pnpm. build:ci:alpha correctly uses pnpm add -D bloom-player@alpha instead of the old yarn upgrade.
.husky/pre-commit Replaced yarn pretty-quick/lint-staged with direct node_modules/.bin/ paths, which is correct for pnpm since it doesn't expose a pnpm exec shim by default in hooks.
.node-version New file pinning Node.js to 22.22.3 for Vite+/setup-vp to consume; replaces the volta block in package.json.
.gitignore Removed yarn/volta specific entries (.pnp, .yarn/, .yarnrc.yml) and replaced yarn-debug/error.log with pnpm-debug.log.

Reviews (1): Last reviewed commit: "volta -> Vite+ for Node.js and pnpm mana..." | Re-trigger Greptile

Comment on lines +20 to +24
uses: voidzero-dev/setup-vp@v1
with:
version: 0.1.24
node-version-file: .node-version
cache: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Mutable action tag for setup-vp

voidzero-dev/setup-vp@v1 resolves to whatever the v1 tag currently points to — a future push to that tag by voidzero-dev would run different code on your next build without any change to this file. The setup-vp docs acknowledge this and offer an "Advanced" composition with socketdev/action@<sha> for full SHA-pinning. Note that actions/checkout@v3 has the same exposure, but that pre-dates this PR. Pinning to a commit SHA (letting Renovate bump it) is the hardened posture.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread pnpm-workspace.yaml
Comment on lines +8 to +12
# bloom-player is our own controlled package; we want to consume newly published
# alphas immediately, so exclude it from the release-age delay above.
minimumReleaseAgeExclude:
- bloom-player

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 minimumReleaseAgeExclude does not cover bloom-player's transitive deps

minimumReleaseAgeExclude: - bloom-player exempts the bloom-player package itself from the 7-day delay, but pnpm's resolution will still enforce minimumReleaseAge on any new transitive dep that bloom-player's latest alpha pulls in. Because minimumReleaseAge is explicitly set, pnpm 11 defaults minimumReleaseAgeStrict to true, meaning a freshly-published transitive dep would cause pnpm add -D bloom-player@alpha to exit non-zero and break every master build until that dep ages out. Adding minimumReleaseAgeStrict: false to the workspace config would keep the delay for normal installs while letting the alpha update step succeed regardless of transitive dep ages.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 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/build-and-deploy.yml:
- Line 20: Replace the mutable action reference `uses: voidzero-dev/setup-vp@v1`
with the immutable commit SHA that it currently resolves to (e.g. `uses:
voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1`) so the workflow
is pinned and cannot drift; update the `uses` entry in the workflow to that SHA
and commit the change.

In @.husky/pre-commit:
- Around line 4-5: Replace direct node_modules/.bin invocations in the
pre-commit hook with pnpm exec to align with pnpm conventions: locate the two
commands where node_modules/.bin/pretty-quick --staged and
node_modules/.bin/lint-staged are invoked and change them to use pnpm exec
(i.e., run the same tools via pnpm exec pretty-quick --staged and pnpm exec
lint-staged) so pnpm resolves and runs the executables.

In `@package.json`:
- Line 10: The npm scripts "build:ci" and "test:ci" use "set CI=true&& pnpm ..."
which doesn't export CI to the command in POSIX shells; replace those with an
inline env assignment so the CI variable is set for the pnpm invocation (e.g.,
change the "build:ci" and "test:ci" script values from the current "set
CI=true&& pnpm ..." form to use "CI=true pnpm ..." so the environment variable
is applied to the subsequent command).
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c23fc258-9e87-44ce-86cf-7257e6e8edb7

📥 Commits

Reviewing files that changed from the base of the PR and between 2a19aa8 and 550ce80.

⛔ Files ignored due to path filters (2)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (10)
  • .github/workflows/build-and-deploy.yml
  • .gitignore
  • .husky/pre-commit
  • .node-version
  • README.md
  • index.html
  • package.json
  • pnpm-workspace.yaml
  • src/translations/README.md
  • vite.config.ts

uses: actions/checkout@v3

- name: Install Vite+ (manages Node.js and pnpm)
uses: voidzero-dev/setup-vp@v1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Resolve the current object behind tag v1 (verify before pinning).
ref_json="$(gh api repos/voidzero-dev/setup-vp/git/ref/tags/v1)"
type="$(jq -r '.object.type' <<<"$ref_json")"
sha="$(jq -r '.object.sha' <<<"$ref_json")"

if [ "$type" = "tag" ]; then
  gh api repos/voidzero-dev/setup-vp/git/tags/"$sha" --jq '.object.sha'
else
  echo "$sha"
fi

Repository: BloomBooks/BloomLibrary2

Length of output: 109


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Confirm the exact line content in the workflow
rg -n "voidzero-dev/setup-vp@v1" .github/workflows/build-and-deploy.yml || true
nl -ba .github/workflows/build-and-deploy.yml | sed -n '1,80p'

Repository: BloomBooks/BloomLibrary2

Length of output: 177


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Resolve tag v1 -> commit SHA (matches prior run)
ref_json="$(gh api repos/voidzero-dev/setup-vp/git/ref/tags/v1)"
type="$(jq -r '.object.type' <<<"$ref_json")"
sha="$(jq -r '.object.sha' <<<"$ref_json")"

if [ "$type" = "tag" ]; then
  gh api repos/voidzero-dev/setup-vp/git/tags/"$sha" --jq '.object.sha'
else
  echo "$sha"
fi

Repository: BloomBooks/BloomLibrary2

Length of output: 109


Pin voidzero-dev/setup-vp to an immutable commit SHA (avoid mutable @v1)

In .github/workflows/build-and-deploy.yml (Line 20), uses: voidzero-dev/setup-vp@v1 is mutable and currently resolves to 2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1. Pin to that SHA to prevent CI supply-chain drift.

🔒 Suggested fix
-              uses: voidzero-dev/setup-vp@v1
+              uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1
📝 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
uses: voidzero-dev/setup-vp@v1
uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1
🧰 Tools
🪛 zizmor (1.25.2)

[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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/build-and-deploy.yml at line 20, Replace the mutable
action reference `uses: voidzero-dev/setup-vp@v1` with the immutable commit SHA
that it currently resolves to (e.g. `uses:
voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1`) so the workflow
is pinned and cannot drift; update the `uses` entry in the workflow to that SHA
and commit the change.

Source: Linters/SAST tools

Comment thread .husky/pre-commit
Comment on lines +4 to +5
node_modules/.bin/pretty-quick --staged
node_modules/.bin/lint-staged

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider using pnpm exec for better ecosystem alignment.

While the direct node_modules/.bin/ invocation is functionally correct, using pnpm exec is more idiomatic for pnpm-based projects and explicitly delegates executable resolution to pnpm.

♻️ Proposed refactor using pnpm exec
-node_modules/.bin/pretty-quick --staged
-node_modules/.bin/lint-staged
+pnpm exec pretty-quick --staged
+pnpm exec lint-staged
📝 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
node_modules/.bin/pretty-quick --staged
node_modules/.bin/lint-staged
pnpm exec pretty-quick --staged
pnpm exec lint-staged
🤖 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 @.husky/pre-commit around lines 4 - 5, Replace direct node_modules/.bin
invocations in the pre-commit hook with pnpm exec to align with pnpm
conventions: locate the two commands where node_modules/.bin/pretty-quick
--staged and node_modules/.bin/lint-staged are invoked and change them to use
pnpm exec (i.e., run the same tools via pnpm exec pretty-quick --staged and pnpm
exec lint-staged) so pnpm resolves and runs the executables.

Comment thread package.json
"build:ci": "set CI=true&& yarn build",
"build:ci:alpha": "yarn upgrade bloom-player --network-timeout 1000000000 && yarn build:ci",
"build": "pnpm copyInBloomPlayer && vite build",
"build:ci": "set CI=true&& pnpm build",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find affected scripts:"
rg -n '"(build:ci|test:ci)"\s*:\s*"set CI=true&&' package.json

echo "Demonstrate POSIX behavior:"
sh -c 'set CI=true&& [ -n "${CI:-}" ] && echo "CI is set" || echo "CI is NOT set"'
# Expected: "CI is NOT set"

Repository: BloomBooks/BloomLibrary2

Length of output: 229


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Current process CI env:"
printf 'CI=%q\n' "${CI-}"

echo
echo "Test 1: 'set CI=true&&' with CI explicitly unset (should show CI empty if claim is correct):"
env -u CI sh -c 'set CI=true && printf "CI=%s; positional $1=%s\n" "${CI-}" "${1-}"'

echo
echo "Test 2: check whether CI becomes set via 'set CI=true&&' by examining ${CI:-}:"
env -u CI sh -c 'set CI=true && [ -n "${CI:-}" ] && echo "CI is set" || echo "CI is NOT set"; [ -n "${1:-}" ] && echo "positional set ($1=$1)"'

echo
echo "Test 3: inline env assignment 'CI=true ...' with CI explicitly unset:"
env -u CI sh -c 'CI=true pnpm -v >/dev/null 2>&1 || true; printf "CI=%s; positional $1=%s\n" "${CI-}" "${1-}"'

Repository: BloomBooks/BloomLibrary2

Length of output: 439


Fix CI env assignment in build:ci/test:ci

set CI=true&& ... does not set the CI environment variable for the following pnpm command (in POSIX sh, set changes positional parameters). Replace with inline env assignment.

💡 Suggested fix
-        "build:ci": "set CI=true&& pnpm build",
+        "build:ci": "CI=true pnpm build",
...
-        "test:ci": "set CI=true&& pnpm test",
+        "test:ci": "CI=true pnpm test",
📝 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
"build:ci": "set CI=true&& pnpm build",
"build:ci": "CI=true pnpm build",
"build:ci:alpha": "bloom-player@alpha pnpm build",
...
"test:ci": "CI=true pnpm test",
🤖 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 `@package.json` at line 10, The npm scripts "build:ci" and "test:ci" use "set
CI=true&& pnpm ..." which doesn't export CI to the command in POSIX shells;
replace those with an inline env assignment so the CI variable is set for the
pnpm invocation (e.g., change the "build:ci" and "test:ci" script values from
the current "set CI=true&& pnpm ..." form to use "CI=true pnpm ..." so the
environment variable is applied to the subsequent command).

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.

1 participant