Skip to content

Commit cc4ff81

Browse files
zkochanzkochan
authored
fix(pacquet/registry): deserialize optionalDependencies and peerDependenciesMeta (#11934)
`PackageVersion` (the per-version registry manifest the npm resolver parses) was missing the `optionalDependencies` and `peerDependenciesMeta` fields. The resolver builds `ResolveResult.manifest` via `serde_json::to_value(picked)` and downstream walks it with [`extract_children`](https://github.com/pnpm/pnpm/blob/1fb8a2d5d8/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs#L752-L759) (reads `optionalDependencies`) and [`extract_peer_dependencies`](https://github.com/pnpm/pnpm/blob/1fb8a2d5d8/pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs#L776-L824) (reads `peerDependenciesMeta`). Without those fields on the struct, both reads always saw nothing — so `optionalDependencies` edges were silently dropped, and every optional peer was treated as required, then auto-installed via the `autoInstallPeers` fallback in [`hoist_peers`](https://github.com/pnpm/pnpm/blob/1fb8a2d5d8/pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs#L134-L136). ## Astro cascade On the vlt [`astro`](https://github.com/vltpkg/benchmarks/tree/main/fixtures/astro) fixture, `unstorage` (a transitive of astro) declares 19 optional peers via `peerDependenciesMeta` (`@azure/*`, `@vercel/*`, `@netlify/blobs`, `@upstash/redis`, `@deno/kv`, `ioredis`, `uploadthing`, …). Pacquet's resolver auto-installed every one of them and walked their transitive trees; astro's own `optionalDependencies` (`sharp`) went missing entirely. The supposed "5.5× astro deep-tree slowdown" tracked in #11902 was almost all wasted work, not a real perf bug. None of the candidate hypotheses listed there (`async_recursion` Box-pinning, per-node `lock_recoverable` mutex acquires, manifest `serde_json::to_value` cost, tarball extraction) were the bottleneck. ## Before / after on vlt astro | Metric | Before | After | pnpm 11.3.0 | |---|---:|---:|---:| | `pacquet install` wall time (warm store) | 39.6 s | 8.5 s | 7.0 s | | Lockfile lines | 13,364 | 3,037 | 3,444 | | `resolution:` entries | 1,535 | 377 | 377 | | Astro root peer suffixes | 30 (`@azure/...`, `@vercel/...`, ...) | `(rollup@4.60.4)(typescript@5.9.3)` | `(rollup@4.60.4)(typescript@5.9.3)` | | `sharp` (`optionalDependencies`) refs in lockfile | 0 | 110 | 85 | Warm-cache hyperfine (3 runs, fresh `node_modules` + lockfile each time): ``` pacquet (patched): 670 ms ± 72 ms pnpm 11.3.0: 1270 ms ± 7 ms pacquet is 1.89 ± 0.20 times faster than pnpm ``` Closes the astro column in #11902. ## Implementation - Add `optional_dependencies: Option<HashMap<String, String>>` and `peer_dependencies_meta: Option<HashMap<String, PeerDependencyMeta>>` to `PackageVersion`. The existing `#[serde(rename_all = "camelCase")]` handles wire format. - Add a `PeerDependencyMeta` newtype with just the `optional` field (the only field the resolver consumes). - Fix up the four struct-literal construction sites in tests + the trust-evidence projection. - Add a regression test that deserializes a fixture with both fields populated and asserts they round-trip through `serde_json::to_value` — which is what the resolver consumes.
1 parent 7120ac0 commit cc4ff81

8 files changed

Lines changed: 514 additions & 1 deletion

File tree

‎pacquet/crates/package-manager/src/build_snapshot/tests.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ fn make_package(name: &str, version: &str) -> PackageVersion {
2727
dependencies: None,
2828
dev_dependencies: None,
2929
peer_dependencies: None,
30+
optional_dependencies: None,
31+
peer_dependencies_meta: None,
3032
npm_user: None,
3133
deprecated: None,
3234
}

‎pacquet/crates/package-manager/src/install/tests.rs‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,180 @@ async fn install_optional_failing_postinstall_dep_via_registry_mock_succeeds() {
895895
drop((dir, mock_instance));
896896
}
897897

898+
/// Regression for pnpm/pnpm#11934: `peerDependenciesMeta` must be
899+
/// preserved end-to-end so optional peers are not auto-installed.
900+
/// Ported from upstream's
901+
/// [`peerDependencies.ts:1181-1255`](https://github.com/pnpm/pnpm/blob/1fb8a2d5d8/installing/deps-installer/test/install/peerDependencies.ts#L1181-L1255).
902+
#[tokio::test]
903+
async fn auto_install_peers_does_not_cascade_optional_peers() {
904+
let mock_instance = AutoMockInstance::load_or_init();
905+
906+
let dir = tempdir().unwrap();
907+
let store_dir = dir.path().join("pacquet-store");
908+
let project_root = dir.path().join("project");
909+
let modules_dir = project_root.join("node_modules");
910+
let virtual_store_dir = modules_dir.join(".pacquet");
911+
912+
let manifest_path = dir.path().join("package.json");
913+
let mut manifest = PackageManifest::create_if_needed(manifest_path.clone()).unwrap();
914+
manifest
915+
.add_dependency("@pnpm.e2e/abc-optional-peers", "1.0.0", DependencyGroup::Prod)
916+
.unwrap();
917+
manifest.save().unwrap();
918+
919+
let mut config = Config::new();
920+
config.store_dir = store_dir.into();
921+
config.modules_dir = modules_dir.to_path_buf();
922+
config.virtual_store_dir = virtual_store_dir.clone();
923+
config.registry = mock_instance.url();
924+
let config = config.leak();
925+
926+
Install {
927+
tarball_mem_cache: Default::default(),
928+
http_client: &Default::default(),
929+
http_client_arc: std::sync::Arc::new(Default::default()),
930+
config,
931+
manifest: &manifest,
932+
lockfile: None,
933+
lockfile_path: None,
934+
dependency_groups: [DependencyGroup::Prod, DependencyGroup::Optional],
935+
frozen_lockfile: false,
936+
prefer_frozen_lockfile: None,
937+
ignore_manifest_check: false,
938+
skip_runtimes: false,
939+
trust_lockfile: false,
940+
supported_architectures: None,
941+
node_linker: pacquet_config::NodeLinker::default(),
942+
resolved_packages: &Default::default(),
943+
}
944+
.run::<SilentReporter>()
945+
.await
946+
.expect("install with optional peers should succeed");
947+
948+
let virtual_store_slots: Vec<String> = std::fs::read_dir(&virtual_store_dir)
949+
.expect("read virtual store dir")
950+
.filter_map(Result::ok)
951+
.map(|entry| entry.file_name().to_string_lossy().into_owned())
952+
.collect();
953+
954+
assert!(
955+
virtual_store_slots.iter().any(|name| name.starts_with("@pnpm.e2e+peer-a@")),
956+
"required peer `peer-a` must be auto-installed under \
957+
`autoInstallPeers: true`; virtual-store slots: {virtual_store_slots:?}",
958+
);
959+
960+
for optional_peer in ["peer-b", "peer-c"] {
961+
let slot_prefix = format!("@pnpm.e2e+{optional_peer}@");
962+
let cascaded: Vec<&String> =
963+
virtual_store_slots.iter().filter(|name| name.starts_with(&slot_prefix)).collect();
964+
assert!(
965+
cascaded.is_empty(),
966+
"optional peer `{optional_peer}` must NOT reach the virtual store; \
967+
found {cascaded:?}",
968+
);
969+
}
970+
971+
assert!(
972+
virtual_store_slots
973+
.iter()
974+
.any(|name| name.starts_with("@pnpm.e2e+abc-optional-peers@1.0.0")),
975+
"abc-optional-peers must reach the virtual store; \
976+
virtual-store slots: {virtual_store_slots:?}",
977+
);
978+
assert!(
979+
is_symlink_or_junction(&project_root.join("node_modules/@pnpm.e2e/abc-optional-peers"))
980+
.unwrap(),
981+
"abc-optional-peers must be symlinked at the importer level",
982+
);
983+
984+
drop((dir, mock_instance));
985+
}
986+
987+
/// Companion to [`auto_install_peers_does_not_cascade_optional_peers`]:
988+
/// `@pnpm.e2e/abc-optional-peers-meta-only@1.0.0` declares `peer-b` and
989+
/// `peer-c` **only** through `peerDependenciesMeta`, with no matching
990+
/// `peerDependencies` entry. Upstream and pacquet treat such entries
991+
/// as optional peers with implicit range `*`; the install must still
992+
/// keep them out of the tree when no other consumer requests them.
993+
///
994+
/// Ported from upstream's "warning is not reported when cannot resolve
995+
/// optional peer dependency (specified by meta field only)" at
996+
/// [`installing/deps-installer/test/install/peerDependencies.ts`](https://github.com/pnpm/pnpm/blob/1fb8a2d5d8/installing/deps-installer/test/install/peerDependencies.ts#L1257-L1323).
997+
#[tokio::test]
998+
async fn auto_install_peers_skips_meta_only_optional_peers() {
999+
let mock_instance = AutoMockInstance::load_or_init();
1000+
1001+
let dir = tempdir().unwrap();
1002+
let store_dir = dir.path().join("pacquet-store");
1003+
let project_root = dir.path().join("project");
1004+
let modules_dir = project_root.join("node_modules");
1005+
let virtual_store_dir = modules_dir.join(".pacquet");
1006+
1007+
let manifest_path = dir.path().join("package.json");
1008+
let mut manifest = PackageManifest::create_if_needed(manifest_path.clone()).unwrap();
1009+
manifest
1010+
.add_dependency("@pnpm.e2e/abc-optional-peers-meta-only", "1.0.0", DependencyGroup::Prod)
1011+
.unwrap();
1012+
manifest.save().unwrap();
1013+
1014+
let mut config = Config::new();
1015+
config.store_dir = store_dir.into();
1016+
config.modules_dir = modules_dir.to_path_buf();
1017+
config.virtual_store_dir = virtual_store_dir.clone();
1018+
config.registry = mock_instance.url();
1019+
let config = config.leak();
1020+
1021+
Install {
1022+
tarball_mem_cache: Default::default(),
1023+
http_client: &Default::default(),
1024+
http_client_arc: std::sync::Arc::new(Default::default()),
1025+
config,
1026+
manifest: &manifest,
1027+
lockfile: None,
1028+
lockfile_path: None,
1029+
dependency_groups: [DependencyGroup::Prod, DependencyGroup::Optional],
1030+
frozen_lockfile: false,
1031+
prefer_frozen_lockfile: None,
1032+
ignore_manifest_check: false,
1033+
skip_runtimes: false,
1034+
trust_lockfile: false,
1035+
supported_architectures: None,
1036+
node_linker: pacquet_config::NodeLinker::default(),
1037+
resolved_packages: &Default::default(),
1038+
}
1039+
.run::<SilentReporter>()
1040+
.await
1041+
.expect("install with meta-only optional peers should succeed");
1042+
1043+
let virtual_store_slots: Vec<String> = std::fs::read_dir(&virtual_store_dir)
1044+
.expect("read virtual store dir")
1045+
.filter_map(Result::ok)
1046+
.map(|entry| entry.file_name().to_string_lossy().into_owned())
1047+
.collect();
1048+
1049+
// peer-a is declared in `peerDependencies` so it stays required and
1050+
// gets auto-installed; peer-b and peer-c are declared *only* in
1051+
// `peerDependenciesMeta` with `optional: true` and stay out of the
1052+
// tree.
1053+
assert!(
1054+
virtual_store_slots.iter().any(|name| name.starts_with("@pnpm.e2e+peer-a@")),
1055+
"required peer `peer-a` must be auto-installed; \
1056+
virtual-store slots: {virtual_store_slots:?}",
1057+
);
1058+
for optional_peer in ["peer-b", "peer-c"] {
1059+
let slot_prefix = format!("@pnpm.e2e+{optional_peer}@");
1060+
let cascaded: Vec<&String> =
1061+
virtual_store_slots.iter().filter(|name| name.starts_with(&slot_prefix)).collect();
1062+
assert!(
1063+
cascaded.is_empty(),
1064+
"meta-only optional peer `{optional_peer}` must NOT reach the virtual store; \
1065+
found {cascaded:?}",
1066+
);
1067+
}
1068+
1069+
drop((dir, mock_instance));
1070+
}
1071+
8981072
/// A v9 lockfile fixture pinned to a placeholder package whose
8991073
/// integrity is bogus on purpose. Pacquet enforces tarball integrity
9001074
/// on the install path, so any test that lets the install reach the

‎pacquet/crates/registry/src/package/tests.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ pub fn package_version_should_include_peers() {
1919
dependencies: Some(dependencies),
2020
dev_dependencies: None,
2121
peer_dependencies: Some(peer_dependencies),
22+
optional_dependencies: None,
23+
peer_dependencies_meta: None,
2224
npm_user: None,
2325
deprecated: None,
2426
};
@@ -40,6 +42,8 @@ pub fn serialized_according_to_params() {
4042
dependencies: None,
4143
dev_dependencies: None,
4244
peer_dependencies: None,
45+
optional_dependencies: None,
46+
peer_dependencies_meta: None,
4347
npm_user: None,
4448
deprecated: None,
4549
};
@@ -95,6 +99,8 @@ fn package_with_versions(name: &str, versions: &[&str], latest: &str) -> Package
9599
dependencies: None,
96100
dev_dependencies: None,
97101
peer_dependencies: None,
102+
optional_dependencies: None,
103+
peer_dependencies_meta: None,
98104
npm_user: None,
99105
deprecated: None,
100106
},

‎pacquet/crates/registry/src/package_version.rs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ pub struct PackageVersion {
3030
skip_serializing_if = "Option::is_none"
3131
)]
3232
pub peer_dependencies: Option<HashMap<String, String>>,
33+
#[serde(
34+
default,
35+
deserialize_with = "deserialize_dependency_map",
36+
skip_serializing_if = "Option::is_none"
37+
)]
38+
pub optional_dependencies: Option<HashMap<String, String>>,
39+
#[serde(default, skip_serializing_if = "Option::is_none")]
40+
pub peer_dependencies_meta: Option<HashMap<String, PeerDependencyMeta>>,
3341

