Skip to content

Commit 04d0f12

Browse files
fengmk2fengmk2
authored
feat(cli): make packages argument required for add/remove commands (#277)
### TL;DR Make package arguments required for `add` and `remove` commands in the CLI. ### What changed? - Added the `required = true` attribute to the `packages` argument in both `add` and `remove` commands - Removed the `NoPackagesSpecified` error from the error enum as it's no longer needed - Removed validation code that checked for empty packages in the `add` and `remove` command implementations - Removed tests that were checking the empty packages case - Updated snap tests to reflect the new required argument behavior - Updated command help text in snap tests to show packages as required (`<PACKAGES>...` instead of `[PACKAGES]...`) - Added a new test case to verify the error when no packages are specified ### How to test? 1. Run `vp add` without any packages and verify it shows an error about missing required arguments 2. Run `vp remove` without any packages and verify it shows an error about missing required arguments 3. Run the snap tests to verify the updated behavior ### Why make this change? This change improves the CLI interface by making it clearer that package names are required for both `add` and `remove` commands. By using Clap's built-in argument validation instead of custom error handling, we get more consistent error messages and better help text. This also simplifies the code by removing unnecessary validation logic and error types.
1 parent 2f39b24 commit 04d0f12

13 files changed

Lines changed: 141 additions & 85 deletions

File tree

‎crates/vite_error/src/lib.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,6 @@ pub enum Error {
9393
#[error("Unrecognized any package manager, please specify the package manager")]
9494
UnrecognizedPackageManager,
9595

96-
#[error("No packages specified. Usage: vite add <packages>...")]
97-
NoPackagesSpecified,
98-
9996
#[error(
10097
"Package manager {name}@{version} in {package_json_path:?} is invalid, expected format: 'package-manager-name@major.minor.patch'"
10198
)]

