Prefer pnpm for init and typegen#261
Conversation
- warn npm users to switch to pnpm 11+ - use manager-specific exec commands in scaffolded scripts - keep packageManager metadata and tests aligned
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 2bf8bfb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe init flow now resolves the effective package manager (preferring pnpm when npm is requested and available), computes a package-manager-specific exec command, substitutes a PNPM_EXECUTE_COMMAND placeholder into templates, and records package-manager version under packageJson.devEngines instead of packageManager. ChangesNPM to PNPM Smart Switching and Exec Command Templating
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (resolveInitRequest / planInit / executeInitPlan)
participant PMS as PackageManagerService
participant Prompt as PromptService
participant FS as FileSystem
User->>CLI: start init (requested packageManager: npm)
CLI->>PMS: detect pnpm version
PMS-->>CLI: pnpm available? (version or error)
alt pnpm available
CLI->>CLI: set packageManager = pnpm
else pnpm missing & interactive
CLI->>Prompt: show npm→pnpm warning (abort/continue)
Prompt-->>CLI: user choice (abort / continue)
alt abort
CLI-->>User: throw UserCancelledError
else continue
CLI->>CLI: keep packageManager = npm
end
else non-interactive
CLI->>CLI: keep packageManager = npm
end
CLI->>CLI: planInit computes packageManagerExecuteCommand
CLI->>FS: executeInitPlan replaces __PNPM_EXECUTE_COMMAND__ in scaffold
FS-->>CLI: scaffold files updated
CLI-->>User: renderNextSteps (includes PNPM warning when npm)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 4
🤖 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 `@packages/cli/src/cli/init.ts`:
- Line 289: The packageManager value is being written with a range
(`${pkgManager}>=${stdout.trim()}`) which Corepack rejects; update the
assignment for pkgJson.packageManager to use an exact specifier by concatenating
the package manager name and the exact version (use `@${stdout.trim()}`), i.e.
set pkgJson.packageManager to `${pkgManager}@${stdout.trim()}` so Corepack can
resolve it correctly; change the assignment near where pkgJson, pkgManager, and
stdout.trim() are used.
In `@packages/cli/src/core/planInit.ts`:
- Line 185: Replace the hardcoded "npx `@tanstack/intent`@latest install" entry in
the nextSteps array with a package-manager-aware invocation: use the existing
packageManagerExecuteCommand variable (the same helper used by
formatPackageManagerCommand) to build the command string so it works for
npm/yarn/pnpm/bun, e.g. construct the entry as `${packageManagerExecuteCommand}
`@tanstack/intent`@latest install` where the current literal appears (look for the
"npx `@tanstack/intent`@latest install" string in planInit.ts and update it).
In `@packages/cli/tests/executor.test.ts`:
- Around line 368-370: The test's exact-string assertion for the pnpm warning is
brittle; update the assertion in packages/cli/tests/executor.test.ts (the
expect(console.info.join("\n")) line) to check intent instead of exact
copy—match a regex or use includes that looks for the invariant parts like
"pnpm" and "11" (e.g. a case-insensitive regex matching "pnpm.*11" or "pnpm.*11
or greater") so the test passes even if surrounding phrasing changes.
In `@packages/cli/tests/planner.test.ts`:
- Around line 128-130: The test asserts an exact warning string on
plan.nextSteps which is brittle; update the assertion to match more loosely
(e.g., use a regex or Jest matchers) so it only checks for the presence of
"pnpm" and "11" case-insensitively. Locate the expect against plan.nextSteps in
the planner.test.ts and replace the exact .toContain(...) string check with a
regex-based check (e.g.,
expect(plan.nextSteps).toEqual(expect.arrayContaining([expect.stringMatching(/pnpm.*11/i)]))
or equivalent) to avoid failures due to minor copy/casing changes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f4b19d58-a7c5-4607-9789-739c755f1fb1
📒 Files selected for processing (15)
.changeset/npm-pnpm-warning.md.changeset/typegen-exec-command.mdpackages/cli/src/cli/init.tspackages/cli/src/core/executeInitPlan.tspackages/cli/src/core/planInit.tspackages/cli/src/core/resolveInitRequest.tspackages/cli/src/core/types.tspackages/cli/src/utils/projectFiles.tspackages/cli/template/vite-wv/package.jsonpackages/cli/tests/executor.test.tspackages/cli/tests/init-scaffold-contract.test.tspackages/cli/tests/planner.test.tspackages/cli/tests/resolve-init.test.tspackages/cli/tests/test-layer.tspackages/cli/tests/webviewer-apps.test.ts
| cwd: projectDir, | ||
| }); | ||
| pkgJson.packageManager = `${pkgManager}@${stdout.trim()}`; | ||
| pkgJson.packageManager = `${pkgManager}>=${stdout.trim()}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does the packageManager field in package.json support version range syntax like >= or does it require exact versions?
💡 Result:
No, the packageManager field in package.json does not support full version range syntax like >=1.0.0 or ^1.0.0; it requires an exact version specifier that resolves to a single specific version.[1][2][3][4] The field follows the format (npm|yarn|pnpm)@Version, where "version" must be a specifier that Corepack can resolve to exactly one version, such as a full semver version (e.g., "pnpm@8.15.4"), a tag like "latest", or a range like "7.x" when used in CLI commands like corepack use pnpm@7.x (which then pins to an exact version in package.json).[1][2][5] Examples from official Node.js Corepack docs show ranges in CLI but resolve to exact versions: corepack use pnpm@7.x sets "packageManager": "pnpm@7.x.x.x" (exact).[2][5] Ranges like "*" or "^1.0.0" in the field itself are rejected, as confirmed in Corepack issues: the field expects a resolved version, not unresolved ranges.[3][4][6] It can include a hash suffix for integrity (e.g., pnpm@10.4.0+sha512....) or direct tarball URLs (with env flags), but still resolves to exact binaries.[7][8] This design ensures deterministic package manager versions across environments via Corepack, unlike dependency fields which use semver ranges.[9][10]
Citations:
- 1: https://stackoverflow.com/questions/71747609/how-to-specify-packagemanager-in-package-json
- 2: https://r2.nodejs.org/docs/v22.8.0/api/corepack.html
- 3: Non-version package specifiers don't work nodejs/corepack#313
- 4: Support for semver ranges? nodejs/corepack#95
- 5: https://r2.nodejs.org/docs/latest-iron/api/corepack.html
- 6: Allow packageManager to be specified by name alone nodejs/corepack#300
- 7: Loosen
packageManagerfield to support any package on npm nodejs/corepack#354 - 8: pnpm issues with corepack packageManager field pnpm/pnpm#7733
- 9:
packageManagerfield is too limited nodejs/corepack#402 - 10: https://github.com/nodejs/corepack/blob/490bf983/sources/types.ts
The packageManager field does not support >= range syntax; Corepack requires exact version specifiers.
The packageManager field in package.json follows the Corepack specification and only accepts exact version specifiers (e.g., pnpm@8.15.4), not version ranges like >=. Range syntax (>=, ^, *) is rejected by Corepack and will not resolve correctly. Change line 289 to use the exact version format: pkgJson.packageManager = ${pkgManager}@${stdout.trim()} instead of ${pkgManager}>=${stdout.trim()}.
🧰 Tools
🪛 GitHub Actions: Release / Deterministic Contract Tests
[error] Command failed: pnpm build && node ./scripts/build-current-binary.mjs && PROOFKIT_DISABLE_BUNDLED_BINARY=1 vitest run (followed by write-cli-version.mjs, tsdown, publint). vitest run exited with code 1 due to failing tests.
🤖 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 `@packages/cli/src/cli/init.ts` at line 289, The packageManager value is being
written with a range (`${pkgManager}>=${stdout.trim()}`) which Corepack rejects;
update the assignment for pkgJson.packageManager to use an exact specifier by
concatenating the package manager name and the exact version (use
`@${stdout.trim()}`), i.e. set pkgJson.packageManager to
`${pkgManager}@${stdout.trim()}` so Corepack can resolve it correctly; change
the assignment near where pkgJson, pkgManager, and stdout.trim() are used.
| expect(console.info.join("\n")).toContain( | ||
| "Warning: We strongly suggest using pnpm 11 or greater as your package manager for security reasons.", | ||
| ); |
There was a problem hiding this comment.
Use an intent-based warning assertion instead of exact copy
Line 368 is currently failing because warning text changed slightly. Match invariant intent (pnpm 11+ warning) rather than full sentence.
Proposed fix
- expect(console.info.join("\n")).toContain(
- "Warning: We strongly suggest using pnpm 11 or greater as your package manager for security reasons.",
- );
+ expect(console.info.join("\n")).toMatch(/warning:.*pnpm 11 or greater/i);📝 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.
| expect(console.info.join("\n")).toContain( | |
| "Warning: We strongly suggest using pnpm 11 or greater as your package manager for security reasons.", | |
| ); | |
| expect(console.info.join("\n")).toMatch(/warning:.*pnpm 11 or greater/i); |
🧰 Tools
🪛 GitHub Actions: Release / Deterministic Contract Tests
[error] Command failed: pnpm build && node ./scripts/build-current-binary.mjs && PROOFKIT_DISABLE_BUNDLED_BINARY=1 vitest run (followed by write-cli-version.mjs, tsdown, publint). vitest run exited with code 1 due to failing tests.
🪛 GitHub Check: Deterministic Contract Tests
[failure] 368-368: tests/executor.test.ts > executeInitPlan command paths > prints pnpm warning in npm next steps
AssertionError: expected 'Scaffolding in /tmp/proofkit-new-npm-…' to contain 'Warning: We strongly suggest using pn…'
- Expected
- Received
- Warning: We strongly suggest using pnpm 11 or greater as your package manager for security reasons.
- Scaffolding in /tmp/proofkit-new-npm-warning-8PPYq7/npm-app
- Next steps:
- Project root: cd npm-app
- Agent setup:
- Have your agent run this in the new project and complete the interactive prompt so it can load the right skills:
- npx
@tanstack/intent@latest install - Install dependencies:
- npm install
- Warning: We strongly suggest using PNPM 11 or greater as your package manager to better protect your computer and your app.
- Start the app:
- npm run dev
- More ProofKit commands:
- npm run proofkit
❯ tests/executor.test.ts:368:37
🤖 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 `@packages/cli/tests/executor.test.ts` around lines 368 - 370, The test's
exact-string assertion for the pnpm warning is brittle; update the assertion in
packages/cli/tests/executor.test.ts (the expect(console.info.join("\n")) line)
to check intent instead of exact copy—match a regex or use includes that looks
for the invariant parts like "pnpm" and "11" (e.g. a case-insensitive regex
matching "pnpm.*11" or "pnpm.*11 or greater") so the test passes even if
surrounding phrasing changes.
- write pnpm into `devEngines.packageManager` - drop top-level `packageManager` - update init warnings and tests
@proofkit/better-auth
@proofkit/cli
create-proofkit
@proofkit/fmdapi
@proofkit/fmodata
@proofkit/typegen
@proofkit/webviewer
commit: |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/cli/src/core/planInit.ts (1)
191-191:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the resolved execute runner for
@tanstack/intentin next steps.Line 191 is still hardcoded to
npx, which is inconsistent with the package-manager-aware init flow and can mislead pnpm/yarn/bun users.Suggested fix
- "npx `@tanstack/intent`@latest install", + `${packageManagerExecuteCommand} `@tanstack/intent`@latest install`,#!/bin/bash # Verify nextSteps currently hardcodes npx and that packageManagerExecuteCommand is available. rg -n "packageManagerExecuteCommand|tanstack/intent|nextSteps" packages/cli/src/core/planInit.ts -C2 sed -n '184,196p' packages/cli/src/core/planInit.ts🤖 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 `@packages/cli/src/core/planInit.ts` at line 191, nextSteps currently hardcodes "npx `@tanstack/intent`@latest install"; replace that element to use the resolved package manager command (packageManagerExecuteCommand) so the init flow respects pnpm/yarn/bun. Update the nextSteps entry that references "@tanstack/intent@latest install" to build the command using packageManagerExecuteCommand (same variable used earlier in this function) instead of the literal "npx", keeping the package spec "@tanstack/intent@latest install" intact.
🤖 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.
Duplicate comments:
In `@packages/cli/src/core/planInit.ts`:
- Line 191: nextSteps currently hardcodes "npx `@tanstack/intent`@latest install";
replace that element to use the resolved package manager command
(packageManagerExecuteCommand) so the init flow respects pnpm/yarn/bun. Update
the nextSteps entry that references "@tanstack/intent@latest install" to build
the command using packageManagerExecuteCommand (same variable used earlier in
this function) instead of the literal "npx", keeping the package spec
"@tanstack/intent@latest install" intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee519862-388f-4b43-9a43-ae4163ebb707
📒 Files selected for processing (9)
.changeset/package-manager-dev-engines.mdpackages/cli/src/cli/init.tspackages/cli/src/core/planInit.tspackages/cli/src/core/types.tspackages/cli/src/utils/sortPackageJson.tspackages/cli/tests/executor.test.tspackages/cli/tests/init-scaffold-contract.test.tspackages/cli/tests/integration.test.tspackages/cli/tests/planner.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/cli/src/utils/sortPackageJson.ts
- .changeset/package-manager-dev-engines.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/tests/planner.test.ts
- packages/cli/tests/executor.test.ts
- update init next step to use PM-specific exec - add tests for npm, pnpm, yarn, bun - include cli patch changeset
- print plan package manager execute command - cover pnpm agent setup path - set release binary targets for bun
1 participant
Summary
pnpmwhen init is launched vianpm, with a prompt fallback ifpnpmis missingnpmusers to move topnpm11+ in init next steps and scaffold outputTesting
resolve-init,planner,executor, and scaffold contract suitesSummary by CodeRabbit
New Features
Tests
Documentation