fix(pacquet): record catalog snapshot for npm-aliased catalog deps by zkochan · Pull Request #12274 · pnpm/pnpm

zkochan · 2026-06-08T21:54:37Z

PR Summary by Qodo

Fix catalog snapshots for npm-aliased catalog dependencies
🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Walkthroughs

User Description

What

Part of the lockfile-parity work (#12266, category 2 — npm: aliases). Fixes a catalog entry resolved through an npm: alias being dropped from the lockfile's catalogs: section.

build_catalog_snapshots read the resolved version via ImporterDepVersion::as_regular, which returns None for npm:-aliased deps (stored as ImporterDepVersion::Alias). So an entry like js-yaml: npm:@zkochan/js-yaml@0.0.11 was silently dropped — pnpm records { specifier: npm:@zkochan/js-yaml@0.0.11, version: 0.0.11 }.

How

Read the version via ImporterDepVersion::ver_peer, which returns the alias's suffix (0.0.11) for Alias and the plain version for Regular.

Verification

A single-entry aliased catalog (js-yaml: npm:@zkochan/js-yaml@0.0.11) is now byte-identical to pnpm 11.5.2.
On the pnpm monorepo, all 8 previously-dropped aliased catalog entries are restored (lockfile diff vs fresh pnpm: 2,191 → 2,167).
New regression test aliased_catalog_dependency_records_catalog_snapshot, verified to fail without the fix.
cargo nextest (graph-to-lockfile + catalog suites) green; clippy + fmt clean.

Note

While investigating category 2, I found the remaining override-related symptoms (catalog:-valued overrides, transitively-overridden js-yaml) are not an npm: alias problem — pacquet doesn't apply pnpm.overrides during fresh resolution at all. Filed separately so this PR stays scoped.

Refs #12266.

Written by an agent (Claude Code, claude-opus-4-8).

AI Description

• Preserve catalogs: lockfile entries for catalog: deps resolved via npm: aliases.
• Use ImporterDepVersion::ver_peer to read resolved versions for alias and regular deps.
• Add a regression test ensuring aliased catalog entries record { specifier, version }.

Diagram

graph TD
  A["dependencies_graph_to_lockfile.rs"] --> B["build_catalog_snapshots()"] --> E[("Lockfile catalogs snapshot")]
  B --> F[("Catalogs input map")]
  B --> C[("ProjectSnapshot deps")]
  C --> D["ImporterDepVersion::ver_peer()"] --> E
  T["tests.rs: aliased catalog test"] --> A

High-Level Assessment

The chosen fix is appropriate: ImporterDepVersion::ver_peer is the API that intentionally abstracts over Regular vs Alias version extraction, preventing silent drops that occur with as_regular. Adding a targeted regression test is sufficient to guard the parity behavior.

File Changes

Bug fix (1)

dependencies_graph_to_lockfile.rs Use 'ver_peer' to retain npm-aliased catalog snapshot versions +7/-1
Use 'ver_peer' to retain npm-aliased catalog snapshot versions
• Updates catalog snapshot version extraction to use 'ImporterDepVersion::ver_peer()' so 'npm:'-aliased dependencies (stored as 'Alias') still contribute a resolved version. Expands the function comment to document the alias-drop failure mode and why 'ver_peer' matches pnpm behavior.
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs

Tests (1)

tests.rs Add regression test for aliased 'catalog:' dependency snapshot entry +64/-0
Add regression test for aliased 'catalog:' dependency snapshot entry
• Introduces 'aliased_catalog_dependency_records_catalog_snapshot', constructing a minimal graph + catalogs map where 'js-yaml' is provided via an 'npm:' alias. Asserts the lockfile 'catalogs:' snapshot records both the aliased specifier and the resolved version string.
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs

Summary by CodeRabbit

Release Notes

Bug Fixes
- Fixed an issue where certain catalog dependencies were being silently omitted from lockfile snapshots. These entries are now properly recorded.
Tests
- Added comprehensive test coverage for catalog dependencies using npm aliases to ensure they're correctly included in lockfiles.
Documentation
- Updated test porting documentation to reflect newly ported test cases.

… deps `build_catalog_snapshots` read the resolved version via `ImporterDepVersion::as_regular`, which returns `None` for `npm:`-aliased deps (stored as `ImporterDepVersion::Alias`). So a catalog entry like `js-yaml: npm:@zkochan/js-yaml@0.0.11` was silently dropped from the `catalogs:` lockfile section — pnpm records `{ specifier: npm:@zkochan/js-yaml@0.0.11, version: 0.0.11 }`. Read the version via `ver_peer` instead, which returns the alias's suffix (`0.0.11`) for `Alias` and the plain version for `Regular`. On the pnpm monorepo this restores all 8 dropped aliased catalog entries (lockfile diff vs fresh pnpm 2,191 -> 2,167); a single-entry catalog now matches pnpm byte-for-byte. Refs #12266.

qodo-free-for-open-source-projects · 2026-06-08T21:54:44Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Previous review results

Review updated until commit e2c1a8d

Results up to commit 261daee

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

coderabbitai · 2026-06-08T21:54:51Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ca4657d8-7047-4a9f-b68b-7621f1a7d8e3

📥 Commits

Reviewing files that changed from the base of the PR and between 261daee and e2c1a8d.

📒 Files selected for processing (1)

pacquet/plans/TEST_PORTING.md

✅ Files skipped from review due to trivial changes (1)

pacquet/plans/TEST_PORTING.md

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: ubuntu-latest / Node.js 24 / Test
GitHub Check: Run benchmark on ubuntu-latest
GitHub Check: Code Coverage
GitHub Check: Lint and Test (windows-latest)
GitHub Check: Lint and Test (macos-latest)
GitHub Check: Lint and Test (ubuntu-latest)

📝 Walkthrough

Walkthrough

This PR fixes catalog snapshot generation for dependencies resolved through npm aliases. The change updates importer_resolved_version to extract versions using ver_peer() instead of as_regular(), preventing aliased catalog entries from being silently omitted. A new test validates that catalog dependencies declared via npm aliases are correctly recorded in the lockfile's catalogs snapshot.

Changes

Catalog Snapshot Fix for npm-Aliased Dependencies

Layer / File(s)	Summary
Fix importer_resolved_version to use ver_peer() `pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs`	Changed `importer_resolved_version` to use `ImporterDepVersion::ver_peer()` instead of `as_regular()` to properly extract resolved versions for catalog snapshot generation when dependencies are resolved through npm aliases.
Test aliased catalog dependency recording `pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs`	Added `aliased_catalog_dependency_records_catalog_snapshot` test that verifies a `catalog:`-resolved dependency via `npm:` alias (e.g., `js-yaml` → `npm:`@zkochan/js-yaml`@0.0.11`) is recorded in `lockfile.catalogs` with correct `specifier` and `version` values.
Docs: mark test ported `pacquet/plans/TEST_PORTING.md`	Marked the pnpm “catalog entry using npm alias can be reused” test case as ported and adjusted surrounding checklist placement/wrapping.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

pacquet: lockfile-parity gaps vs pnpm CLI (npm: aliases, minimumReleaseAge, peer suffixes) #12266: The change directly addresses the npm-alias-in-catalogs gap described in this issue by ensuring aliased catalog entries record versions.

Poem

🐰 A version hops through alias gates,
ver_peer() now resonates,
No more catalogs left behind,
When npm aliases unwind! 📦✨

🚥 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 PR title precisely describes the main change: fixing a bug where catalog snapshots for npm-aliased catalog dependencies were not recorded in the lockfile.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch pacquet-npm-aliases

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.}

