feat: support pnpm catalogs#1397
Conversation
📝 WalkthroughWalkthroughThe Nuxt CLI now supports pnpm catalogs by reading workspace catalog definitions, resolving catalog-managed dependency versions through the npm registry, and updating catalog entries during upgrades. The Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/nuxt-cli/test/unit/commands/upgrade.spec.ts (1)
64-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for
resolveCatalogSpecifier.It decides the exact specifier string written into
pnpm-workspace.yaml(plain^versionvsnpm:target@^versionfor aliases), which is core to the catalog-write path. MockingresolveRegistryVersion(e.g. viavi.mock) would let this be tested without a real network call, alongside the existingparseInstallSpecsuite.🤖 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/nuxt-cli/test/unit/commands/upgrade.spec.ts` around lines 64 - 90, Add unit coverage for resolveCatalogSpecifier alongside the existing parseInstallSpec suite, mocking resolveRegistryVersion to avoid network access. Assert that regular packages produce ^version and aliased packages produce npm:target@^version, covering both catalog-write specifier formats.packages/nuxt-cli/src/utils/catalog.ts (1)
86-95: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
resolveCatalogEntryre-reads and re-parsespnpm-workspace.yamlon every call.
readCatalogConfigre-runsfindPnpmWorkspaceYaml+readFileSync+ full YAML parse each time it's invoked. Across a single command this adds up:upgrade.tscallsresolveCatalogEntryonce per package name in its prep loop,versions.ts'sgetNuxtVersioncalls it separately, andinfo.ts'sgetDepVersioncalls it once per listed module — so a singlenuxi upgrade/nuxi inforun can re-parse the same file many times.Consider caching the parsed
CatalogConfigpercwd(or accepting an already-parsed config) so repeated lookups within one command invocation don't re-hit the filesystem.🤖 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/nuxt-cli/src/utils/catalog.ts` around lines 86 - 95, Update resolveCatalogEntry and the readCatalogConfig path to cache the parsed CatalogConfig per cwd, reusing the cached result for repeated lookups during a command invocation instead of repeatedly finding, reading, and parsing pnpm-workspace.yaml. Preserve the existing catalog and specifier resolution behavior, including undefined results when no catalog configuration exists.packages/nuxt-cli/test/unit/utils/catalog.spec.ts (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo direct test coverage for
setCatalogEntry.This is the only exported function that mutates
pnpm-workspace.yaml, and it's whatupgrade.tsrelies on to persist resolved catalog versions. Given the existing temp-dir fixture pattern here, a test writing a workspace file, callingsetCatalogEntry, and asserting the on-disk content (plus thehasChanged()-driven boolean return) would be low effort and directly protect this write path.🤖 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/nuxt-cli/test/unit/utils/catalog.spec.ts` at line 7, Add direct unit coverage for the exported setCatalogEntry function in catalog.spec.ts. Follow the existing temporary-directory fixture pattern to create a pnpm-workspace.yaml, invoke setCatalogEntry, and assert both the persisted on-disk catalog content and the boolean result driven by hasChanged(), including unchanged and updated cases as appropriate.
🤖 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/nuxt-cli/src/commands/upgrade.ts`:
- Around line 257-278: Use the boolean result returned by setCatalogEntry in the
catalogUpdates loop before adding an entry to updated. Only push the package
specifier when the catalog write succeeds; otherwise leave it out of the success
summary and preserve the existing unresolved handling for resolution failures.
In `@packages/nuxt-cli/src/utils/catalog.ts`:
- Around line 101-115: Update setCatalogEntry to wrap readFileSync and
parsePnpmWorkspaceYaml in the same try/catch pattern used by readCatalogConfig,
returning false when reading or parsing fails. Keep the existing file-not-found,
unchanged-workspace, write, and success behavior intact.
In `@packages/nuxt-cli/src/utils/versions.ts`:
- Around line 37-52: Update resolveRegistryVersion to pass a 10-second timeout
in the $fetch options, and replace the order-dependent findLast selection with
verkit.findMaxSatisfying over the packument version keys for semver-aware
highest-version resolution. Preserve the dist-tags lookup as the first-choice
result.
---
Nitpick comments:
In `@packages/nuxt-cli/src/utils/catalog.ts`:
- Around line 86-95: Update resolveCatalogEntry and the readCatalogConfig path
to cache the parsed CatalogConfig per cwd, reusing the cached result for
repeated lookups during a command invocation instead of repeatedly finding,
reading, and parsing pnpm-workspace.yaml. Preserve the existing catalog and
specifier resolution behavior, including undefined results when no catalog
configuration exists.
In `@packages/nuxt-cli/test/unit/commands/upgrade.spec.ts`:
- Around line 64-90: Add unit coverage for resolveCatalogSpecifier alongside the
existing parseInstallSpec suite, mocking resolveRegistryVersion to avoid network
access. Assert that regular packages produce ^version and aliased packages
produce npm:target@^version, covering both catalog-write specifier formats.
In `@packages/nuxt-cli/test/unit/utils/catalog.spec.ts`:
- Line 7: Add direct unit coverage for the exported setCatalogEntry function in
catalog.spec.ts. Follow the existing temporary-directory fixture pattern to
create a pnpm-workspace.yaml, invoke setCatalogEntry, and assert both the
persisted on-disk catalog content and the boolean result driven by hasChanged(),
including unchanged and updated cases as appropriate.
🪄 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: CHILL
Plan: Pro Plus
Run ID: f98b69cc-f46e-403d-99af-d8b60f94ed32
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/nuxt-cli/package.jsonpackages/nuxt-cli/src/commands/info.tspackages/nuxt-cli/src/commands/upgrade.tspackages/nuxt-cli/src/utils/catalog.tspackages/nuxt-cli/src/utils/versions.tspackages/nuxt-cli/test/unit/commands/upgrade.spec.tspackages/nuxt-cli/test/unit/utils/catalog.spec.tspackages/nuxt-cli/test/unit/utils/versions.spec.ts
a981612 to
1b01417
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/nuxt-cli/src/utils/catalog.ts`:
- Around line 106-114: Update resolveCatalogEntry to resolve the package
specifier across dependencies, devDependencies, optionalDependencies, and
peerDependencies, preserving the existing precedence and catalog lookup behavior
so catalog-managed packages in any supported section use the workspace catalog
value.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 1d088525-f2a7-4536-ab6b-8aa2465cbe0f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/nuxt-cli/package.jsonpackages/nuxt-cli/src/commands/info.tspackages/nuxt-cli/src/commands/upgrade.tspackages/nuxt-cli/src/utils/catalog.tspackages/nuxt-cli/src/utils/versions.tspackages/nuxt-cli/test/unit/commands/upgrade.spec.tspackages/nuxt-cli/test/unit/utils/catalog.spec.tspackages/nuxt-cli/test/unit/utils/versions.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/nuxt-cli/test/unit/commands/upgrade.spec.ts
- packages/nuxt-cli/src/utils/versions.ts
- packages/nuxt-cli/src/commands/info.ts
- packages/nuxt-cli/src/commands/upgrade.ts
| export function resolveCatalogEntry(cwd: string, pkgJson: PackageJson | null | undefined, pkg: string): CatalogEntry | undefined { | ||
| const specifier = pkgJson?.dependencies?.[pkg] || pkgJson?.devDependencies?.[pkg] | ||
| const catalog = parseCatalogSpecifier(specifier) | ||
| if (!catalog) { | ||
| return undefined | ||
| } | ||
|
|
||
| const config = readCatalogConfig(cwd) | ||
| return { catalog, specifier: config?.catalogs[catalog]?.[pkg] } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 --glob '*.ts' \
'resolveCatalogEntry\(|optionalDependencies|peerDependencies' \
packages/nuxt-cli/src packages/nuxt-cli/testRepository: nuxt/cli
Length of output: 14300
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- packages/nuxt-cli/src/utils/catalog.ts outline ---\n'
ast-grep outline packages/nuxt-cli/src/utils/catalog.ts --view expanded || true
printf '\n--- packages/nuxt-cli/src/utils/catalog.ts relevant lines ---\n'
sed -n '1,150p' packages/nuxt-cli/src/utils/catalog.ts | cat -n
printf '\n--- packages/nuxt-cli/src/utils/versions.ts ---\n'
sed -n '1,60p' packages/nuxt-cli/src/utils/versions.ts | cat -n
printf '\n--- pnpm catalog references in package manifests ---\n'
rg -n '"catalog:("' -g 'package.json' .Repository: nuxt/cli
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- packages/nuxt-cli/src/utils/catalog.ts outline ---'
ast-grep outline packages/nuxt-cli/src/utils/catalog.ts --view expanded || true
printf '%s\n' ''
printf '%s\n' '--- packages/nuxt-cli/src/utils/catalog.ts relevant lines ---'
sed -n '1,150p' packages/nuxt-cli/src/utils/catalog.ts | cat -n
printf '%s\n' ''
printf '%s\n' '--- packages/nuxt-cli/src/utils/versions.ts ---'
sed -n '1,60p' packages/nuxt-cli/src/utils/versions.ts | cat -n
printf '%s\n' ''
printf '%s\n' '--- pnpm catalog references in package manifests ---'
rg -n '"catalog:("' -g 'package.json' . || trueRepository: nuxt/cli
Length of output: 10135
🌐 Web query:
pnpm catalog references dependency sections dependencies devDependencies peerDependencies optionalDependencies workspace.yaml
💡 Result:
In pnpm, catalogs are a workspace feature that allows you to define dependency version ranges centrally in your pnpm-workspace.yaml file and reference them across your workspace packages [1][2]. You can use the catalog: protocol to reference these versions in the following dependency sections within any package.json file in your workspace: - dependencies [3] - devDependencies [3] - peerDependencies [3][4] - optionalDependencies [3] Additionally, the catalog: protocol can be used within the overrides field inside your pnpm-workspace.yaml file [5][3]. How it works: 1. Define your catalogs in pnpm-workspace.yaml using either the singular catalog field (for the default catalog) or the plural catalogs field (for named catalogs) [1][2]. 2. Reference the catalog version in a package.json file using the catalog: protocol [1][3]. - For the default catalog, you can use the shorthand catalog: [1][6]. - For named catalogs, use the syntax catalog:<catalog_name> (e.g., catalog:react18) [1][6]. Example: In pnpm-workspace.yaml: catalog: react: ^18.3.1 catalogs: testing: vitest: ^2.0.0 In a package.json: { "dependencies": { "react": "catalog:" }, "devDependencies": { "vitest": "catalog:testing" } } You can also add dependencies to catalogs directly via the CLI using pnpm add --save-catalog or pnpm add --save-catalog-name <catalog_name> [7].
Citations:
- 1: https://pnpm.io/catalogs
- 2: https://pnpm.io/10.x/catalogs
- 3: https://github.com/antfu/skills/blob/a74f281a27dadc02397bc1a174b0f2c97531b6ae/skills/pnpm/references/features-catalogs.md
- 4: https://pnpm.io/package_json
- 5: https://pnpm.io/settings
- 6: https://github.com/pnpm/rfcs/blob/main/text/0001-catalogs.md
- 7: https://pnpm.io/cli/add
Resolve catalog references from all dependency sections.
resolveCatalogEntry() currently ignores optionalDependencies and peerDependencies, but pnpm catalogs are valid in those sections too. This can make catalog-managed packages bypass the workspace catalog value, including in the Nuxt version fallback.
Proposed fix
- const specifier = pkgJson?.dependencies?.[pkg] || pkgJson?.devDependencies?.[pkg]
+ const specifier = pkgJson?.dependencies?.[pkg]
+ ?? pkgJson?.devDependencies?.[pkg]
+ ?? pkgJson?.optionalDependencies?.[pkg]
+ ?? pkgJson?.peerDependencies?.[pkg]📝 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.
| export function resolveCatalogEntry(cwd: string, pkgJson: PackageJson | null | undefined, pkg: string): CatalogEntry | undefined { | |
| const specifier = pkgJson?.dependencies?.[pkg] || pkgJson?.devDependencies?.[pkg] | |
| const catalog = parseCatalogSpecifier(specifier) | |
| if (!catalog) { | |
| return undefined | |
| } | |
| const config = readCatalogConfig(cwd) | |
| return { catalog, specifier: config?.catalogs[catalog]?.[pkg] } | |
| export function resolveCatalogEntry(cwd: string, pkgJson: PackageJson | null | undefined, pkg: string): CatalogEntry | undefined { | |
| const specifier = pkgJson?.dependencies?.[pkg] | |
| ?? pkgJson?.devDependencies?.[pkg] | |
| ?? pkgJson?.optionalDependencies?.[pkg] | |
| ?? pkgJson?.peerDependencies?.[pkg] | |
| const catalog = parseCatalogSpecifier(specifier) | |
| if (!catalog) { | |
| return undefined | |
| } | |
| const config = readCatalogConfig(cwd) | |
| return { catalog, specifier: config?.catalogs[catalog]?.[pkg] } |
🤖 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/nuxt-cli/src/utils/catalog.ts` around lines 106 - 114, Update
resolveCatalogEntry to resolve the package specifier across dependencies,
devDependencies, optionalDependencies, and peerDependencies, preserving the
existing precedence and catalog lookup behavior so catalog-managed packages in
any supported section use the workspace catalog value.
0deaf68 to
adcefef
Compare
1 participant
🔗 Linked issue
resolves #866
📚 Description
this follows path taken by https://github.com/antfu-collective/ni in resolving/handling pnpm catalogs