Skip to content

Commit 4b6e3e5

Browse files
committed
fix(cli): address Codex review round 4
- vp/vpr script commands with a leading -C run verbatim instead of being parsed as CLIArgs (which has no global flags): the spawned binary applies the directory change exactly like a direct invocation - bare app commands in scripts get the same workspace-root target elicitation as direct invocations via a pure needs_elicitation predicate: the handler spawns the real binary (which elicits identically) instead of synthesizing a silent root run - add --configLoader/--config-loader/--tsconfig/--log-level/ --deps.never-bundle to the required-value flag list - PTY regression cases: a root script 'build: vp build' fails with the listing through vp run, and a 'vp -C' script runs in the target package Shell-integration template gaps (env-use wrapper, vpr completers) are tracked separately in #2056.
1 parent 50a6b75 commit 4b6e3e5

10 files changed

Lines changed: 151 additions & 2 deletions

File tree

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
1-
{ "name": "app-root-listing", "private": true, "workspaces": ["apps/*", "packages/*"] }
1+
{
2+
"name": "app-root-listing",
3+
"private": true,
4+
"workspaces": [
5+
"apps/*",
6+
"packages/*"
7+
],
8+
"scripts": {
9+
"build": "vp build"
10+
}
11+
}

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/app_root_listing/snapshots.toml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,13 @@ steps = [
3838
{ "write-key" = "ctrl-c" },
3939
] },
4040
]
41+
42+
[[case]]
43+
name = "script_at_root_elicits"
44+
vp = ["local", "global"]
45+
comment = """
46+
A root package script "build": "vp build" run through vp run gets the same
47+
target elicitation as a direct bare invocation: the spawned vp prints the
48+
listing and the task fails instead of silently building the root.
49+
"""
50+
steps = [{ argv = ["vp", "run", "build"], tty = false }]
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# script_at_root_elicits
2+
3+
A root package script "build": "vp build" run through vp run gets the same
4+
target elicitation as a direct bare invocation: the spawned vp prints the
5+
listing and the task fails instead of silently building the root.
6+
7+
## `vp run build`
8+
9+
**Exit code:** 1
10+
11+
```
12+
$ vp build ⊘ cache disabled
13+
14+
error: `vp build` at the workspace root needs a target package.
15+
16+
Packages in this workspace:
17+
admin apps/admin
18+
web apps/web
19+
ui packages/ui
20+
21+
Pass a directory: vp -C apps/admin build
22+
Or run every package's build script: vp run -r build
23+
```
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# script_at_root_elicits
2+
3+
A root package script "build": "vp build" run through vp run gets the same
4+
target elicitation as a direct bare invocation: the spawned vp prints the
5+
listing and the task fails instead of silently building the root.
6+
7+
## `vp run build`
8+
9+
**Exit code:** 1
10+
11+
```
12+
$ vp build ⊘ cache disabled
13+
14+
error: `vp build` at the workspace root needs a target package.
15+
16+
Packages in this workspace:
17+
admin apps/admin
18+
web apps/web
19+
ui packages/ui
20+
21+
Pass a directory: vp -C apps/admin build
22+
Or run every package's build script: vp run -r build
23+
```
Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,10 @@
1-
{ "name": "cwd-flag", "private": true, "workspaces": ["packages/*"] }
1+
{
2+
"name": "cwd-flag",
3+
"private": true,
4+
"workspaces": [
5+
"packages/*"
6+
],
7+
"scripts": {
8+
"where:hello": "vp -C packages/hello run where"
9+
}
10+
}

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/cwd_flag/snapshots.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ steps = [
1717
{ argv = ["vp", "-C", "packages/missing", "build"], comment = "missing directory errors" },
1818
{ argv = ["vp", "pack", "packages/hello"], comment = "positional stays a tsdown entry resolved from the invocation directory" },
1919
{ argv = ["vpt", "list-dir", "dist"], comment = "upstream semantics: output lands at the invocation directory" },
20+
{ argv = ["vp", "run", "where:hello"], comment = "a script whose command starts with vp -C runs verbatim and honors the directory" },
2021
]

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/cwd_flag/snapshots/cwd_flag.global.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,17 @@ upstream semantics: output lands at the invocation directory
101101
```
102102
hello.mjs
103103
```
104+
105+
## `vp run where:hello`
106+
107+
a script whose command starts with vp -C runs verbatim and honors the directory
108+
109+
```
110+
VITE+ - The Unified Toolchain for the Web
111+
112+
$ vp -C packages/hello run where ⊘ cache disabled
113+
VITE+ - The Unified Toolchain for the Web
114+
115+
~/packages/hello$ node -e "console.log('cwd base: ' + require('node:path').basename(process.cwd()))" ⊘ cache disabled
116+
cwd base: hello
117+
```

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/cwd_flag/snapshots/cwd_flag.local.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,13 @@ upstream semantics: output lands at the invocation directory
9191
```
9292
hello.mjs
9393
```
94+
95+
## `vp run where:hello`
96+
97+
a script whose command starts with vp -C runs verbatim and honors the directory
98+
99+
```
100+
$ vp -C packages/hello run where ⊘ cache disabled
101+
~/packages/hello$ node -e "console.log('cwd base: ' + require('node:path').basename(process.cwd()))" ⊘ cache disabled
102+
cwd base: hello
103+
```

