Repository navigation
fix(lockfile): resolve optional peers like pnpm 11 - #1793
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M5D1PN1G6J2PEWVHKN2Y0PP3 Entire-Checkpoint: 01M5D1XBC4VR1JDY0KPSYGFTXM Entire-Checkpoint: 01M5D1YHYRXQBGTXSR3A0PAX36
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
[Medium impact] The PR appears safe to merge; all previous findings are addressed and no new actionable issue remains. SummaryThis PR brings optional-peer resolution closer to pnpm 11.
All three previous findings are addressed. No new actionable issue was found. Reviews (2) · Last reviewed commit: "fix(lockfile): merge main and address op..." · Reviewed by Greptile |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M5D2YMQBASH10R41KZB08S1Q
Instruction countsNothing was compared, and so nothing was gated. No series appears on both sides: either the base has no measurements recorded, or the two were measured on different runner classes, which are deliberately not comparable — counts shift between machine types by more than a real regression does. New, nothing to compare against: 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.
|
… like pnpm 11 (#1805) <!-- entire-trail-link-start --> https://entire.com/gh/aubepkg/aube/trails/189 <!-- entire-trail-link-end --> Follow-up to #1793 for difference #2 of discussion #1779. aube-written `pnpm-lock.yaml` snapshots left out peers that pnpm 11 carries on a package because one of its *dependencies* needs them, so Dependabot (which runs pnpm) rewrote those keys. ## Problem pnpm 11 puts a peer resolved from above into every ancestor's snapshot key, even when the ancestor declares peers of its own. aube only did that for ancestors with no declared peers, and it also dropped a flat peer when an own peer's nested segment mentioned the same name. With `autoInstallPeers: false` and `@ai-sdk/react@4.0.138`, `ai@7.0.135` and `zod@^4` in one importer: ``` pnpm 11.27.1: @ai-sdk/react@4.0.138(react@19.3.1)(zod@4.6.7) aube before: @ai-sdk/react@4.0.138(react@19.3.1) ``` `zod` is only a peer of its child `@ai-sdk/mcp`. The same applies to `@testing-library/react@14.0.0(@types/react@…)(react-dom@…)(react@…)`, where `@types/react` comes from the child `@types/react-dom`. ## Changes - A package that declares peers now also takes the peers its dependencies resolved from above. - Edges to a package's own declared peers don't bubble their peers upward. - A peer only nested in another segment (`(ai@7(zod@4))`) is still listed flat when a descendant needs it. - `transitivePeerDependencies:` lists those inherited peers (`zod`, `@types/react`), as pnpm does. Existing aube-written lockfiles shift once on the next resolve for affected packages. Lockfiles are read as-is, so frozen installs keep working. ## Validation Fresh `pnpm@11.27.1 install --lockfile-only` vs `aube install --lockfile-only`, `snapshots:` identical on four small fixtures (descendant required peer, descendant `@types/react`, older `@ai-sdk/react`, optional-peer variant). On the speakeasy-api/gram repro the `@ai-sdk/*` and `@assistant-ui/*` descendant-peer diffs are gone. Still open there: optional peers hoisted into a nested peer's scope (`@assistant-ui/react(...)(ai)(immer)(use-sync-external-store)`), `ajv-formats`/`@inquirer/*` optional-peer handling, and resolution picks like `@babel/runtime`. The bats expectation in `peer_deps.bats` now includes `@types/react`, which pnpm 11.27.1 writes for that fixture. Full bats suite passes except one test needing `bun`, which isn't installed here. *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 --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Improved peer dependency resolution for nested dependency paths. * Correctly propagates peer requirements through packages that declare peers and their ancestors. * Prevents duplicate, self-referencing, and incorrectly inherited peer dependencies. * Preserves transitive peer dependency information during resolution. * Ensures references use consistent resolved paths after peer-context changes. * Applies consistent handling for long inherited peer dependency paths. * Updated lockfile validation for pnpm 11 dependency metadata changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
aube-written
pnpm-lock.yamlfiles 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-onlyon small fixtures to establish the rules, and the aube output now matches pnpm'ssnapshots:byte for byte on all of them (and on the importer rows):dedupePeerDependentsis on, the default). If a satisfying version is already resolved anywhere in the workspace, even in an unrelated importer's subtree, pnpm binds it. WithautoInstallPeers: false,vite@7.1.0in one importer gets(jiti@2.7.0)when only another importer'sc12pulls in jiti. aube only looked at ancestors, the package's own deps and the workspace root. This also covers debug'ssupports-color, which pnpm 11 binds whenchalkbrings it in.optionalDependencies:in the snapshot, notdependencies:.aresolving vite withjitiand importerbwithyamlboth end up asvite@7.1.0(jiti@2.5.1)(yaml@2.8.1). Variants whose peers conflict (differentjitiversions) stay separate.Before (importer
apps/webandapps/othershare vite, onlywebhas jiti and yaml):After (same as pnpm 11):
With
dedupePeerDependents: falsepnpm 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), wherezodis 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
snapshots:againstpnpm@11.27.1on 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).optional_peer_is_not_bound_via_graph_scanasserted the opposite of pnpm 11's behavior (which I checked: pnpm 11.27.1 bindssupports-color@7.2.0fordebugwhenchalk@4is 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; batspeer_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
Summary by CodeRabbit