‎packages/cli/binding/src/cli.rs‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ pub enum Commands {
180180
global: bool,
181181

182182
/// Packages to add
183+
#[arg(required = true)]
183184
packages: Vec<String>,
184185

185186
/// Additional arguments to pass through to the package manager
@@ -218,6 +219,7 @@ pub enum Commands {
218219
global: bool,
219220

220221
/// Packages to remove
222+
#[arg(required = true)]
221223
packages: Vec<String>,
222224

223225
/// Additional arguments to pass through to the package manager
@@ -2009,17 +2011,6 @@ mod tests {
20092011
assert!(args.is_err());
20102012
}
20112013

2012-
#[test]
2013-
fn test_args_remove_command_no_packages() {
2014-
// Remove command should accept empty packages list (package manager will handle this)
2015-
let args = Args::try_parse_from(&["vite-plus", "remove"]).unwrap();
2016-
if let Commands::Remove { packages, .. } = &args.commands {
2017-
assert!(packages.is_empty());
2018-
} else {
2019-
panic!("Expected Remove command");
2020-
}
2021-
}
2022-
20232014
#[test]
20242015
fn test_args_remove_command_complex_scenario() {
20252016
let args = Args::try_parse_from(&[

‎packages/cli/binding/src/commands/add.rs‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ impl AddCommand {
3434
allow_build: Option<&str>,
3535
pass_through_args: Option<&[String]>,
3636
) -> Result<ExitStatus, Error> {
37-
if packages.is_empty() {
38-
return Err(Error::NoPackagesSpecified);
39-
}
40-
4137
let add_command_options = AddCommandOptions {
4238
packages,
4339
save_dependency_type,
@@ -73,20 +69,4 @@ mod tests {
7369
let cmd = AddCommand::new(workspace_root.clone());
7470
assert_eq!(cmd.cwd, workspace_root);
7571
}
76-
77-
#[tokio::test]
78-
async fn test_add_command_no_packages() {
79-
let workspace_root = if cfg!(windows) {
80-
AbsolutePathBuf::new("C:\\test".into()).unwrap()
81-
} else {
82-
AbsolutePathBuf::new("/test".into()).unwrap()
83-
};
84-
85-
let cmd = AddCommand::new(workspace_root);
86-
let result =
87-
cmd.execute(&vec![], None, false, None, None, false, false, false, None, None).await;
88-
89-
assert!(result.is_err());
90-
assert!(matches!(result.unwrap_err(), Error::NoPackagesSpecified));
91-
}
9272
}

‎packages/cli/binding/src/commands/remove.rs‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ impl RemoveCommand {
3030
global: bool,
3131
pass_through_args: Option<&[String]>,
3232
) -> Result<ExitStatus, Error> {
33-
if packages.is_empty() {
34-
return Err(Error::NoPackagesSpecified);
35-
}
36-
3733
// Detect package manager
3834
let package_manager = PackageManager::builder(&self.cwd).build().await?;
3935

@@ -67,20 +63,4 @@ mod tests {
6763
let cmd = RemoveCommand::new(workspace_root.clone());
6864
assert_eq!(cmd.cwd, workspace_root);
6965
}
70-
71-
#[tokio::test]
72-
async fn test_remove_command_no_packages() {
73-
let workspace_root = if cfg!(windows) {
74-
AbsolutePathBuf::new("C:\\test".into()).unwrap()
75-
} else {
76-
AbsolutePathBuf::new("/test".into()).unwrap()
77-
};
78-
79-
let cmd = RemoveCommand::new(workspace_root);
80-
let result =
81-
cmd.execute(&vec![], false, false, false, None, false, false, false, None).await;
82-
83-
assert!(result.is_err());
84-
assert!(matches!(result.unwrap_err(), Error::NoPackagesSpecified));
85-
}
8666
}

‎packages/global/snap-tests/command-add-npm10/snap.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
> vp add --help # should show help
22
Add packages to dependencies
33

4-
Usage: vp add [OPTIONS] [PACKAGES]... [-- <PASS_THROUGH_ARGS>...]
4+
Usage: vp add [OPTIONS] <PACKAGES>... [-- <PASS_THROUGH_ARGS>...]
55

66
Arguments:
7-
[PACKAGES]... Packages to add
7+
<PACKAGES>... Packages to add
88
[PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager
99

1010
Options:

‎packages/global/snap-tests/command-add-npm11/snap.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
> vp add --help # should show help
22
Add packages to dependencies
33

4-
Usage: vp add [OPTIONS] [PACKAGES]... [-- <PASS_THROUGH_ARGS>...]
4+
Usage: vp add [OPTIONS] <PACKAGES>... [-- <PASS_THROUGH_ARGS>...]
55

66
Arguments:
7-
[PACKAGES]... Packages to add
7+
<PACKAGES>... Packages to add
88
[PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager
99

1010
Options:

‎packages/global/snap-tests/command-add-pnpm10/snap.txt‎

Lines changed: 114 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
> vp add --help # should show help
22
Add packages to dependencies
33

4-
Usage: vp add [OPTIONS] [PACKAGES]... [-- <PASS_THROUGH_ARGS>...]
4+
Usage: vp add [OPTIONS] <PACKAGES>... [-- <PASS_THROUGH_ARGS>...]
55

66
Arguments:
7-
[PACKAGES]... Packages to add
7+
<PACKAGES>... Packages to add
88
[PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager
99

1010
Options:
@@ -35,6 +35,14 @@ Options:
3535
-h, --help
3636
Print help
3737

38+
[2]> vp add # should error because no packages specified
39+
error: the following required arguments were not provided:
40+
<PACKAGES>...
41+
42+
Usage: vp add <PACKAGES>... [-- <PASS_THROUGH_ARGS>...]
43+
44+
For more information, try '--help'.
45+
3846
> vp add testnpm2 urllib -D -- --loglevel=verbose --verbose && cat package.json # should add package as dev dependencies
3947
Running: pnpm add --save-dev --loglevel=verbose --verbose testnpm2 urllib
4048
Packages: +<variable>
@@ -56,8 +64,107 @@ Done in <variable>ms using pnpm v<semver>
5664
}
5765
}
5866

59-
> #vp add testnpm2 test-vite-plus-install --allow-build=test-vite-plus-install && cat package.json # should add packages to dependencies
60-
> #vp install test-vite-plus-package@1.0.0 --save-peer && cat package.json # should install package alias for add
61-
> #vp add test-vite-plus-package-optional -O && cat package.json # should add package as optional dependencies
62-
> #vp add test-vite-plus-package-optional -- --loglevel=warn && cat package.json # support pass through arguments
63-
> #vp add -g testnpm2 -- --dry-run && cat package.json # support add global package with dry-run
67+
> vp add testnpm2 test-vite-plus-install --allow-build=test-vite-plus-install && cat package.json # should add packages to dependencies
68+
Running: pnpm add --allow-build=test-vite-plus-install testnpm2 test-vite-plus-install
69+
Packages: +<variable>
70+
+<repeat>
71+
Progress: resolved <variable>, reused <variable>, downloaded <variable>, added <variable>, done
72+
73+
dependencies:
74+
+ test-vite-plus-install <semver>
75+
76+
Done in <variable>ms using pnpm v<semver>
77+
{
78+
"name": "command-add-pnpm10",
79+
"version": "1.0.0",
80+
"packageManager": "pnpm@<semver>",
81+
"devDependencies": {
82+
"testnpm2": "^1.0.1",
83+
"urllib": "^4.8.2"
84+
},
85+
"dependencies": {
86+
"test-vite-plus-install": "^1.0.0"
87+
}
88+
}
89+
90+
> vp install test-vite-plus-package@1.0.0 --save-peer && cat package.json # should install package alias for add
91+
Running: pnpm add --save-peer test-vite-plus-package@<semver>
92+
Packages: +<variable>
93+
+<repeat>
94+
Progress: resolved <variable>, reused <variable>, downloaded <variable>, added <variable>, done
95+
96+
peerDependencies:
97+
+ test-vite-plus-package <semver>
98+
99+
devDependencies:
100+
+ test-vite-plus-package <semver> already in devDependencies, was not moved to dependencies.
101+
102+
Done in <variable>ms using pnpm v<semver>
103+
{
104+
"name": "command-add-pnpm10",
105+
"version": "1.0.0",
106+
"packageManager": "pnpm@<semver>",
107+
"devDependencies": {
108+
"test-vite-plus-package": "1.0.0",
109+
"testnpm2": "^1.0.1",
110+
"urllib": "^4.8.2"
111+
},
112+
"dependencies": {
113+
"test-vite-plus-install": "^1.0.0"
114+
},
115+
"peerDependencies": {
116+
"test-vite-plus-package": "1.0.0"
117+
}
118+
}
119+
120+
> vp add test-vite-plus-package-optional -O && cat package.json # should add package as optional dependencies
121+
Running: pnpm add --save-optional test-vite-plus-package-optional
122+
Packages: +<variable>
123+
+<repeat>
124+
Progress: resolved <variable>, reused <variable>, downloaded <variable>, added <variable>, done
125+
126+
optionalDependencies:
127+
+ test-vite-plus-package-optional <semver>
128+
129+
Done in <variable>ms using pnpm v<semver>
130+
{
131+
"name": "command-add-pnpm10",
132+
"version": "1.0.0",
133+
"packageManager": "pnpm@<semver>",
134+
"devDependencies": {
135+
"test-vite-plus-package": "1.0.0",
136+
"testnpm2": "^1.0.1",
137+
"urllib": "^4.8.2"
138+
},
139+
"dependencies": {
140+
"test-vite-plus-install": "^1.0.0"
141+
},
142+
"peerDependencies": {
143+
"test-vite-plus-package": "1.0.0"
144+
},
145+
"optionalDependencies": {
146+
"test-vite-plus-package-optional": "^1.0.0"
147+
}
148+
}
149+
150+
> vp add test-vite-plus-package-optional -- --loglevel=warn && cat package.json # support pass through arguments
151+
Running: pnpm add --loglevel=warn test-vite-plus-package-optional
152+
{
153+
"name": "command-add-pnpm10",
154+
"version": "1.0.0",
155+
"packageManager": "pnpm@<semver>",
156+
"devDependencies": {
157+
"test-vite-plus-package": "1.0.0",
158+
"testnpm2": "^1.0.1",
159+
"urllib": "^4.8.2"
160+
},
161+
"dependencies": {
162+
"test-vite-plus-install": "^1.0.0"
163+
},
164+
"peerDependencies": {
165+
"test-vite-plus-package": "1.0.0"
166+
},
167+
"optionalDependencies": {
168+
"test-vite-plus-package-optional": "^1.0.0"
169+
}
170+
}

‎packages/global/snap-tests/command-add-pnpm10/steps.json‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
},
55
"commands": [
66
"vp add --help # should show help",
7+
"vp add # should error because no packages specified",
78
"vp add testnpm2 urllib -D -- --loglevel=verbose --verbose && cat package.json # should add package as dev dependencies",
8-
"#vp add testnpm2 test-vite-plus-install --allow-build=test-vite-plus-install && cat package.json # should add packages to dependencies",
9-
"#vp install test-vite-plus-package@1.0.0 --save-peer && cat package.json # should install package alias for add",
10-
"#vp add test-vite-plus-package-optional -O && cat package.json # should add package as optional dependencies",
11-
"#vp add test-vite-plus-package-optional -- --loglevel=warn && cat package.json # support pass through arguments",
12-
"#vp add -g testnpm2 -- --dry-run && cat package.json # support add global package with dry-run"
9+
"vp add testnpm2 test-vite-plus-install --allow-build=test-vite-plus-install && cat package.json # should add packages to dependencies",
10+
"vp install test-vite-plus-package@1.0.0 --save-peer && cat package.json # should install package alias for add",
11+
"vp add test-vite-plus-package-optional -O && cat package.json # should add package as optional dependencies",
12+
"vp add test-vite-plus-package-optional -- --loglevel=warn && cat package.json # support pass through arguments"
1313
]
1414
}

‎packages/global/snap-tests/command-add-pnpm9/snap.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
> vp add --help # should show help
22
Add packages to dependencies
33

4-
Usage: vp add [OPTIONS] [PACKAGES]... [-- <PASS_THROUGH_ARGS>...]
4+
Usage: vp add [OPTIONS] <PACKAGES>... [-- <PASS_THROUGH_ARGS>...]
55

66
Arguments:
7-
[PACKAGES]... Packages to add
7+
<PACKAGES>... Packages to add
88
[PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager
99

1010
Options:

‎packages/global/snap-tests/command-add-yarn4/snap.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
> vp add --help # should show help
22
Add packages to dependencies
33

4-
Usage: vp add [OPTIONS] [PACKAGES]... [-- <PASS_THROUGH_ARGS>...]
4+
Usage: vp add [OPTIONS] <PACKAGES>... [-- <PASS_THROUGH_ARGS>...]
55

66
Arguments:
7-
[PACKAGES]... Packages to add
7+
<PACKAGES>... Packages to add
88
[PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager
99

1010
Options:

0 commit comments

Comments
 (0)