packages/cli/binding/src/cli/app_target.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ const VALUE_TAKING_FLAGS: &[&str] = &[
6262
"--assetsDir",
6363
"--assetsInlineLimit",
6464
"--filter",
65+
"--configLoader",
66+
"--config-loader",
67+
"--tsconfig",
68+
"--log-level",
69+
"--deps.never-bundle",
6570
"-d",
6671
"--out-dir",
6772
"--target",
@@ -190,6 +195,31 @@ fn run_package_picker(command: &str, rows: &[PackageRow]) -> Result<Option<usize
190195
})
191196
}
192197

198+
/// Pure predicate for the vp-script interception: would `resolve_app_target`
199+
/// do anything other than run in `cwd`? Never prints and never runs the
200+
/// picker. Slightly over-approximates (an empty workspace reports true), in
201+
/// which case the script merely spawns the real binary, which then behaves
202+
/// identically to a direct invocation.
203+
pub(super) fn needs_elicitation(
204+
subcommand: &SynthesizableSubcommand,
205+
cwd: &AbsolutePathBuf,
206+
) -> bool {
207+
let Some((_, args)) = app_command_parts(subcommand) else {
208+
return false;
209+
};
210+
if !is_bare(args) {
211+
return false;
212+
}
213+
if vite_static_config::resolve_static_config(cwd).get_declared("defaultPackage").is_some() {
214+
return true;
215+
}
216+
let Ok((workspace_root, rel_from_root)) = vite_workspace::find_workspace_root(cwd) else {
217+
return false;
218+
};
219+
rel_from_root.as_str().is_empty()
220+
&& !matches!(workspace_root.workspace_file, WorkspaceFile::NonWorkspacePackage(_))
221+
}
222+
193223
pub(super) fn resolve_app_target(
194224
subcommand: &SynthesizableSubcommand,
195225
cwd: &AbsolutePathBuf,

packages/cli/binding/src/cli/handler.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ impl CommandHandler for VitePlusCommandHandler {
4444
if program != "vp" && program != "vpr" {
4545
return Ok(HandledCommand::Verbatim);
4646
}
47+
// A leading global `-C` runs verbatim: CLIArgs has no global flags, and
48+
// the spawned vp/vpr binary applies the directory change (and target
49+
// elicitation) exactly like a direct invocation.
50+
if command
51+
.args
52+
.first()
53+
.map(Str::as_str)
54+
.is_some_and(|arg| arg == "-C" || (arg.starts_with("-C") && arg.len() > 2))
55+
{
56+
return Ok(HandledCommand::Verbatim);
57+
}
4758
// "vpr <args>" is shorthand for "vp run <args>", so prepend "run" for parsing.
4859
let is_vpr = program == "vpr";
4960
let cli_args = match CLIArgs::try_parse_from(
@@ -72,6 +83,14 @@ impl CommandHandler for VitePlusCommandHandler {
7283
)))
7384
}
7485
CLIArgs::Synthesizable(subcmd) => {
86+
// Bare app commands in scripts get the same workspace-root
87+
// target elicitation as direct invocations: spawn the real
88+
// binary, which elicits (defaultPackage note, listing +
89+
// exit 1) identically instead of silently running the root.
90+
let cwd = command.cwd.to_absolute_path_buf();
91+
if super::app_target::needs_elicitation(&subcmd, &cwd) {
92+
return Ok(HandledCommand::Verbatim);
93+
}
7594
let resolved = self.resolver.resolve(subcmd, None, &command.envs).await?;
7695
Ok(HandledCommand::Synthesized(resolved.into_synthetic_plan_request()))
7796
}

0 commit comments

Comments
 (0)