chore: provision node and bun via nix instead of pnpm - #1270
Conversation
The test job spent ~73s on "Warm Nix dev shell" realizing the complete development shell — every linter, formatter, renovate, and the pre-commit hook closure — even though pnpm install itself was only ~7s. Now that coverage is a `nix build` derivation, Vitest is the sole step that needs a JS toolchain. Replace `nix develop` with `nix profile install pnpm_11 bun just`, mirroring the perf-comment and pkg-pr-new jobs, then run `just test-vitest` directly. This skips the heavy shell closure while keeping the exact tools Vitest needs.
`package.json` declared `engines.runtime` for node and bun with `onFail: download`, so pnpm fetched and managed those runtimes itself. That made every environment depend on a pnpm-managed download at runtime instead of the pinned Nix toolchain. Drop `engines.runtime` and provide both runtimes explicitly: - dev shell: add `nodejs_24` (bun was already present). - test job: install `nodejs_24` for Vitest (dropping the unused bun). - pkg-pr-new: add `nodejs_24` and `bun` for the tsdown / ensure-native-binary prepack scripts that previously relied on the managed runtimes. - perf jobs: add `nodejs_24` alongside the existing bun. - release: add `bun`; node still comes from `setup-node`, which is kept for the npm registry auth and provenance setup. Verified locally with a clean pnpm state: `pnpm install --frozen-lockfile` no longer manages runtimes and Vitest runs on the PATH node.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The lockfile recorded the `engines.runtime` node/bun entries as `runtime:` dependencies. With `engines.runtime` removed, regenerate the lockfile so `pnpm install --frozen-lockfile` stays in sync and no longer resolves the pnpm-managed runtimes.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — mechanical swap from pnpm-managed runtime downloads to Nix-provisioned runtimes for Node.js and Bun across dev shell, CI jobs, and release workflow.
- Drop
engines.runtimefrompackage.json— pnpm no longer manages node/bun downloads at install time - Add
nodejs_24to Nix dev shell — bun was already present; node now joins it - CI test job — stop realizing full dev shell — replaces
nix develop --command true+nix develop --command just test-vitestwith targetednix profile installfor pnpm, node, and just, then plainjust test-vitest - CI pkg-pr-new job — add node + bun — both runtimes needed for tsdown and ensure-native-binary prepack scripts
- CI perf jobs — add
nodejs_24— perf scripts may need node alongside existing bun - Release workflow — add
bun— needed for ensure-native-binary; node stays onsetup-nodefor npm registry auth - pnpm-lock.yaml — remove runtime entries — drops the
bun@runtime:1.3.14andnode@runtime:24.15.0package entries and their snapshot stubs
Big Pickle (free via Pullfrog for OSS) ｜ 𝕏
Pin to the unversioned `pnpm` and `nodejs` attributes rather than `pnpm_11` / `nodejs_24`. `pnpm` currently resolves to 11.1.1 (same as the explicit alias, matching the lockfile) and `nodejs` tracks the active LTS, so the dev shell and CI follow the maintained defaults without hard-coding a major.
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
1 participant
Summary
package.jsondeclaredengines.runtimefor node and bun withonFail: download, so pnpm fetched and managed those runtimes itself ratherthan using the pinned Nix toolchain. This removes that and provisions both
runtimes explicitly via Nix.
What changed
engines.runtime.nodejs_24(bun already present).test: installnodejs_24for Vitest (drop the unused bun).pkg-pr-new: addnodejs_24+bunfor the tsdown / ensure-native-binary prepack scripts.nodejs_24alongside bun.bun; node still comes fromsetup-node(kept — needed for npm registry auth / provenance).Verification
With a clean pnpm state (
PNPM_HOME/XDG dirs in a tempdir), after removingengines.runtime:pnpm install --frozen-lockfileno longer manages/downloads node or bun.pnpm exec vitest runruns on the PATH Nix node (v24.14.1); 34 tests pass.Notes
mainas those merge.setup-nodein release.yaml and preview-e2e is intentionally retained.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Provision Node and Bun via Nix (using default
nodejs/pnpm) so dev and CI run on consistent, pinned runtimes without downloads. Also speeds up tests by installing only the JS tools instead of realizing the full dev shell.engines.runtimefrompackage.json; regeneratedpnpm-lock.yamlto dropruntime:Node/Bun entries.nodejsandpnpm(replacesnodejs_24/pnpm_11);bununchanged.nix profileinstalls. Tests installpnpm/nodejs/justand runjust test-vitest(nonix develop).pkg-pr-newand perf installnodejs+bun. Release installsbunvia Nix; keepsetup-nodefor npm auth/provenance.Written for commit 509f5df. Summary will update on new commits.