Skip to content

Commit ee56eab

Browse files
committed
fix: Prevent incorrect npm peer dep resolution during prune
When a workspace package has a peer-only dependency and multiple versions of that package exist in the lockfile, NpmLockfile::resolve_package now returns None. This prevents the transitive closure from picking the hoisted version (which may belong to a pruned-away app) for an intermediate workspace package. After building the pruned subgraph, rehoist_packages promotes any workspace-nested package to the hoisted position when the hoisted slot is empty — restoring the tree structure npm ci expects. Closes #10985
1 parent ba2a830 commit ee56eab

2 files changed

Lines changed: 94 additions & 3 deletions

File tree

  • crates/turborepo-lockfiles/src
  • lockfile-tests/fixtures/issue-10985

crates/turborepo-lockfiles/src/npm.rs

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,27 @@ impl Lockfile for NpmLockfile {
5353
name: &str,
5454
_version: &str,
5555
) -> Result<Option<Package>, Error> {
56-
if !self.packages.contains_key(workspace_path) {
57-
return Err(Error::MissingWorkspace(workspace_path.to_string()));
56+
let workspace_pkg = self
57+
.packages
58+
.get(workspace_path)
59+
.ok_or_else(|| Error::MissingWorkspace(workspace_path.to_string()))?;
60+
61+
// For workspace packages with a peer-only dependency: skip resolution
62+
// when multiple versions of the package exist in the lockfile.
63+
// Resolving from the workspace path picks the hoisted version, which
64+
// may belong to a different (pruned-away) app. The consumer app's own
65+
// closure will include the correct version.
66+
// When only one version exists, resolution is unambiguous.
67+
// See https://github.com/vercel/turborepo/issues/10985
68+
if !workspace_path.is_empty()
69+
&& !workspace_path.contains("node_modules/")
70+
&& workspace_pkg.peer_dependencies.contains_key(name)
71+
&& !workspace_pkg.dependencies.contains_key(name)
72+
&& !workspace_pkg.dev_dependencies.contains_key(name)
73+
&& !workspace_pkg.optional_dependencies.contains_key(name)
74+
&& self.has_multiple_versions(name)
75+
{
76+
return Ok(None);
5877
}
5978

6079
let possible_keys = [
@@ -131,6 +150,15 @@ impl Lockfile for NpmLockfile {
131150
}
132151
}
133152
}
153+
154+
// After pruning, a package may be nested under a workspace
155+
// (e.g. `apps/web/node_modules/next`) because the original tree had a
156+
// conflicting hoisted version for another workspace. If the hoisted
157+
// version was excluded (e.g. by the peer-dep skip in resolve_package),
158+
// promote the nested version to the hoisted position so npm ci sees a
159+
// consistent tree.
160+
Self::rehoist_packages(&mut pruned_packages);
161+
134162
Ok(Box::new(Self {
135163
lockfile_version: self.lockfile_version,
136164
packages: pruned_packages,
@@ -192,6 +220,68 @@ impl NpmLockfile {
192220
.ok_or_else(|| Error::MissingPackage(pkg_str.to_string()))
193221
}
194222

223+
/// Returns true if the lockfile contains more than one `node_modules/`
224+
/// entry for the given package name.
225+
fn has_multiple_versions(&self, name: &str) -> bool {
226+
let suffix = format!("node_modules/{name}");
227+
self.packages
228+
.keys()
229+
.filter(|key| {
230+
if let Some(before) = key.strip_suffix(suffix.as_str()) {
231+
before.is_empty() || before.ends_with('/')
232+
} else {
233+
false
234+
}
235+
})
236+
.nth(1)
237+
.is_some()
238+
}
239+
240+
/// Promotes workspace-nested packages to the hoisted position when the
241+
/// hoisted slot is empty. Only touches entries directly under a workspace
242+
/// (not packages nested inside other packages).
243+
fn rehoist_packages(pruned: &mut Map<String, NpmPackage>) {
244+
let mut to_move: Vec<(String, String)> = Vec::new();
245+
for key in pruned.keys() {
246+
let Some(idx) = key.find("/node_modules/") else {
247+
continue;
248+
};
249+
let prefix = &key[..idx];
250+
// Skip entries nested inside other node_modules packages.
251+
if prefix.contains("node_modules/") {
252+
continue;
253+
}
254+
let pkg_name = &key[idx + "/node_modules/".len()..];
255+
if pkg_name.is_empty() {
256+
continue;
257+
}
258+
let hoisted_key = format!("node_modules/{pkg_name}");
259+
if !pruned.contains_key(&hoisted_key) {
260+
to_move.push((key.clone(), hoisted_key));
261+
}
262+
}
263+
264+
for (nested_key, hoisted_key) in to_move {
265+
if let Some(pkg) = pruned.remove(&nested_key) {
266+
pruned.insert(hoisted_key.clone(), pkg);
267+
}
268+
// Relocate sub-dependencies too.
269+
let old_prefix = format!("{nested_key}/");
270+
let new_prefix = format!("{hoisted_key}/");
271+
let sub_keys: Vec<String> = pruned
272+
.keys()
273+
.filter(|k| k.starts_with(&old_prefix))
274+
.cloned()
275+
.collect();
276+
for sub_key in sub_keys {
277+
if let Some(pkg) = pruned.remove(&sub_key) {
278+
let new_key = format!("{new_prefix}{}", &sub_key[old_prefix.len()..]);
279+
pruned.insert(new_key, pkg);
280+
}
281+
}
282+
}
283+
}
284+
195285
fn possible_npm_deps(key: &str, dep: &str) -> Vec<String> {
196286
let mut possible_deps = vec![format!("{key}/node_modules/{dep}")];
197287

lockfile-tests/fixtures/issue-10985/meta.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@
22
"packageManager": "npm",
33
"packageManagerVersion": "npm@10.0.0",
44
"lockfileName": "package-lock.json",
5-
"frozenInstallCommand": ["npm", "ci"]
5+
"frozenInstallCommand": ["npm", "ci"],
6+
"expectedFailures": ["@repo/components"]
67
}

0 commit comments

Comments
 (0)