Skip to content

Commit 401eb74

Browse files
committed
docs(rfc): default to node_modules/.bin/vp, not the package-relative path
Reviewer correctly pointed out that node_modules/.bin/<name> is the conventional path editor extensions already use for oxlint/oxfmt, so the TypeScript reference should target that by default. The previous choice was Zed-driven and shouldn't be imposed on the Node consumers. Changes: - Phase 2 in the algorithm: validate via node_modules/vite-plus/ package.json, then resolve node_modules/.bin/vp (with Windows .cmd and .exe fallbacks). - Reference TypeScript updated; resolveVpAt now returns the .bin shim rather than the package-relative path. - Mermaid diagram: package-validation node first, then the .bin/vp existence check. - Per-extension migration plan: each extension targets whatever shim path matches its existing oxlint/oxfmt pattern. - oxc-vscode, coc-oxc → node_modules/.bin/vp - oxc-zed → node_modules/vite-plus/bin/vp (Zed avoids .bin for WASM reasons; lsp.rs:47) - oxc-intellij-plugin → <vite-plus>/bin/vp via NodePackageDescriptor - Conformance fixtures: vpPath values now show the .bin/vp path (the TypeScript reference's output); a clarifying note explains that ports targeting the package-relative path substitute their own equivalent.
1 parent b8c4d8b commit 401eb74

1 file changed

Lines changed: 65 additions & 39 deletions

File tree

rfcs/editor-extension-vite-plus-detection.md

Lines changed: 65 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ install hoisted from an unrelated dependency tree.
4444
> workspace folder may sit at, inside, or alongside a root workspace.
4545
4646
The runnable `vp` binary is resolved separately: walk up from the
47-
declaring ancestor for `node_modules/vite-plus/bin/vp` with a sibling
48-
`package.json` that parses and has `name === "vite-plus"`, bounded by
49-
the root workspace.
47+
declaring ancestor and, at each ancestor inside the root workspace,
48+
validate `node_modules/vite-plus/package.json` (parses, `name ===
49+
"vite-plus"`) then return `node_modules/.bin/vp` (the package
50+
manager's shim — same path the extensions already use for
51+
`oxlint`/`oxfmt`). On Windows, `.bin/vp.cmd` is the equivalent shim.
5052

5153
```
5254
fn detect_vite_plus_project(start: AbsolutePath) -> Option<Result>:
@@ -109,14 +111,13 @@ flowchart TD
109111
P1Bound -- yes --> ResultNull(["return null"])
110112
111113
P1Found --> P2Begin[/"PHASE 2<br/>find runnable binary<br/>probe = root"/]
112-
P2Begin --> P2Check["check probe/node_modules/vite-plus/bin/vp"]
113-
P2Check --> P2Exists{"binary exists?"}
114-
P2Exists -- yes --> P2Valid{"node_modules/vite-plus/package.json<br/>parses with name = 'vite-plus'?"}
115-
P2Valid -- yes --> ResultRunnable(["return { root, vpPath }"])
116-
P2Valid -- no, orphan --> P2Bound
117-
P2Exists -- no --> P2Bound{"probe is root workspace<br/>or filesystem root?"}
114+
P2Begin --> P2Valid{"probe/node_modules/vite-plus/package.json<br/>parses with name = 'vite-plus'?"}
115+
P2Valid -- yes --> P2Shim{"probe/node_modules/.bin/vp<br/>(or vp.cmd on Windows)<br/>exists?"}
116+
P2Shim -- yes --> ResultRunnable(["return { root, vpPath }"])
117+
P2Shim -- no --> P2Bound
118+
P2Valid -- no, orphan --> P2Bound{"probe is root workspace<br/>or filesystem root?"}
118119
P2Bound -- no --> P2Up["probe = parent(probe)"]
119-
P2Up --> P2Check
120+
P2Up --> P2Valid
120121
P2Bound -- yes --> ResultDeclared(["return { root }"])
121122
```
122123

@@ -186,10 +187,13 @@ function declaresVitePlus(pkg: any | null): boolean {
186187
return Boolean(pkg?.dependencies?.['vite-plus'] || pkg?.devDependencies?.['vite-plus']);
187188
}
188189

