mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-21 05:02:30 -04:00
b53a50f037
## Summary Lifts the previous `UnsupportedPeerDependency` upfront guard and replaces it with the real peer check from upstream's `getNodeHoistInfo`. The hoister now accepts lockfiles whose packages declare peer dependencies and produces the layout pnpm would: a peer-constrained dep only floats up to the root when doing so doesn't change which package its peers resolve to. ## How it works - `HoisterResult` carries the input `peer_names` set forward. Upstream's `HoisterResult` doesn't (peer info lives on its intermediate `HoisterWorkTree`); pacquet runs the algorithm on `HoisterResult` directly, so the peer set rides along. - The hoist driver is a recursive `hoist_subtree` that walks the result graph depth-first. Each recursion receives the candidate parent's *current* ancestor path (`Vec<Rc<HoisterResult>>` exclusive of the parent). After a child's hoist decision is applied, the path passed into the child's recursion reflects its *new* position — a freshly-hoisted child recurses with `[root]`, a child that stayed nested recurses with `parent_path + [parent]`. (An earlier version of this PR used BFS with paths captured at queue time; that path went stale whenever a node was hoisted between being queued and dequeued, leading to over-refused hoists. The recursive form was Copilot's recommendation in code review; the bug regression is pinned by `peer_check_uses_post_hoist_ancestor_path_not_queue_time_path`.) - `would_shadow_peer` walks that path to decide whether a candidate hoist would change its own peer resolution. - New `AbsorbDecision::PeerShadow` variant alongside `Free` / `SameNode` / `Conflict`. Fires when: 1. Root declares the candidate's own name as a peer (root-shadow guard — vacuous today since the wrapper's `.` root has empty `peer_names`, kept for parity). 2. For each peer name `P` the candidate declares, the closest ancestor providing `P` does so with a different ident than the root's `P`. Promoting the candidate would silently re-resolve the peer. - The previous `UnsupportedPeerDependency` error variant and `find_first_peer_constrained` upfront scan are gone. The `#[non_exhaustive]` tag stays so future variants can be added without breaking callers. ## DAG-vs-tree caveat Upstream's `cloneTree` duplicates the work tree into a strict tree per parent path, so each candidate visit has a unique ancestor chain. Pacquet preserves the DAG (its identity-dedup is a feature) and records only the path DFS actually used to reach the candidate. In the rare case where the same `Rc` is reachable through both a peer-compatible and a peer-shadowing path, pacquet refuses to hoist — the layout ends up at most over-nested, never under-nested. The trade-off is documented on `would_shadow_peer`. ## Upstream reference - [`hoist.ts:414`](4287909fa6/packages/yarnpkg-nm/sources/hoist.ts (L414)) — root-shadow guard. - [`hoist.ts:454-479`](4287909fa6/packages/yarnpkg-nm/sources/hoist.ts (L454-L479)) — ancestor-path peer check. - [`hoist.ts:670`](4287909fa6/packages/yarnpkg-nm/sources/hoist.ts (L670)) — `cloneTree`, the per-path-tree shape pacquet skips. ## Test plan - [x] `peer_constrained_node_stays_under_parent_when_root_provides_different_ident` — `app → widget (peer: react) + react@17`, `root → react@18`. widget cannot hoist because hoisting would change its peer to react@18. - [x] `peer_constrained_node_hoists_when_ancestor_and_root_agree` — same shape but `app → react@18` matches root's `react@18` (shared `Rc` via wrapper dedup), so widget hoists freely. - [x] `peer_check_uses_post_hoist_ancestor_path_not_queue_time_path` — regression for the BFS-stale-path bug Copilot caught. `root → app → mid → terminal` (peer: `react`) where `mid` hoists past a conflicting `app.react@17`. Under the previous BFS the stale path `[root, app, mid]` made `terminal`'s peer check trip on `app.react@17 ≠ root.react@18` and refuse the hoist; under DFS the actual post-hoist path `[root, mid]` finds no provider mismatch and `terminal` correctly flattens to root. - [x] All 10 pre-existing tests still pass (`one_transitive_dep_hoists_to_root`, `diamond_dep_hoists_once_to_root`, `version_conflict_keeps_loser_at_parent`, `deep_chain_flattens_in_one_pass`, `transitive_npm_alias_resolves_target_snapshot`, `non_empty_hoisting_limits_surfaces_unsupported`, `multi_importer_lockfile_surfaces_unsupported_workspace`, `external_dependencies_are_stripped_from_the_result`, `empty_lockfile_yields_empty_root`, `hoist_throws_on_broken_lockfile`). - [x] The previously-failing `peer_dependency_in_lockfile_surfaces_unsupported` test from #452 is replaced (peers are no longer an unsupported input). - [x] All Copilot review threads addressed and resolved (stale ancestor path, misleading comment). - [x] `just ready` (836 tests pass), `cargo doc --document-private-items`, `taplo format --check`, `just dylint` all clean.