Skip to content

Commit 531f2a3

Browse files
morning-verluzkochan
morning-verlu
and
zkochan
authored
fix: preserve workspace specs on update (#12140)
## What - preserve existing `workspace:` dependency specifiers when `updateProjectManifest` saves updated direct dependencies and `preserveWorkspaceProtocol` is enabled - keep catalog specifiers taking precedence over resolver-normalized specs - add focused coverage for preserved and normalized local spec behavior - add a changeset for the published `@pnpm/installing.deps-resolver` change ### pacquet parity Ported the same fix to pacquet's `update` command. Previously `pacquet update --latest` routed every direct dependency through a registry `latest` lookup, so a `workspace:` local-path dependency (e.g. `workspace:../packages/foo/dist`) was rewritten into a registry version — corrupting the manifest (in the regression test it became `0.0.1-security`). Both `--latest` rewrite sites now skip registry resolution for such specs via `is_workspace_local_path_specifier`, a faithful port of pnpm's `isWorkspaceLocalPathSpecifier`. The gate is unconditional in the `--latest` path because `preserveWorkspaceProtocol` is always on there (its only override derives from `linkWorkspacePackages` under `--workspace`, which cannot be combined with `--latest`). Fixes #3902 --------- Co-authored-by: morning-verlu <258725120+morning-verlu@users.noreply.github.com> Co-authored-by: Zoltan Kochan <z@kochan.io>
1 parent c38f68a commit 531f2a3

6 files changed

Lines changed: 223 additions & 4 deletions

File tree

‎.changeset/preserve-workspace-update-spec.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+
Fixed `pnpm update` rewriting a `workspace:` dependency that points at a local path (e.g. `workspace:../packages/foo/dist`) into a normalized `link:` or version-range specifier. Such specifiers are now preserved verbatim when the workspace protocol is preserved [#3902](https://github.com/pnpm/pnpm/issues/3902).

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export async function updateProjectManifest (
2525
return {
2626
alias: rdd.alias,
2727
peer: importer.peer,
28-
bareSpecifier: rdd.catalogLookup?.userSpecifiedBareSpecifier ?? rdd.normalizedBareSpecifier ?? wantedDep.bareSpecifier,
28+
bareSpecifier: getBareSpecifierToSave(wantedDep, rdd, opts.preserveWorkspaceProtocol),
2929
resolvedVersion: rdd.version,
3030
pinnedVersion: importer.pinnedVersion,
3131
saveType: importer.targetDependenciesField,
@@ -54,3 +54,23 @@ export async function updateProjectManifest (
5454
: undefined
5555
return [hookedManifest, originalManifest]
5656
}
57+
58+
function getBareSpecifierToSave (
59+
wantedDep: ImporterToResolve['wantedDependencies'][number],
60+
resolvedDep: ResolvedDirectDependency,
61+
preserveWorkspaceProtocol: boolean
62+
): string {
63+
if (resolvedDep.catalogLookup != null) {
64+
return resolvedDep.catalogLookup.userSpecifiedBareSpecifier
65+
}
66+
if (preserveWorkspaceProtocol && isWorkspaceLocalPathSpecifier(wantedDep.bareSpecifier)) {
67+
return wantedDep.bareSpecifier
68+
}
69+
return resolvedDep.normalizedBareSpecifier ?? wantedDep.bareSpecifier
70+
}
71+
72+
function isWorkspaceLocalPathSpecifier (bareSpecifier: string): boolean {
73+
if (!bareSpecifier.startsWith('workspace:')) return false
74+
const pref = bareSpecifier.slice('workspace:'.length)
75+
return pref.startsWith('.') || pref.startsWith('/') || pref.startsWith('~/') || /^[A-Z]:/i.test(pref)
76+
}

‎installing/deps-resolver/test/updateProjectManifest.test.ts‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { expect, test } from '@jest/globals'
2+
import type { PkgResolutionId, ProjectId, ProjectRootDir } from '@pnpm/types'
3+
4+
import type { ImporterToResolve } from '../lib/index.js'
5+
import type { ResolvedDirectDependency } from '../lib/resolveDependencyTree.js'
6+
import { updateProjectManifest } from '../lib/updateProjectManifest.js'
7+
8+
test('updateProjectManifest preserves workspace protocol specs when requested', async () => {
9+
const [manifest] = await updateProjectManifest(createImporter('workspace:../packages/foo/dist'), {
10+
directDependencies: [createDirectDependency()],
11+
preserveWorkspaceProtocol: true,
12+
saveWorkspaceProtocol: 'rolling',
13+
})
14+
15+
expect(manifest?.dependencies?.foo).toBe('workspace:../packages/foo/dist')
16+
})
17+
18+
test('updateProjectManifest saves normalized local specs when workspace protocol is not preserved', async () => {
19+
const [manifest] = await updateProjectManifest(createImporter('workspace:../packages/foo/dist'), {
20+
directDependencies: [createDirectDependency()],
21+
preserveWorkspaceProtocol: false,
22+
saveWorkspaceProtocol: 'rolling',
23+
})
24+
25+
expect(manifest?.dependencies?.foo).toBe('link:../packages/foo/dist')
26+
})
27+
28+
test('updateProjectManifest saves normalized workspace range specs', async () => {
29+
const [manifest] = await updateProjectManifest(createImporter('workspace:*'), {
30+
directDependencies: [
31+
createDirectDependency({
32+
normalizedBareSpecifier: 'workspace:^1.0.0',
33+
}),
34+
],
35+
preserveWorkspaceProtocol: true,
36+
saveWorkspaceProtocol: 'rolling',
37+
})
38+
39+
expect(manifest?.dependencies?.foo).toBe('workspace:^1.0.0')
40+
})
41+
42+
test('updateProjectManifest preserves catalog specifier precedence', async () => {
43+
const [manifest] = await updateProjectManifest(createImporter('workspace:../packages/foo/dist'), {
44+
directDependencies: [
45+
createDirectDependency({
46+
catalogLookup: {
47+
catalogName: 'default',
48+
specifier: '^1.0.0',
49+
userSpecifiedBareSpecifier: 'catalog:',
50+
},
51+
}),
52+
],
53+
preserveWorkspaceProtocol: true,
54+
saveWorkspaceProtocol: 'rolling',
55+
})
56+
57+
expect(manifest?.dependencies?.foo).toBe('catalog:')
58+
})
59+
60+
function createImporter (bareSpecifier: string): ImporterToResolve {
61+
return {
62+
binsDir: '/project/node_modules/.bin',
63+
id: '.' as ProjectId,
64+
manifest: {
65+
dependencies: {
66+
foo: bareSpecifier,
67+
},
68+
},
69+
modulesDir: '/project/node_modules',
70+
rootDir: '/project' as ProjectRootDir,
71+
targetDependenciesField: 'dependencies',
72+
updatePackageManifest: true,
73+
wantedDependencies: [
74+
{
75+
alias: 'foo',
76+
bareSpecifier,
77+
dev: false,
78+
optional: false,
79+
updateSpec: true,
80+
},
81+
],
82+
}
83+
}
84+
85+
function createDirectDependency (overrides: Partial<ResolvedDirectDependency> = {}): ResolvedDirectDependency {
86+
return {
87+
alias: 'foo',
88+
dev: false,
89+
name: 'foo',
90+
normalizedBareSpecifier: 'link:../packages/foo/dist',
91+
optional: false,
92+
pkgId: 'link:../packages/foo/dist' as PkgResolutionId,
93+
resolution: {
94+
directory: '../packages/foo/dist',
95+
type: 'directory',
96+
},
97+
version: '1.0.0',
98+
...overrides,
99+
}
100+
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,46 @@ fn update_latest_preserves_exact() {
188188
drop((root, anchor));
189189
}
190190

191+
/// `--latest` must not rewrite a `workspace:` dependency that points at a
192+
/// local path. Resolving it against the registry would either fail (the
193+
/// package is workspace-only, not published) or replace the path — which can
194+
/// target a publish directory — with a version range. Regression test for
195+
/// <https://github.com/pnpm/pnpm/issues/3902>.
196+
#[test]
197+
fn update_latest_preserves_workspace_local_path_specifier() {
198+
let (root, workspace, anchor) = setup();
199+
200+
// A workspace-only sibling package, not published to the mocked
201+
// registry, referenced by a `workspace:` local path.
202+
let sibling = workspace.join("local-dep");
203+
fs::create_dir_all(&sibling).expect("mkdir local-dep");
204+
fs::write(sibling.join("package.json"), r#"{ "name": "local-dep", "version": "1.0.0" }"#)
205+
.expect("write local-dep/package.json");
206+
207+
let workspace_yaml = workspace.join("pnpm-workspace.yaml");
208+
let mut yaml = fs::read_to_string(&workspace_yaml).expect("read pnpm-workspace.yaml");
209+
// Fail loudly if the harness ever starts writing `packages:` — appending a
210+
// second top-level mapping key produces invalid YAML.
211+
assert!(
212+
!yaml.contains("packages:"),
213+
"pnpm-workspace.yaml already has a `packages:` key — update this test",
214+
);
215+
if !yaml.ends_with('\n') {
216+
yaml.push('\n');
217+
}
218+
yaml.push_str("packages:\n - 'local-dep'\n");
219+
fs::write(&workspace_yaml, yaml).expect("write pnpm-workspace.yaml");
220+
221+
write_manifest(&workspace, r#"{ "local-dep": "workspace:./local-dep" }"#);
222+
pacquet(&workspace, ["install"]).assert().success();
223+
224+
pacquet(&workspace, ["update", "--latest"]).assert().success();
225+
226+
assert_eq!(dep_spec(&workspace, "local-dep").as_deref(), Some("workspace:./local-dep"));
227+
228+
drop((root, anchor));
229+
}
230+
191231
/// A package selector only updates the matched dependency; others keep
192232
/// their manifest ranges.
193233
#[test]

‎pacquet/crates/package-manager/src/update.rs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ impl Update<'_> {
288288
if is_ignored(name) {
289289
continue;
290290
}
291-
if latest {
291+
if latest && !is_workspace_local_path_specifier(prev) {
292292
let version = fetch_latest(name, http_client, config).await?;
293293
let pin =
294294
latest_pin(&mut catalog_ctx, manifest, config, prev, name, pinned_version)?;
@@ -390,7 +390,7 @@ impl Update<'_> {
390390
} else {
391391
for (name, group, prev) in &matched_direct {
392392
drop_names.insert(name.clone());
393-
if latest {
393+
if latest && !is_workspace_local_path_specifier(prev) {
394394
let version = fetch_latest(name, http_client, config).await?;
395395
let pin = latest_pin(
396396
&mut catalog_ctx,
@@ -702,5 +702,27 @@ async fn fetch_latest(
702702
.map_err(|error| UpdateError::ResolveLatest { name: name.to_string(), error })
703703
}
704704

705+
/// Whether `bare_specifier` is a `workspace:` spec that points at a local
706+
/// path (e.g. `workspace:../packages/foo/dist`) rather than a version range
707+
/// (`workspace:*`, `workspace:^1.0.0`). Such specs are preserved verbatim on
708+
/// `--latest` instead of being resolved against the registry, since the path
709+
/// may target a publish directory that a normalized range would drop.
710+
///
711+
/// pnpm keeps these out of the registry-resolution path via
712+
/// `preserveWorkspaceProtocol`, which is always on under `update --latest`
713+
/// (the override that derives it from `linkWorkspacePackages` only runs under
714+
/// `--workspace`, and `--workspace` cannot be combined with `--latest`).
715+
/// Mirrors [`isWorkspaceLocalPathSpecifier`](https://github.com/pnpm/pnpm/blob/fddb8a4032/installing/deps-resolver/src/updateProjectManifest.ts#L72-L76).
716+
fn is_workspace_local_path_specifier(bare_specifier: &str) -> bool {
717+
let Some(pref) = bare_specifier.strip_prefix("workspace:") else {
718+
return false;
719+
};
720+
let is_windows_drive = {
721+
let mut chars = pref.chars();
722+
chars.next().is_some_and(|first| first.is_ascii_alphabetic()) && chars.next() == Some(':')
723+
};
724+
pref.starts_with('.') || pref.starts_with('/') || pref.starts_with("~/") || is_windows_drive
725+
}
726+
705727
#[cfg(test)]
706728
mod tests;

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::parse_update_param;
1+
use super::{is_workspace_local_path_specifier, parse_update_param};
22

33
#[test]
44
fn parses_bare_name_without_version() {
@@ -50,3 +50,34 @@ fn negated_unscoped_pattern_without_version() {
5050
assert_eq!(parsed.pattern, "!foo");
5151
assert_eq!(parsed.version, None);
5252
}
53+
54+
#[test]
55+
fn workspace_local_path_specifiers_are_detected() {
56+
for spec in [
57+
"workspace:.",
58+
"workspace:./packages/foo",
59+
"workspace:../packages/foo/dist",
60+
"workspace:/abs/path",
61+
"workspace:~/home/path",
62+
r"workspace:C:\packages\foo",
63+
] {
64+
assert!(is_workspace_local_path_specifier(spec), "expected {spec} to be a local path");
65+
}
66+
}
67+
68+
#[test]
69+
fn workspace_range_specifiers_are_not_local_paths() {
70+
for spec in [
71+
"workspace:*",
72+
"workspace:^",
73+
"workspace:~",
74+
"workspace:^1.0.0",
75+
"workspace:~1.2.3",
76+
"workspace:1.0.0",
77+
"workspace:alias@*",
78+
"^1.0.0",
79+
"link:../foo",
80+
] {
81+
assert!(!is_workspace_local_path_specifier(spec), "expected {spec} not to be a local path");
82+
}
83+
}

0 commit comments

Comments
 (0)