Skip to content

Commit 5c12968

Browse files
aqeelatzkochan
aqeelat
and
zkochan
authored
fix(update): handle mixed direct and transitive selectors (#12105)
* fix(update): handle mixed direct and transitive selectors * test(update): strengthen regression test and port to pacquet The pnpm regression test passed with and without the fix: the fixture's `latest` dist-tag made a fresh install of `^100.0.0` already resolve to 100.1.0, so the assertion was trivially true. Pin the transitive dep-of-pkg-with-1-dep to 100.0.0 before install so the test genuinely fails without the fix and passes with it. Add pacquet parity regression tests for the same mixed direct/transitive selector scenario (exact-name and glob forms). pacquet has no equivalent source change to make — its `update` matches every bare-name/glob selector against direct deps and locked snapshot names in one pass, so a direct selector never gates the transitive one — but the behavior is guarded by tests to lock in #12103 parity. --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
1 parent 531f2a3 commit 5c12968

4 files changed

Lines changed: 110 additions & 1 deletion

File tree

‎.changeset/fix-recursive-update-mixed-selectors.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@pnpm/installing.deps-resolver": patch
3+
"pnpm": patch
4+
---
5+
6+
Fix recursive updates of transitive dependencies when the update command mixes transitive dependency patterns with direct dependency selectors. For example, `pnpm up -r "@babel/core" uuid` now updates matching transitive `@babel/core` dependencies even when `uuid` is a direct dependency selector [#12103](https://github.com/pnpm/pnpm/issues/12103).

‎installing/commands/test/update/update.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,40 @@ test('update with negation pattern', async () => {
105105
expect(lockfile.packages['@pnpm.e2e/foo@2.0.0']).toBeTruthy()
106106
})
107107

108+
test('update transitive dependency when mixed with a direct dependency selector', async () => {
109+
// Pin the transitive @pnpm.e2e/dep-of-pkg-with-1-dep to 100.0.0 at install
110+
// time, so the later update has an older version to bump from.
111+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.0.0', distTag: 'latest' })
112+
113+
const project = prepare({
114+
dependencies: {
115+
'@pnpm.e2e/foo': '1.0.0',
116+
'@pnpm.e2e/pkg-with-1-dep': '100.0.0',
117+
},
118+
})
119+
120+
await install.handler({
121+
...DEFAULT_OPTS,
122+
dir: process.cwd(),
123+
})
124+
125+
expect(project.readLockfile().packages['@pnpm.e2e/dep-of-pkg-with-1-dep@100.0.0']).toBeTruthy()
126+
127+
await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.1.0', distTag: 'latest' })
128+
129+
// @pnpm.e2e/dep-of-pkg-with-1-dep is a transitive selector (matched via
130+
// updateMatching); @pnpm.e2e/foo is a direct dependency selector. The
131+
// presence of the direct selector must not block the transitive update.
132+
await update.handler({
133+
...DEFAULT_OPTS,
134+
dir: process.cwd(),
135+
}, ['@pnpm.e2e/dep-of-pkg-with-1-dep', '@pnpm.e2e/foo'])
136+
137+
const lockfile = project.readLockfile()
138+
139+
expect(lockfile.packages['@pnpm.e2e/dep-of-pkg-with-1-dep@100.1.0']).toBeTruthy()
140+
})
141+
108142
test('update: fail when both "latest" and "workspace" are true', async () => {
109143
preparePackages([
110144
{

‎installing/deps-resolver/src/toResolveImporter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export async function toResolveImporter (
7777
? updateLocalTarballs
7878
: (dep) => ({ ...dep, updateDepth: defaultUpdateDepth })),
7979
...existingDeps.map(
80-
opts.noDependencySelectors && project.updateMatching != null
80+
project.updateMatching != null
8181
? updateLocalTarballs
8282
: (dep) => ({ ...dep, updateDepth: -1 })
8383
),

‎pacquet/crates/cli/tests/update.rs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,75 @@ fn update_bumps_within_range() {
116116
drop((root, anchor));
117117
}
118118

119+
/// Mixing a transitive selector with a direct dependency selector must
120+
/// still update the matching transitive package. Ports pnpm's regression
121+
/// test for <https://github.com/pnpm/pnpm/issues/12103>, where a direct
122+
/// selector wrongly suppressed recursive transitive updates. pacquet
123+
/// matches every bare-name selector against direct deps and locked
124+
/// package names alike, so the direct selector never gates the
125+
/// transitive one.
126+
#[test]
127+
fn update_transitive_mixed_with_direct_selector() {
128+
let (root, workspace, anchor) = setup();
129+
130+
// Pin the transitive dep-of-pkg-with-1-dep at 100.0.0 (via a direct
131+
// exact entry), then drop it to a pure transitive of pkg-with-1-dep.
132+
write_manifest(
133+
&workspace,
134+
&format!(r#"{{ "{FOO}": "1.0.0", "{PARENT}": "100.0.0", "{DEP}": "100.0.0" }}"#),
135+
);
136+
pacquet(&workspace, ["install"]).assert().success();
137+
eprintln!("virtual store contents: {:?}", list_virtual_store(&workspace));
138+
assert!(virtual_store_has(&workspace, "@pnpm.e2e+dep-of-pkg-with-1-dep@100.0.0"));
139+
140+
write_manifest(&workspace, &format!(r#"{{ "{FOO}": "1.0.0", "{PARENT}": "100.0.0" }}"#));
141+
142+
// DEP is a transitive selector; FOO is a direct dependency selector.
143+
pacquet(&workspace, ["update", DEP, FOO]).assert().success();
144+
145+
eprintln!("virtual store contents: {:?}", list_virtual_store(&workspace));
146+
assert!(
147+
virtual_store_has(&workspace, "@pnpm.e2e+dep-of-pkg-with-1-dep@100.1.0"),
148+
"the transitive selector should bump even alongside a direct selector",
149+
);
150+
151+
drop((root, anchor));
152+
}
153+
154+
/// The glob form of the mixed-selector case — the shape from
155+
/// <https://github.com/pnpm/pnpm/issues/12103> (`pnpm up "@babel/*" uuid`).
156+
/// A glob that names only a transitive
157+
/// dependency must still bump it when a direct selector rides alongside.
158+
/// The glob is matched against locked package names through the same
159+
/// `create_matcher` path as a bare name, so the direct selector cannot
160+
/// gate it.
161+
#[test]
162+
fn update_transitive_glob_mixed_with_direct_selector() {
163+
let (root, workspace, anchor) = setup();
164+
165+
write_manifest(
166+
&workspace,
167+
&format!(r#"{{ "{FOO}": "1.0.0", "{PARENT}": "100.0.0", "{DEP}": "100.0.0" }}"#),
168+
);
169+
pacquet(&workspace, ["install"]).assert().success();
170+
eprintln!("virtual store contents: {:?}", list_virtual_store(&workspace));
171+
assert!(virtual_store_has(&workspace, "@pnpm.e2e+dep-of-pkg-with-1-dep@100.0.0"));
172+
173+
write_manifest(&workspace, &format!(r#"{{ "{FOO}": "1.0.0", "{PARENT}": "100.0.0" }}"#));
174+
175+
// "@pnpm.e2e/dep-of-*" matches the transitive dep-of-pkg-with-1-dep
176+
// only; FOO is a direct dependency selector.
177+
pacquet(&workspace, ["update", "@pnpm.e2e/dep-of-*", FOO]).assert().success();
178+
179+
eprintln!("virtual store contents: {:?}", list_virtual_store(&workspace));
180+
assert!(
181+
virtual_store_has(&workspace, "@pnpm.e2e+dep-of-pkg-with-1-dep@100.1.0"),
182+
"the transitive glob selector should bump even alongside a direct selector",
183+
);
184+
185+
drop((root, anchor));
186+
}
187+
119188
/// `pacquet update --latest` ignores the manifest range, bumps to the
120189
/// `latest` dist-tag, and rewrites `package.json`.
121190
#[test]

0 commit comments

Comments
 (0)