diff --git a/Cargo.lock b/Cargo.lock index c86cd2578e..20040d9c1a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8595,6 +8595,7 @@ dependencies = [ "semver 1.0.28", "serde", "serde_json", + "serde_yaml", "sha1", "sha2 0.10.9", "tar", diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/.gitignore b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/.gitignore new file mode 100644 index 0000000000..c2658d7d1b --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/.gitignore @@ -0,0 +1 @@ +node_modules/ diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/package.json new file mode 100644 index 0000000000..be1efb7222 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/package.json @@ -0,0 +1,7 @@ +{ + "name": "command-update-catalog-protocol-yarn", + "devDependencies": { + "vite": "^7.0.0" + }, + "packageManager": "yarn@4.12.0" +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/snapshots.toml new file mode 100644 index 0000000000..21f9073b78 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/snapshots.toml @@ -0,0 +1,37 @@ +[[case]] +name = "command_update_catalog_protocol_yarn" +vp = "global" +skip-platforms = ["windows"] +unset-env = ["CI", "VP_SKIP_INSTALL"] +local-registry = true +steps = [ + { argv = [ + "vp", + "migrate", + "--no-interactive", + "--no-hooks", + "--package-manager", + "yarn", + ], comment = "migrate pins the toolchain through the Yarn catalog", continue-on-failure = true }, + { argv = [ + "vpt", + "print-file", + "package.json", + ], comment = "the migrated project references the catalog", continue-on-failure = true }, + { argv = [ + "vp", + "up", + "vite", + "vite-plus", + ], comment = "#2309 Yarn variant: `yarn up` rewrites `catalog:` specs, so catalog-pinned names are skipped", continue-on-failure = true }, + { argv = [ + "vpt", + "print-file", + "package.json", + ], comment = "`vite` and `vite-plus` stay `catalog:` instead of concrete ranges", continue-on-failure = true }, + { argv = [ + "vpt", + "print-file", + ".yarnrc.yml", + ], comment = "the catalog keeps owning the resolved toolchain version", continue-on-failure = true }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/snapshots/command_update_catalog_protocol_yarn.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/snapshots/command_update_catalog_protocol_yarn.md new file mode 100644 index 0000000000..78969f10c0 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_update_catalog_protocol_yarn/snapshots/command_update_catalog_protocol_yarn.md @@ -0,0 +1,81 @@ +# command_update_catalog_protocol_yarn + +## `vp migrate --no-interactive --no-hooks --package-manager yarn` + +migrate pins the toolchain through the Yarn catalog + +``` +VITE+ - The Unified Toolchain for the Web + +⚠ Vite+ does not currently support Yarn Plug'n'Play (PnP). + +✔ Switched Yarn to node-modules mode + +Formatting code... + +Code formatted +◇ Migrated . to Vite+ +• Node yarn +✓ Dependencies installed in +• 1 config update applied +• Package manager settings configured +``` + +## `vpt print-file package.json` + +the migrated project references the catalog + +``` +{ + "name": "command-update-catalog-protocol-yarn", + "devDependencies": { + "vite": "catalog:", + "vite-plus": "catalog:" + }, + "resolutions": { + "vite": "npm:@voidzero-dev/vite-plus-core@" + }, + "packageManager": "yarn@4.12.0" +} +``` + +## `vp up vite vite-plus` + +#2309 Yarn variant: `yarn up` rewrites `catalog:` specs, so catalog-pinned names are skipped + +``` +warn: Skipped vite: the Yarn catalog pins its version, and `yarn up` would overwrite the `catalog:` reference. Edit the catalog entry in .yarnrc.yml, or run `vp migrate` when Vite+ manages the pin. +warn: Skipped vite-plus: the Yarn catalog pins its version, and `yarn up` would overwrite the `catalog:` reference. Edit the catalog entry in .yarnrc.yml, or run `vp migrate` when Vite+ manages the pin. +``` + +## `vpt print-file package.json` + +`vite` and `vite-plus` stay `catalog:` instead of concrete ranges + +``` +{ + "name": "command-update-catalog-protocol-yarn", + "devDependencies": { + "vite": "catalog:", + "vite-plus": "catalog:" + }, + "resolutions": { + "vite": "npm:@voidzero-dev/vite-plus-core@" + }, + "packageManager": "yarn@4.12.0" +} +``` + +## `vpt print-file .yarnrc.yml` + +the catalog keeps owning the resolved toolchain version + +``` +nodeLinker: node-modules +npmPreapprovedPackages: + - vitest + - "@vitest/*" +catalog: + vite: npm:@voidzero-dev/vite-plus-core@ + vite-plus: +``` diff --git a/crates/vp_pm_cli/Cargo.toml b/crates/vp_pm_cli/Cargo.toml index c0e5c7b181..fc974a70ef 100644 --- a/crates/vp_pm_cli/Cargo.toml +++ b/crates/vp_pm_cli/Cargo.toml @@ -23,6 +23,7 @@ pathdiff = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["preserve_order"] } +serde_yaml = { workspace = true } sha1 = { workspace = true } sha2 = { workspace = true } tar = { workspace = true } diff --git a/crates/vp_pm_cli/src/dispatch.rs b/crates/vp_pm_cli/src/dispatch.rs index b38e19867b..7b286847d1 100644 --- a/crates/vp_pm_cli/src/dispatch.rs +++ b/crates/vp_pm_cli/src/dispatch.rs @@ -57,6 +57,15 @@ pub async fn dispatch_with_metadata( }; let package_manager = manager.client; + let mut command = command; + if let PackageManagerCommand::Update(args) = &mut command + && !args.packages.is_empty() + { + // Resolution stays free of filesystem access, so the Yarn catalog + // context is read here and handed over as data. The berry update + // resolver uses it to keep `catalog:` references out of `yarn up`. + args.yarn_catalog_packages = crate::yarn_catalog::yarn_catalog_package_names(cwd, &manager); + } let resolution = command.resolve_for_manager(&manager)?; let status = run_resolution(cwd, resolution, render_diagnostics).await?; Ok(DispatchResult { status, package_manager }) diff --git a/crates/vp_pm_cli/src/lib.rs b/crates/vp_pm_cli/src/lib.rs index 98b025aac8..f471480e3d 100644 --- a/crates/vp_pm_cli/src/lib.rs +++ b/crates/vp_pm_cli/src/lib.rs @@ -16,6 +16,7 @@ mod package_manager; mod request; pub(crate) mod resolution; mod shim; +mod yarn_catalog; pub use cli::{ManagedGlobalCommand, PackageManagerCommand, PmCommand}; pub use config::npm_registry; diff --git a/crates/vp_pm_cli/src/resolution/commands/update.rs b/crates/vp_pm_cli/src/resolution/commands/update.rs index f547e7ac89..0e86173ee6 100644 --- a/crates/vp_pm_cli/src/resolution/commands/update.rs +++ b/crates/vp_pm_cli/src/resolution/commands/update.rs @@ -2,7 +2,7 @@ use vp_pm_cli_macros::pm_args; use super::parse_positive_usize; use crate::resolution::{ - Bun, CommandBuilder, CommandResolution, Diagnostics, Npm, Pnpm, Resolve, Yarn, + Bun, CommandBuilder, CommandResolution, DiagnosticKind, Diagnostics, Npm, Pnpm, Resolve, Yarn, }; #[pm_args] @@ -70,6 +70,14 @@ pub struct UpdateArgs { /// Additional arguments to pass through to the package manager #[arg(last = true, allow_hyphen_values = true)] pub(crate) pass_through_args: Vec, + + /// Package names the Yarn catalog pins (`catalog`/`catalogs` keys in + /// `.yarnrc.yml`). Dispatch fills this from the workspace root; it never + /// comes from the command line. `yarn up ` rewrites the manifest + /// spec of every named package and would replace a `catalog:` reference + /// with a concrete range, so the berry resolver skips these names. + #[arg(skip)] + pub(crate) yarn_catalog_packages: Vec, } impl Resolve for Pnpm { @@ -111,9 +119,9 @@ impl Resolve for Npm { } impl Resolve for Yarn { - fn resolve(&self, args: &UpdateArgs, _diag: &mut Diagnostics) -> CommandResolution { + fn resolve(&self, args: &UpdateArgs, diag: &mut Diagnostics) -> CommandResolution { if self.is_berry() { - Yarn::resolve_berry_update(args) + Yarn::resolve_berry_update(args, diag) } else { Yarn::resolve_v1_update(args) } @@ -121,7 +129,23 @@ impl Resolve for Yarn { } impl Yarn { - fn resolve_berry_update(args: &UpdateArgs) -> CommandResolution { + fn resolve_berry_update(args: &UpdateArgs, diag: &mut Diagnostics) -> CommandResolution { + // `yarn up ` rewrites the manifest spec of every named package. + // A `catalog:` reference would come back as a concrete range and lose + // its catalog provenance (issue #2309 under pnpm; Yarn has no + // upstream fix as of 4.18), so catalog-pinned bare names are skipped. + // A descriptor with an explicit range (`vite@^8`) states the user's + // intent to leave the catalog and passes through. + let packages: Vec<&String> = args + .packages + .iter() + .filter(|package| !Self::skip_yarn_catalog_pinned(package.as_str(), args, diag)) + .collect(); + if packages.is_empty() && !args.packages.is_empty() { + // Every requested package is catalog-pinned. A bare `yarn up` + // would re-resolve the whole project instead, so do not run one. + return CommandResolution::Noop; + } let mut cmd = CommandBuilder::new("yarn"); if !args.filter.is_empty() { cmd.arg("workspaces").arg("foreach").arg("--all"); @@ -131,10 +155,27 @@ impl Yarn { .arg_if("--recursive", args.recursive) .arg_if("--interactive", args.interactive) .extend(args.pass_through_args.iter()) - .extend(args.packages.iter()); + .extend(packages); cmd.into() } + /// True when `package` is a bare name the Yarn catalog pins; also emits + /// the skip warning. Glob patterns and `name@range` descriptors never + /// match: they carry explicit user intent and pass through to `yarn up`. + fn skip_yarn_catalog_pinned(package: &str, args: &UpdateArgs, diag: &mut Diagnostics) -> bool { + let has_explicit_range = package.char_indices().any(|(index, ch)| ch == '@' && index > 0); + if has_explicit_range || !args.yarn_catalog_packages.iter().any(|name| name == package) { + return false; + } + diag.warn( + DiagnosticKind::BehaviorChange, + vt_str::format!( + "Skipped {package}: the Yarn catalog pins its version, and `yarn up` would overwrite the `catalog:` reference. Edit the catalog entry in .yarnrc.yml, or run `vp migrate` when Vite+ manages the pin." + ), + ); + true + } + fn resolve_v1_update(args: &UpdateArgs) -> CommandResolution { let mut cmd = CommandBuilder::new("yarn"); if let Some(filter) = args.filter.first() { @@ -379,6 +420,65 @@ mod tests { ); } + #[test] + fn test_yarn_v4_update_skips_catalog_pinned_bare_names() { + let mut options = update_args(&["vite-plus", "react"]); + options.yarn_catalog_packages = vec!["vite".to_string(), "vite-plus".to_string()]; + let resolution = resolve(&yarn("4.12.0"), options); + let command = expect_run(resolution.outcome); + + assert_eq!(command.program, "yarn"); + assert_eq!(command.args, vec!["up", "react"]); + let messages = + resolution.diagnostics.iter().map(|entry| entry.message.as_str()).collect::>(); + assert_eq!(messages.len(), 1); + assert!(messages[0].starts_with("Skipped vite-plus:")); + } + + #[test] + fn test_yarn_v4_update_noop_when_all_names_catalog_pinned() { + let mut options = update_args(&["vite", "vite-plus"]); + options.yarn_catalog_packages = vec!["vite".to_string(), "vite-plus".to_string()]; + let resolution = resolve(&yarn("4.12.0"), options); + + assert_eq!(resolution.outcome, CommandResolution::Noop); + assert_eq!(resolution.diagnostics.len(), 2); + } + + #[test] + fn test_yarn_v4_update_explicit_range_bypasses_catalog_pin() { + // `vite@^8` and `@scope/pkg@^1` carry an explicit range: the user + // chose to leave the catalog, so the descriptors pass through. + let mut options = update_args(&["vite@^8.0.0", "@scope/pkg@^1.0.0"]); + options.yarn_catalog_packages = vec!["vite".to_string(), "@scope/pkg".to_string()]; + let resolution = resolve(&yarn("4.12.0"), options); + let command = expect_run(resolution.outcome); + + assert_eq!(command.args, vec!["up", "vite@^8.0.0", "@scope/pkg@^1.0.0"]); + assert!(resolution.diagnostics.is_empty()); + } + + #[test] + fn test_yarn_v4_update_skips_catalog_pinned_scoped_bare_name() { + let mut options = update_args(&["@scope/pkg"]); + options.yarn_catalog_packages = vec!["@scope/pkg".to_string()]; + let resolution = resolve(&yarn("4.12.0"), options); + + assert_eq!(resolution.outcome, CommandResolution::Noop); + assert_eq!(resolution.diagnostics.len(), 1); + } + + #[test] + fn test_yarn_v1_update_ignores_catalog_context() { + let mut options = update_args(&["vite"]); + options.yarn_catalog_packages = vec!["vite".to_string()]; + let resolution = resolve(&yarn("1.22.0"), options); + let command = expect_run(resolution.outcome); + + assert_eq!(command.args, vec!["upgrade", "vite"]); + assert!(resolution.diagnostics.is_empty()); + } + #[test] fn test_yarn_v4_update_recursive() { let options = UpdateArgs { recursive: true, ..Default::default() }; diff --git a/crates/vp_pm_cli/src/yarn_catalog.rs b/crates/vp_pm_cli/src/yarn_catalog.rs new file mode 100644 index 0000000000..ba879ddb23 --- /dev/null +++ b/crates/vp_pm_cli/src/yarn_catalog.rs @@ -0,0 +1,119 @@ +//! Package names pinned through the Yarn catalog. +//! +//! Yarn Berry resolves `catalog:` references through the `catalog`/`catalogs` +//! maps in `.yarnrc.yml` (Yarn >= 4.10.0). `yarn up ` REWRITES the +//! manifest spec of every named package, and a `catalog:` reference gets +//! replaced with a concrete range (the Yarn variant of issue #2309). Dispatch +//! reads the catalog names here and hands them to update resolution, which +//! skips those packages instead of letting the rewrite destroy the reference. + +use vt_path::AbsolutePath; +use vt_workspace::find_workspace_root; + +use crate::{PackageManager, PackageManagerType}; + +/// Collect the package names the Yarn catalog pins for the project that owns +/// `cwd`. Returns an empty list for non-Yarn managers, projects without a +/// workspace root, and rc files without catalog entries. Read failures and +/// malformed YAML also return an empty list: the guard must never block an +/// update that Yarn itself would accept. +pub(crate) fn yarn_catalog_package_names( + cwd: &AbsolutePath, + manager: &PackageManager, +) -> Vec { + if manager.client != PackageManagerType::Yarn { + return Vec::new(); + } + let Ok((workspace_root, _cwd)) = find_workspace_root(cwd) else { + return Vec::new(); + }; + read_catalog_names(&workspace_root.path) +} + +fn read_catalog_names(workspace_root: &AbsolutePath) -> Vec { + let yarnrc_yml_path = workspace_root.join(".yarnrc.yml"); + let Ok(content) = std::fs::read_to_string(&yarnrc_yml_path) else { + return Vec::new(); + }; + let Ok(doc) = serde_yaml::from_str::(&content) else { + return Vec::new(); + }; + let mut names = Vec::new(); + collect_mapping_keys(doc.get("catalog"), &mut names); + if let Some(catalogs) = doc.get("catalogs").and_then(serde_yaml::Value::as_mapping) { + for named_catalog in catalogs.values() { + collect_mapping_keys(Some(named_catalog), &mut names); + } + } + names.sort_unstable(); + names.dedup(); + names +} + +fn collect_mapping_keys(value: Option<&serde_yaml::Value>, names: &mut Vec) { + if let Some(mapping) = value.and_then(serde_yaml::Value::as_mapping) { + names.extend(mapping.keys().filter_map(|key| key.as_str().map(str::to_string))); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn manager(client: PackageManagerType, install_dir: &AbsolutePath) -> PackageManager { + PackageManager { + client, + version: "4.12.0".into(), + install_dir: install_dir.to_absolute_path_buf(), + } + } + + fn project(yarnrc: Option<&str>) -> (tempfile::TempDir, vt_path::AbsolutePathBuf) { + let temp_dir = tempfile::tempdir().unwrap(); + let root = vt_path::AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap(); + std::fs::write(root.join("package.json"), "{\"name\": \"test\"}").unwrap(); + if let Some(content) = yarnrc { + std::fs::write(root.join(".yarnrc.yml"), content).unwrap(); + } + (temp_dir, root) + } + + #[test] + fn collects_default_and_named_catalog_entries() { + let (_guard, root) = project(Some( + "catalog:\n vite: npm:@voidzero-dev/vite-plus-core@0.2.9\n vite-plus: 0.2.9\ncatalogs:\n vite7:\n react: ^19.0.0\n", + )); + let names = yarn_catalog_package_names(&root, &manager(PackageManagerType::Yarn, &root)); + + assert_eq!(names, vec!["react", "vite", "vite-plus"]); + } + + #[test] + fn non_yarn_manager_reads_nothing() { + let (_guard, root) = project(Some("catalog:\n vite: ^7.0.0\n")); + let names = yarn_catalog_package_names(&root, &manager(PackageManagerType::Pnpm, &root)); + + assert!(names.is_empty()); + } + + #[test] + fn missing_rc_and_malformed_rc_are_empty() { + let (_guard, root) = project(None); + assert!( + yarn_catalog_package_names(&root, &manager(PackageManagerType::Yarn, &root)).is_empty() + ); + + let (_guard, root) = project(Some(": not yaml [")); + assert!( + yarn_catalog_package_names(&root, &manager(PackageManagerType::Yarn, &root)).is_empty() + ); + } + + #[test] + fn rc_without_catalog_is_empty() { + let (_guard, root) = project(Some("nodeLinker: node-modules\n")); + let names = yarn_catalog_package_names(&root, &manager(PackageManagerType::Yarn, &root)); + + assert!(names.is_empty()); + } +} diff --git a/docs/guide/install.md b/docs/guide/install.md index df4419d148..59910161bb 100644 --- a/docs/guide/install.md +++ b/docs/guide/install.md @@ -148,6 +148,8 @@ Use these commands to maintain the dependency graph over time. - `vp outdated` shows which packages have newer versions available - `vp dedupe` asks the package manager to collapse duplicates where possible +Under a Yarn catalog, `vp update ` skips a name the catalog pins and prints a warning, because `yarn up` would overwrite the `catalog:` reference with a concrete range. Edit the catalog entry in `.yarnrc.yml` to change the version, or pass an explicit range (`vp update vite@^8`) to write a plain range on purpose. + #### Inspect Use these when you need to understand the current state of dependencies. diff --git a/docs/guide/upgrade.md b/docs/guide/upgrade.md index d3ff934d4b..52abd88a67 100644 --- a/docs/guide/upgrade.md +++ b/docs/guide/upgrade.md @@ -73,6 +73,8 @@ If you migrated with `vp migrate`, your project pins `vitest` to an exact versio Under pnpm the managed keys use an explicit `@*` range (`vite@*`, `vitest@*`). pnpm applies an override by replacing the declared spec on every manifest, importer manifests included. A bare key matches any spec, including `catalog:`. The `@*` range keeps the override on the semver ranges that transitive and peer declarations use, and leaves `catalog:` references intact. `vp up` therefore no longer rewrites them to a concrete version. +Under Yarn 4.10+ the toolchain pins live in the `.yarnrc.yml` catalog, and `package.json` references them with `catalog:`. `yarn up ` replaces such a reference with a concrete range, which destroys it. `vp up` therefore skips catalog-pinned names and prints a warning. To change a pinned version, edit the catalog entry in `.yarnrc.yml`, or run `vp migrate` for the pins Vite+ manages. A descriptor with an explicit range (`vp up vite@^8`) still passes through. + A Vite+ release can bump the bundled Vitest. Because that pin also applies to `vite-plus`'s own `vitest` dependency, an out-of-date pin keeps installing the previous runner even after you upgrade `vite-plus` — splitting Vitest's internals (mocks, `expect`, runner state) between the pinned copy and the one `vp test` loads. After upgrading `vite-plus`, re-pin `vitest` to the version Vite+ now bundles. Check that version with: