Skip to content

fix: resolve missing peer from versions already present in other parts of the dep graph#7812

Merged
zkochan merged 7 commits into
mainfrom
auto-install-peers-from-hoisted-deps
Mar 22, 2024
Merged

fix: resolve missing peer from versions already present in other parts of the dep graph#7812
zkochan merged 7 commits into
mainfrom
auto-install-peers-from-hoisted-deps

Conversation

@zkochan

@zkochan zkochan commented Mar 20, 2024

Copy link
Copy Markdown
Member

No description provided.

@zkochan zkochan force-pushed the auto-install-peers-from-hoisted-deps branch from 6a121c7 to cbb46eb Compare March 22, 2024 14:28
@zkochan zkochan marked this pull request as ready for review March 22, 2024 18:06
@zkochan zkochan requested review from gluxon and weyert as code owners March 22, 2024 18:06
@zkochan zkochan merged commit 8eddd21 into main Mar 22, 2024
@zkochan zkochan deleted the auto-install-peers-from-hoisted-deps branch March 22, 2024 18:45
zkochan added a commit that referenced this pull request Mar 23, 2024
@gluxon

gluxon commented Mar 30, 2024

Copy link
Copy Markdown
Member

I like this change. With this one and #7830, I think we get the deduping peer dependencies behavior with pnpm's defaults. Do we need dedupe-peer-dependents after these changes?

@zkochan

zkochan commented Mar 30, 2024

Copy link
Copy Markdown
Member Author

We do need it for the case when auto-install-peers is set to false. I also did a related change to dedupe-peer-dependents: #7841

@gluxon

gluxon commented Mar 30, 2024

Copy link
Copy Markdown
Member

That make sense. I personally like auto-install-peers, but I could see users not liking that but still wanting the behavior that comes with dedupe-peer-dependents.

zkochan added a commit that referenced this pull request Jun 12, 2026
…e, peer-vs-own-dep, peer overrides, optional-peer hoist) (#12345)

## Summary

Four lockfile-parity fixes for pacquet, continuing #12266. Measured on the monorepo itself (fresh state, `install --lockfile-only`, back-to-back against the live registry vs **pnpm 11.6.0**), the real-lockfile document diff drops from **806 to 461 changed lines**. Two consecutive pacquet runs are byte-identical before and after.

### 1. Importer's regular dep wins over its own peer range (`fix(deps-resolver)`)

An importer that declares the same alias in both `peerDependencies` and a regular group (e.g. `@pnpm/logger`: `workspace:*` devDependency + `catalog:` peer — 67 importers in this repo) resolved both specs, and the peer's registry resolution overwrote the workspace link in the importers section (`version: 1001.0.1` instead of `link:../../core/logger`, 182 diff lines). `importer_direct_wanted_specs` now merges the groups the way pnpm spreads them in [`getWantedDependencies`](https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/getWantedDependencies.ts#L32-L43): peers first, then `dependencies` < `devDependencies` < `optionalDependencies`, one wanted spec per alias. The `@pnpm/logger` resolution tallies now match pnpm's exactly.

### 2. A package's peer survives when it also lists the name as a dependency (`fix(deps-resolver)`)

Under `autoInstallPeers`, pnpm removes peer-shadowed names from a resolved package's `dependencies` **before** extracting peers ([resolveDependencies.ts#L1527-L1542](https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1527-L1542)), so the peer edge supplies the package and the depPath carries the suffix. pacquet did the inverse (dropped the peer, walked the own dep), so `@babel/parser` — whose packageExtensions-added `@babel/types` peer is also its regular dependency — lost its `(@babel/types@7.29.7)` suffix and its `peerDependencies:` block. Also aligns `extract_peer_dependencies` with [`peerDependenciesWithoutOwn`](https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1840-L1864): the package's own name counts as an own dep, and a `peerDependenciesMeta`-only entry becomes a peer only when `optional: true`. The non-`autoInstallPeers` arm (omit only peers resolvable from the parent scope) is not ported — pacquet's per-package children cache has no parent context; behavior there matches pnpm whenever the peer is not in scope.

### 3. Overrides apply to `peerDependencies` (`fix(package-manager)`)

Ports the peer arm of [`overrideDepsOfPkg`](https://github.com/pnpm/pnpm/blob/01b3d45ddb/hooks/read-package-hook/src/createVersionsOverrider.ts#L68-L129): a matched peer is deleted on `-`, rewritten in place when the override value is a valid peer range, otherwise written into `dependencies` while the declared peer stays. Fixes e.g. `ajv@>=7.0.0-alpha.0 <8.18.0 → >=8.18.0` not rewriting peer ranges and `@yarnpkg/libzip` losing its `(@yarnpkg/fslib@3.x)` suffix.

### 4. Optional-peer hoist: run-extended preferred versions, meta-only peers excluded (`fix(deps-resolver)`)

Corrects the model from #12267. pnpm folds **every run-resolved version** into `ctx.allPreferredVersions` ([resolveDependencies.ts#L1483-L1488](https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1483-L1488), since #7812) and `getHoistableOptionalPeers` reads that map after each wave — so an optional peer with a **real `peerDependencies` entry** (eslint's `jiti`) resolves against a provider anywhere in the freshly resolved tree. What keeps `debug`'s `supports-color` bare is not a static map but the missing-peer set: [`getMissingPeers`](https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1773-L1782) iterates `peerDependencies` only, so a **meta-only** peer never feeds the hoist. Both behaviors verified empirically against pnpm 11.6.0 (`eslint` + `cosmiconfig-typescript-loader` hoists `jiti@2.6.1`; `debug` + `concurrently` stays bare). pacquet's static-snapshot approach got the meta-only case right by the wrong mechanism and under-hoisted every real-entry optional peer — the missing `(jiti@2.6.1)`, `(typanion@3.14.0)`, `(conventional-commits-parser@6.4.0)`, `(@types/node@…)` suffixes and their cascades on the monorepo. The 12267-era regression test asserted the anti-pnpm behavior for the real-entry shape and was inverted; a new test pins the meta-only shape.

## Verification

- New unit tests in `pacquet-resolving-deps-resolver` (importer group merge ×4, peer-vs-own-dep shadowing ×3, optional-peer hoist real-entry/meta-only ×2) and `pacquet-package-manager` (peer overrides ×4).
- Full workspace suite: 3218 tests pass; clippy `--deny warnings` clean; rustfmt + typos clean.
- Whole-monorepo lockfile diff vs fresh pnpm 11.6.0: 806 → 461 changed lines; `@pnpm/logger`, `jiti`, `typanion`, `@babel/types`, `ajv` categories eliminated. Two consecutive pacquet runs byte-identical.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants