From 480b1e76263fa1745a7523e45a568675ef74b938 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 14 Sep 2026 15:09:26 +0900 Subject: [PATCH] fix(pm): reject filtered add and remove on Yarn Classic --- .../package.json | 9 +++ .../packages/web/package.json | 7 ++ .../snapshots.toml | 12 +++ ...mand_yarn_classic_filtered_dependencies.md | 78 +++++++++++++++++++ .../vp_pm_cli/src/resolution/commands/add.rs | 30 ++++++- .../src/resolution/commands/remove.rs | 53 ++++++++----- 6 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/packages/web/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots.toml create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots/command_yarn_classic_filtered_dependencies.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/package.json new file mode 100644 index 0000000000..d4acbd2233 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/package.json @@ -0,0 +1,9 @@ +{ + "name": "command-yarn-classic-filtered-dependencies", + "private": true, + "packageManager": "yarn@1.22.22", + "workspaces": ["packages/*"], + "dependencies": { + "lodash": "4.17.21" + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/packages/web/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/packages/web/package.json new file mode 100644 index 0000000000..f7d91288ab --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/packages/web/package.json @@ -0,0 +1,7 @@ +{ + "name": "@example/app", + "version": "1.0.0", + "dependencies": { + "lodash": "4.17.21" + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots.toml new file mode 100644 index 0000000000..728dd382ca --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots.toml @@ -0,0 +1,12 @@ +[[case]] +name = "command_yarn_classic_filtered_dependencies" +vp = "global" +steps = [ + { argv = ["vp", "add", "react", "--filter", "@example/app"], comment = "Classic filtered add reports an unsupported option instead of invoking foreach", continue-on-failure = true }, + { argv = ["vp", "install", "react", "--filter", "@example/app"], comment = "install with packages uses the same add guard", continue-on-failure = true }, + { argv = ["vp", "remove", "lodash", "--filter", "@example/app"], comment = "Classic filtered remove fails before changing dependencies", continue-on-failure = true }, + { argv = ["vp", "remove", "lodash", "--filter", "@example/*", "--filter", "other", "--recursive"], comment = "recursive remove must not silently discard Classic filters", continue-on-failure = true }, + { argv = ["vpt", "print-file", "package.json", "packages/web/package.json"], comment = "root and workspace manifests remain unchanged", continue-on-failure = true }, + ["vpt", "stat-file", "yarn.lock", "--assert", "missing"], + ["vpt", "stat-file", "node_modules", "--assert", "missing"], +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots/command_yarn_classic_filtered_dependencies.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots/command_yarn_classic_filtered_dependencies.md new file mode 100644 index 0000000000..55cfb8cd1a --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_yarn_classic_filtered_dependencies/snapshots/command_yarn_classic_filtered_dependencies.md @@ -0,0 +1,78 @@ +# command_yarn_classic_filtered_dependencies + +## `vp add react --filter @example/app` + +Classic filtered add reports an unsupported option instead of invoking foreach + +**Exit code:** 1 + +``` +Invalid argument: `--filter` is not supported by Yarn Classic `add`. +``` + +## `vp install react --filter @example/app` + +install with packages uses the same add guard + +**Exit code:** 1 + +``` +VITE+ - The Unified Toolchain for the Web + +Invalid argument: `--filter` is not supported by Yarn Classic `add`. +``` + +## `vp remove lodash --filter @example/app` + +Classic filtered remove fails before changing dependencies + +**Exit code:** 1 + +``` +Invalid argument: `--filter` is not supported by Yarn Classic `remove`. +``` + +## `vp remove lodash --filter @example/* --filter other --recursive` + +recursive remove must not silently discard Classic filters + +**Exit code:** 1 + +``` +Invalid argument: `--filter` is not supported by Yarn Classic `remove`. +``` + +## `vpt print-file package.json packages/web/package.json` + +root and workspace manifests remain unchanged + +``` +{ + "name": "command-yarn-classic-filtered-dependencies", + "private": true, + "packageManager": "yarn@1.22.22", + "workspaces": ["packages/*"], + "dependencies": { + "lodash": "4.17.21" + } +} +{ + "name": "@example/app", + "version": "1.0.0", + "dependencies": { + "lodash": "4.17.21" + } +} +``` + +## `vpt stat-file yarn.lock --assert missing` + +``` +yarn.lock: missing +``` + +## `vpt stat-file node_modules --assert missing` + +``` +node_modules: missing +``` diff --git a/crates/vp_pm_cli/src/resolution/commands/add.rs b/crates/vp_pm_cli/src/resolution/commands/add.rs index 683c7a849e..a7c4f55e26 100644 --- a/crates/vp_pm_cli/src/resolution/commands/add.rs +++ b/crates/vp_pm_cli/src/resolution/commands/add.rs @@ -209,6 +209,13 @@ impl Resolve for Yarn { let mut cmd = CommandBuilder::new("yarn"); if !args.filter.is_empty() { + if !self.is_berry() { + return CommandResolution::InvalidArgument( + "Invalid argument: `--filter` is not supported by Yarn Classic `add`." + .to_string(), + ); + } + cmd.arg("workspaces").arg("foreach").arg("--all"); cmd.repeated("--include", args.filter.iter()); } @@ -433,10 +440,10 @@ mod tests { } #[test] - fn test_yarn_add_with_workspace() { + fn test_yarn_berry_add_with_workspace() { let mut options = add_args(&["react"]); options.filter = vec!["app".to_string()]; - let resolution = resolve(&yarn("1.22.22"), options); + let resolution = resolve(&yarn("4.0.0"), options); let command = expect_run(resolution.outcome); assert_eq!(command.program, "yarn"); @@ -446,6 +453,25 @@ mod tests { ); } + #[test] + fn test_yarn_classic_rejects_filtered_add() { + for filters in + [vec!["app".to_string()], vec!["app-*".to_string(), "@scope/web".to_string()]] + { + let mut options = add_args(&["react"]); + options.filter = filters; + let resolution = resolve(&yarn("1.22.22"), options); + + assert_eq!( + resolution.outcome, + CommandResolution::InvalidArgument( + "Invalid argument: `--filter` is not supported by Yarn Classic `add`." + .to_string() + ) + ); + } + } + #[test] fn test_yarn_add_workspace_root() { let mut options = add_args(&["typescript"]); diff --git a/crates/vp_pm_cli/src/resolution/commands/remove.rs b/crates/vp_pm_cli/src/resolution/commands/remove.rs index 4635f9637e..0888888a68 100644 --- a/crates/vp_pm_cli/src/resolution/commands/remove.rs +++ b/crates/vp_pm_cli/src/resolution/commands/remove.rs @@ -103,9 +103,18 @@ impl Resolve for Yarn { } let mut cmd = CommandBuilder::new("yarn"); - if !args.filter.is_empty() && !args.recursive { - cmd.arg("workspaces").arg("foreach").arg("--all"); - cmd.repeated("--include", args.filter.iter()); + if !args.filter.is_empty() { + if !self.is_berry() { + return CommandResolution::InvalidArgument( + "Invalid argument: `--filter` is not supported by Yarn Classic `remove`." + .to_string(), + ); + } + + if !args.recursive { + cmd.arg("workspaces").arg("foreach").arg("--all"); + cmd.repeated("--include", args.filter.iter()); + } } cmd.arg("remove") .arg_if("--all", args.recursive) @@ -224,17 +233,25 @@ mod tests { } #[test] - fn test_yarn_remove_with_workspace() { - let mut options = remove_args(&["lodash"]); - options.filter = vec!["app".to_string()]; - let resolution = resolve(&yarn("1.22.0"), options); - let command = expect_run(resolution.outcome); - - assert_eq!(command.program, "yarn"); - assert_eq!( - command.args, - vec!["workspaces", "foreach", "--all", "--include", "app", "remove", "lodash"] - ); + fn test_yarn_classic_rejects_filtered_remove() { + for filters in + [vec!["app".to_string()], vec!["app-*".to_string(), "@scope/web".to_string()]] + { + for recursive in [false, true] { + let mut options = remove_args(&["lodash"]); + options.filter = filters.clone(); + options.recursive = recursive; + let resolution = resolve(&yarn("1.22.22"), options); + + assert_eq!( + resolution.outcome, + CommandResolution::InvalidArgument( + "Invalid argument: `--filter` is not supported by Yarn Classic `remove`." + .to_string() + ) + ); + } + } } #[test] @@ -456,10 +473,10 @@ mod tests { } #[test] - fn test_yarn_remove_with_multiple_filters() { + fn test_yarn_berry_remove_with_multiple_filters() { let mut options = remove_args(&["lodash"]); options.filter = vec!["app".to_string(), "web".to_string()]; - let resolution = resolve(&yarn("1.22.0"), options); + let resolution = resolve(&yarn("4.0.0"), options); let command = expect_run(resolution.outcome); assert_eq!(command.program, "yarn"); @@ -480,11 +497,11 @@ mod tests { } #[test] - fn test_yarn_remove_with_recursive_and_multiple_filters() { + fn test_yarn_berry_remove_with_recursive_and_multiple_filters() { let mut options = remove_args(&["lodash"]); options.filter = vec!["app".to_string(), "web".to_string()]; options.recursive = true; - let resolution = resolve(&yarn("1.22.0"), options); + let resolution = resolve(&yarn("4.0.0"), options); let command = expect_run(resolution.outcome); assert_eq!(command.program, "yarn");