3442
/// npm registry's per-version publisher metadata. When
3543
/// `trusted_publisher` is present alongside
@@ -172,6 +180,16 @@ where
172180
deserializer.deserialize_any(DeprecatedVisitor)
173181
}
174182

183+
/// `peerDependenciesMeta[name]` shape from the npm registry. Only the
184+
/// `optional` flag is consumed by the resolver; other fields the
185+
/// registry may serve are ignored.
186+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
187+
#[serde(rename_all = "camelCase")]
188+
pub struct PeerDependencyMeta {
189+
#[serde(default, skip_serializing_if = "Option::is_none")]
190+
pub optional: Option<bool>,
191+
}
192+
175193
/// `_npmUser` field on a per-version manifest. The verifier reads
176194
/// `trusted_publisher` to assign the higher of the two trust ranks
177195
/// (`trustedPublisher` > `provenance` > none). `name` / `email` are
@@ -311,4 +329,53 @@ mod tests {
311329
assert_eq!(pkg_version.name, "acme");
312330
mock.assert_async().await;
313331
}
332+
333+
/// The abbreviated registry response (`application/vnd.npm.install-v1+json`)
334+
/// carries `optionalDependencies` and `peerDependenciesMeta` for any
335+
/// package that publishes them. Both must round-trip through
336+
/// [`PackageVersion`] so the resolver's `extract_children` reads the
337+
/// optional-dep edges and `extract_peer_dependencies` reads the
338+
/// per-peer `optional` flag. Dropping either field silently treats
339+
/// optional peers as required (auto-installed via
340+
/// `autoInstallPeers`) and skips `optionalDependencies` entirely.
341+
#[test]
342+
fn deserializes_optional_dependencies_and_peer_dependencies_meta() {
343+
let body = r#"{
344+
"name": "unstorage",
345+
"version": "1.17.5",
346+
"dist": {
347+
"integrity": "sha512-AAAA",
348+
"shasum": "0000000000000000000000000000000000000000",
349+
"tarball": "https://registry.test/unstorage-1.17.5.tgz"
350+
},
351+
"peerDependencies": {
352+
"@vercel/kv": "^1 || ^2 || ^3",
353+
"ioredis": "^5.4.2"
354+
},
355+
"peerDependenciesMeta": {
356+
"@vercel/kv": { "optional": true },
357+
"ioredis": { "optional": true }
358+
},
359+
"optionalDependencies": {
360+
"sharp": "^0.34.0"
361+
}
362+
}"#;
363+
364+
let pkg: PackageVersion =
365+
serde_json::from_str(body).expect("deserialize PackageVersion fixture");
366+
367+
let optional = pkg.optional_dependencies.as_ref().expect("optionalDependencies present");
368+
assert_eq!(optional.get("sharp").map(String::as_str), Some("^0.34.0"));
369+
370+
let peer_meta = pkg.peer_dependencies_meta.as_ref().expect("peerDependenciesMeta present");
371+
assert_eq!(peer_meta["@vercel/kv"].optional, Some(true));
372+
assert_eq!(peer_meta["ioredis"].optional, Some(true));
373+
374+
// The JSON shape `serde_json::to_value(pkg)` produces feeds
375+
// `extract_children` / `extract_peer_dependencies` downstream;
376+
// both consume the camelCase keys verbatim.
377+
let value = serde_json::to_value(&pkg).expect("serialize PackageVersion");
378+
assert!(value.get("optionalDependencies").is_some_and(|v| v.is_object()));
379+
assert!(value.get("peerDependenciesMeta").is_some_and(|v| v.is_object()));
380+
}
314381
}

‎pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,8 @@ fn project_trust_package_version(version: &PackageVersion) -> PackageVersion {
899899
dependencies: None,
900900
dev_dependencies: None,
901901
peer_dependencies: None,
902+
optional_dependencies: None,
903+
peer_dependencies_meta: None,
902904
npm_user,
903905
deprecated: None,
904906
}

‎pacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rs‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,84 @@ async fn jsr_specifier_without_selector_uses_default_tag() {
328328
assert_eq!(result.resolved_via, "jsr-registry");
329329
}
330330

331+
/// `optionalDependencies` and `peerDependenciesMeta` round-trip from the
332+
/// registry's per-version manifest into [`ResolveResult::manifest`]
333+
/// (a [`serde_json::Value`]). Downstream
334+
/// `extract_children` reads the optional-dep edges and
335+
/// `extract_peer_dependencies` reads the per-peer `optional` flag;
336+
/// dropping either field silently treats optional peers as required
337+
/// (so `autoInstallPeers` cascades them in) and skips
338+
/// `optionalDependencies` entirely. See pnpm/pnpm#11934.
339+
#[tokio::test]
340+
async fn resolved_manifest_carries_optional_dependencies_and_peer_dependencies_meta() {
341+
const BODY: &str = r#"{
342+
"name": "consumer",
343+
"dist-tags": { "latest": "1.0.0" },
344+
"modified": "2025-01-15T12:00:00.000Z",
345+
"versions": {
346+
"1.0.0": {
347+
"name": "consumer",
348+
"version": "1.0.0",
349+
"peerDependencies": {
350+
"@vercel/kv": "^1 || ^2 || ^3",
351+
"ioredis": "^5.4.2"
352+
},
353+
"peerDependenciesMeta": {
354+
"@vercel/kv": { "optional": true },
355+
"ioredis": { "optional": true }
356+
},
357+
"optionalDependencies": {
358+
"sharp": "^0.34.0"
359+
},
360+
"dist": {
361+
"integrity": "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==",
362+
"shasum": "0000000000000000000000000000000000000000",
363+
"tarball": "https://registry/consumer-1.0.0.tgz"
364+
}
365+
}
366+
}
367+
}"#;
368+
369+
let mut server = mockito::Server::new_async().await;
370+
let _mock =
371+
server.mock("GET", "/consumer").with_status(200).with_body(BODY).create_async().await;
372+
let registry = format!("{}/", server.url());
373+
let (resolver, _tempdir) = build_resolver(&registry);
374+
375+
let wanted = WantedDependency {
376+
alias: Some("consumer".to_string()),
377+
bare_specifier: Some("^1.0.0".to_string()),
378+
..WantedDependency::default()
379+
};
380+
let result = resolver.resolve(&wanted, &ResolveOptions::default()).await.unwrap().unwrap();
381+
let manifest = result.manifest.as_ref().expect("npm resolver populates manifest");
382+
383+
let optional = manifest
384+
.get("optionalDependencies")
385+
.and_then(serde_json::Value::as_object)
386+
.expect("optionalDependencies present");
387+
assert_eq!(optional.get("sharp").and_then(serde_json::Value::as_str), Some("^0.34.0"));
388+
389+
let peer_meta = manifest
390+
.get("peerDependenciesMeta")
391+
.and_then(serde_json::Value::as_object)
392+
.expect("peerDependenciesMeta present");
393+
assert_eq!(
394+
peer_meta
395+
.get("@vercel/kv")
396+
.and_then(|v| v.get("optional"))
397+
.and_then(serde_json::Value::as_bool),
398+
Some(true),
399+
);
400+
assert_eq!(
401+
peer_meta
402+
.get("ioredis")
403+
.and_then(|v| v.get("optional"))
404+
.and_then(serde_json::Value::as_bool),
405+
Some(true),
406+
);
407+
}
408+
331409
#[tokio::test]
332410
async fn jsr_specifier_with_invalid_scope_propagates_parser_error() {
333411
let server = mockito::Server::new_async().await;

0 commit comments

Comments
 (0)