github-actions · 2026-06-08T22:00:00Z

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.01      7.6±0.20ms   568.4 KB/sec    1.00      7.6±0.24ms   571.7 KB/sec

Notes that catalogs.ts:789 "catalog entry using npm alias can be reused" is covered: its `catalogs:` snapshot assertion by the new `aliased_catalog_dependency_records_catalog_snapshot` unit test, its reuse half by the existing single-importer reuse test (the two-project shape needs workspaces). Also flags the still-unported general catalogs.ts install/workspace integration cases. Refs #12266.

qodo-free-for-open-source-projects · 2026-06-08T22:17:21Z

Code review by qodo was updated up to the latest commit e2c1a8d

This was referenced Jun 8, 2026

pacquet: pnpm.overrides not applied during fresh-lockfile resolution #12275

Closed

pacquet: lockfile-parity gaps vs pnpm CLI (npm: aliases, minimumReleaseAge, peer suffixes) #12266

Closed

coderabbitai Bot approved these changes Jun 8, 2026

View reviewed changes

zkochan merged commit 55a4035 into main Jun 8, 2026
27 of 28 checks passed

zkochan deleted the pacquet-npm-aliases branch June 8, 2026 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(pacquet): record catalog snapshot for npm-aliased catalog deps#12274

fix(pacquet): record catalog snapshot for npm-aliased catalog deps#12274
zkochan merged 2 commits into
mainfrom
pacquet-npm-aliases

zkochan commented Jun 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 8, 2026 •

edited

Loading

Great, no issues found!

Uh oh!

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

Uh oh!

github-actions Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

zkochan commented Jun 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Walkthroughs

What

How

Verification

Note

File Changes

Summary by CodeRabbit

Release Notes

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Great, no issues found!

Previous review results

Great, no issues found!

Uh oh!

coderabbitai Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

Uh oh!

github-actions Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Micro-Benchmark Results

Linux

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zkochan commented Jun 8, 2026 •

edited by coderabbitai Bot

Loading

qodo-free-for-open-source-projects Bot commented Jun 8, 2026 •

edited

Loading

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

github-actions Bot commented Jun 8, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 8, 2026 •

edited

Loading