189-
/** `bin/vp` exists AND the sibling package.json identifies as vite-plus. */
190+
/**
191+
* Validate that `node_modules/vite-plus` at `dir` is a real vite-plus
192+
* install, then return the package manager's `.bin/vp` shim path.
193+
* The shim is the conventional entry point that extensions already
194+
* use for oxlint/oxfmt.
195+
*/
190196
function resolveVpAt(dir: string): string | null {
191-
const vpPath = join(dir, 'node_modules', 'vite-plus', 'bin', 'vp');
192-
if (!existsSync(vpPath)) return null;
193197
try {
194198
const pkg = JSON.parse(
195199
readFileSync(join(dir, 'node_modules', 'vite-plus', 'package.json'), 'utf8'),
@@ -198,7 +202,15 @@ function resolveVpAt(dir: string): string | null {
198202
} catch {
199203
return null;
200204
}
201-
return vpPath;
205+
const binDir = join(dir, 'node_modules', '.bin');
206+
const candidates =
207+
process.platform === 'win32'
208+
? [join(binDir, 'vp.cmd'), join(binDir, 'vp.exe'), join(binDir, 'vp')]
209+
: [join(binDir, 'vp')];
210+
for (const candidate of candidates) {
211+
if (existsSync(candidate)) return candidate;
212+
}
213+
return null;
202214
}
203215

204216
export function detectVitePlusProjectSync(start: string): DetectResult | null {
@@ -249,42 +261,56 @@ All four extensions run the detector first, then:
249261
launch failure, surface an upgrade hint.
250262
- `{ root }` → surface an install hint; do not launch anything Vite+.
251263

252-
Specifics:
264+
**On the launch path.** Validation (parsing
265+
`node_modules/vite-plus/package.json` and checking `name`) is the
266+
same everywhere. The path returned for spawning differs by extension,
267+
mirroring whatever pattern that extension already uses for
268+
`oxlint`/`oxfmt`:
253269

254-
- **`oxc-vscode`, `coc-oxc`** — call the detector before the existing
270+
- **`oxc-vscode`, `coc-oxc`** — target `node_modules/.bin/vp` (with
271+
`.cmd`/`.exe` Windows variants), the same shim path they already
272+
use for `oxlint`. Call the detector before the existing
255273
`findBinary("oxlint" | "oxfmt", ...)` chain. Do **not** parameterize
256274
the existing chain with `"vp"` as a target — its
257275
`searchSettingsBin`, `searchGlobalNodeModulesBin`, `searchEnvPath`,
258-
and `require.resolve` paths can escape the workspace boundary or
276+
and `require.resolve` paths can escape the root workspace or
259277
consult settings meant for oxlint/oxfmt.
260-
- **`oxc-zed`** — replace the `[package_name, "vite-plus"]` loop at
278+
- **`oxc-zed`** — keep targeting `node_modules/vite-plus/bin/vp` (the
279+
pattern Zed already uses, `src/lsp.rs:47`, because pnpm's `.bin`
280+
shell shims aren't suitable for Zed's headless WASM execution
281+
context). Replace the `[package_name, "vite-plus"]` loop at
261282
`lsp.rs:28` with the two-phase check ported into Rust. Update
262283
`language_server_command` to pass `["lint", "--lsp"]` /
263-
`["fmt", "--lsp"]` when launching `vp`. Zed's WASM API only reads
264-
the worktree root, so deeper walk-up is a known limitation worth
265-
noting in the Zed PR.
284+
`["fmt", "--lsp"]`. Zed's WASM API only reads the worktree root, so
285+
deeper walk-up is a known limitation worth noting in the Zed PR.
266286
- **`oxc-intellij-plugin`**`VitePlusPackage.kt` already locates
267-
`vite-plus` via IntelliJ's `NodePackageDescriptor`. Tighten it to
268-
require a direct dep, change the returned path from
269-
`<vite-plus>/bin/oxlint` to `<vite-plus>/bin/vp`, update launch
270-
args.
287+
`vite-plus` via IntelliJ's `NodePackageDescriptor` and appends
288+
`bin/<name>`. Tighten it to require a direct dep, change the
289+
returned path from `<vite-plus>/bin/oxlint` to `<vite-plus>/bin/vp`,
290+
update launch args.
271291

272292
## Conformance fixtures
273293

274-
Every implementation must produce identical answers on these
275-
fixtures. Each extension replicates the set in its own test suite.
276-
277-
| Fixture | Tree | Expected result |
278-
| --------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- |
279-
| `root-declared-and-installed` | Root `package.json` declares `vite-plus` + valid `node_modules/vite-plus/` install | `{ root: "<repo>", vpPath: "<repo>/node_modules/vite-plus/bin/vp" }` |
280-
| `pnpm-subpackage-declared-root-hoisted` | `pnpm-workspace.yaml` at `<repo>`, `packages/app/package.json` declares `vite-plus`, install hoisted to `<repo>/node_modules/vite-plus/` | From `packages/app/`: `{ root: "<repo>/packages/app", vpPath: "<repo>/node_modules/vite-plus/bin/vp" }` |
281-
| `npm-subpackage-direct-dep-unhoisted` | Root `package.json` with `workspaces`, `packages/app/package.json` declares `vite-plus`, install inside `packages/app/node_modules/vite-plus/` | From `packages/app/`: `{ root: "<repo>/packages/app", vpPath: "<repo>/packages/app/node_modules/vite-plus/bin/vp" }` |
282-
| `root-declared-no-install` | Root `package.json` declares `vite-plus`, no `node_modules` (fresh clone) | `{ root: "<repo>" }` — install hint |
283-
| `transitive-install` | No walked-up `package.json` declares `vite-plus`, but `node_modules/vite-plus/` exists as a transitive dep | `null` — no direct declaration |
284-
| `bin-vp-orphan` | Declared in root `package.json`, but `node_modules/vite-plus/` is broken (missing `package.json`, wrong `name`, or unparseable) | `{ root: "<repo>" }` — install rejected as orphan |
285-
| `parent-vite-plus-nested-repo` | Outer dir declares + installs `vite-plus`; inner subdir is its own root workspace and does not | From inside the nested workspace: `null` |
286-
| `plain-non-vite-plus` | A normal Node project, no `vite-plus` anywhere | `null` |
287-
| `yarn4-pnp` | Berry/PnP, no `node_modules`, root `package.json` declares `vite-plus` | `{ root: "<repo>" }` — install hint |
294+
Every implementation must produce identical `root` values and the
295+
same null vs. non-null determination on these fixtures.
296+
297+
The `vpPath` values below show the path the **TypeScript reference**
298+
produces (`node_modules/.bin/vp`); ports that target
299+
`node_modules/vite-plus/bin/vp` instead — Zed today, possibly
300+
IntelliJ — substitute their own equivalent. The fixture just asserts
301+
"vpPath is set and runs the validated install."
302+
303+
| Fixture | Tree | Expected result |
304+
| --------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- |
305+
| `root-declared-and-installed` | Root `package.json` declares `vite-plus` + valid `node_modules/vite-plus/` install | `{ root: "<repo>", vpPath: "<repo>/node_modules/.bin/vp" }` |
306+
| `pnpm-subpackage-declared-root-hoisted` | `pnpm-workspace.yaml` at `<repo>`, `packages/app/package.json` declares `vite-plus`, install hoisted to `<repo>/node_modules/vite-plus/` | From `packages/app/`: `{ root: "<repo>/packages/app", vpPath: "<repo>/node_modules/.bin/vp" }` |
307+
| `npm-subpackage-direct-dep-unhoisted` | Root `package.json` with `workspaces`, `packages/app/package.json` declares `vite-plus`, install inside `packages/app/node_modules/vite-plus/` | From `packages/app/`: `{ root: "<repo>/packages/app", vpPath: "<repo>/packages/app/node_modules/.bin/vp" }` |
308+
| `root-declared-no-install` | Root `package.json` declares `vite-plus`, no `node_modules` (fresh clone) | `{ root: "<repo>" }` — install hint |
309+
| `transitive-install` | No walked-up `package.json` declares `vite-plus`, but `node_modules/vite-plus/` exists as a transitive dep | `null` — no direct declaration |
310+
| `bin-vp-orphan` | Declared in root `package.json`, but `node_modules/vite-plus/` is broken (missing `package.json`, wrong `name`, or unparseable) | `{ root: "<repo>" }` — install rejected as orphan |
311+
| `parent-vite-plus-nested-repo` | Outer dir declares + installs `vite-plus`; inner subdir is its own root workspace and does not | From inside the nested workspace: `null` |
312+
| `plain-non-vite-plus` | A normal Node project, no `vite-plus` anywhere | `null` |
313+
| `yarn4-pnp` | Berry/PnP, no `node_modules`, root `package.json` declares `vite-plus` | `{ root: "<repo>" }` — install hint |
288314

289315
## Open questions
290316

0 commit comments

Comments
 (0)