Repository navigation
fix(lockfile): sort peer suffix by full name@version like pnpm - #1792
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M5CZSMB9Z2J1SNP1D9ZQZE62
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 19 minutes. View limit details
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 |
Contributor
 [Medium impact] The PR appears safe to merge. SummaryChanges peer suffixes to sort by the full
Reviews (1) · Last reviewed commit: "fix(lockfile): sort peer suffix by full ..." · Reviewed by Greptile |
jdx
added a commit
that referenced
this pull request
Oct 20, 2026
aube-written `pnpm-lock.yaml` files leave out optional peers that pnpm 11 resolves, so Dependabot (which runs pnpm) rewrites snapshot keys it did not need to touch. This is difference #2 of discussion #1779. Difference #1 (suffix order) is in #1792 and #3 (unused catalog entries) in #1788. I ran `pnpm@11.27.1 install --lockfile-only` on small fixtures to establish the rules, and the aube output now matches pnpm's `snapshots:` byte for byte on all of them (and on the importer rows): 1. **Optional peers resolve from anywhere in the graph (when `dedupePeerDependents` is on, the default).** If a satisfying version is already resolved anywhere in the workspace, even in an unrelated importer's subtree, pnpm binds it. With `autoInstallPeers: false`, `vite@7.1.0` in one importer gets `(jiti@2.7.0)` when only another importer's `c12` pulls in jiti. aube only looked at ancestors, the package's own deps and the workspace root. This also covers debug's `supports-color`, which pnpm 11 binds when `chalk` brings it in. 2. **A resolved optional peer is listed under `optionalDependencies:`** in the snapshot, not `dependencies:`. 3. **Compatible variants of the same package merge across importers.** Importer `a` resolving vite with `jiti` and importer `b` with `yaml` both end up as `vite@7.1.0(jiti@2.5.1)(yaml@2.8.1)`. Variants whose peers conflict (different `jiti` versions) stay separate. Before (importer `apps/web` and `apps/other` share vite, only `web` has jiti and yaml): ``` apps/other: vite 7.1.0(@types/node@24.3.0)(tsx@4.20.5) apps/web: vite 7.1.0(@types/node@24.3.0)(jiti@2.5.1)(tsx@4.20.5)(yaml@2.8.1) ``` After (same as pnpm 11): ``` apps/other: vite 7.1.0(@types/node@24.3.0)(jiti@2.5.1)(tsx@4.20.5)(yaml@2.8.1) apps/web: vite 7.1.0(@types/node@24.3.0)(jiti@2.5.1)(tsx@4.20.5)(yaml@2.8.1) ``` With `dedupePeerDependents: false` pnpm 11 leaves those peers unresolved, and so does aube. ## Migration Existing aube-written lockfiles shift once on the next resolve for any package whose optional peers are now resolved. Lockfiles are read as-is, so frozen installs of old lockfiles keep working. ## Not covered yet Running both tools fresh on the repro repo (speakeasy-api/gram at e537882348) still leaves differences. They come from optional peers a package gets only because a *descendant* needs them, e.g. `@ai-sdk/react@4.0.138(react@19.3.0)(zod@4.6.7)`, where `zod` is only a peer of its child `@ai-sdk/mcp`, and `@testing-library/react@14.0.0(@types/react@…)`. pnpm hoists those resolved optional peers into the importer's scope so every package below sees them; aube's ancestor-propagation gate does not. I did not change that because I could not pin the rule down from output alone without risking regressions. This is a follow-up. ## Validation - Compared `snapshots:` against `pnpm@11.27.1` on fixtures covering: optional peer provided by another importer's subtree, by the root, by a cousin dep, version conflicts across importers, three importers with partial overlap, dedupePeerDependents off, and required peers across importers (already matching). - The old unit test `optional_peer_is_not_bound_via_graph_scan` asserted the opposite of pnpm 11's behavior (which I checked: pnpm 11.27.1 binds `supports-color@7.2.0` for `debug` when `chalk@4` is in the graph). It is replaced by tests for the new behavior, plus a variant-merge test. - `cargo test`, `cargo clippy --all-targets -- -D warnings`, `cargo fmt`; bats `peer_deps`, `peers`, `install`, `update`, `prune`, `optional_platform`, `pnpm_monorepo_dedupePeers`. *AI-assisted — Tool: Claude Code; model: anthropic/claude-sonnet-5-5; version: unavailable.* 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved optional peer dependency resolution across the dependency graph when deduplication is enabled. * Compatible optional peers are now consolidated across variants, while conflicting versions remain separate. * Resolved optional peers are recorded correctly, and unresolved peers continue to be reported when no satisfying version is available. * Clarified peer dependency resolution behavior in relevant output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
aube-written
pnpm-lock.yamlfiles order a package's peer suffix differently from pnpm 11, so tools that regenerate the lockfile with pnpm (e.g. Dependabot) rewrite lines that did not really change. This is difference #1 of discussion #1779.pnpm sorts the peer suffix by the full
name@version(...)string, soreact-dom@…comes beforereact@…(-sorts before@). aube sorted by peer name only.Before:
After (same as pnpm 11.27.1 for this package, apart from optional peers, which are a separate follow-up):
The issue's example
4.7.0(monaco-editor@0.57.0)(react-dom@19.3.0(react@19.3.0))(react@19.3.0)is now covered by a unit test.Migration
Existing aube-written lockfiles that contain a package with both a
reactand areact-dom-style peer pair (any suffix where a name is a--prefixed extension of another) shift once on the next resolve. Only the key order changes. Lockfiles read as-is: keys are not re-sorted on read, so a frozen install of an old-order lockfile keeps working, and pnpm accepts either order.Scope
Only the dep_path suffix order changes (the per-package suffix and the cumulative suffix rewrite). Scope propagation and the fixed-point loop are unchanged.
Validation
pnpm@11.27.1 install --lockfile-onlyonreact+react-dom+@testing-library/react@14.0.0.cargo test,cargo clippy --all-targets -- -D warnings,cargo fmtpass; new testsuffix_orders_peers_by_full_name_at_version_string.peer_deps.bats,peers.batsandpnpm_monorepo_dedupePeers.bats.AI-assisted — Tool: Claude Code; model: anthropic/claude-sonnet-5-5; version: unavailable.
🤖 Generated with Claude Code