Conversation
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| `dependencies` or `devDependencies`. A `node_modules/vite-plus/` | ||
| directory on its own does not qualify — that could be a transitive | ||
| install hoisted from an unrelated dependency tree. |
There was a problem hiding this comment.
A `node_modules/vite-plus/` directory on its own does not qualify — that could be a transitive install hoisted from an unrelated dependency tree.
I am not sure if vite-plus will ever be a dependency of another package. My naive brain does not like this.
VS Code Extension searched for package.json and tries to find node_modules/.bin/xyz.
Opening the package-file, transforming it to JSON, and checking its properties, will be done by every package.json it will find.
On a monorepo this could be expensive. 🤔
There was a problem hiding this comment.
I have considered this issue. If we do not read package.json to determine but instead judge by checking node_modules/vite-plus, there will be an unavoidable problem: if vite-plus is indirectly introduced by a dependency of the project through dependencies or peerDependencies, and it is installed using npm install, vite-plus will be hoisted to node_modules/vite-plus due to the hoisting mechanism, which will lead us to incorrectly determine the current project as Vite+.
If this check is only executed once during the vscode startup phase, the performance should be acceptable?
There was a problem hiding this comment.
If this check is only executed once during the vscode startup phase, the performance should be acceptable?
The startup is already effected by the package.json search for sub-packages. On a big project this could take over 1 second.
I can accept this performance cost. I have plans to "delay" to search to not block the extension activation.
| Both phases stop AT the workspace root and never cross into its | ||
| parent. The walk-up bound is what prevents a nested checkout from | ||
| inheriting an unrelated parent's Vite+ install. |
There was a problem hiding this comment.
Some users will open only a sub-package of a monorepo. Vite-plus from the root will not be detected.
Note: VS Code Extension does search outside the root (beside require.resolve), because mostly they are using Nested Multi Root Workspaces. But we should make sure setups like this will detect vite-plus
There was a problem hiding this comment.
Some users will open only a sub-package of a monorepo
In this case, will there be similar issues with the search for the oxlint bin? When the sub-package does not depend on oxlint, but only the root workspace depends on oxlint.
There was a problem hiding this comment.
Does require.resolve not resolves this correctly 🤔 The problem is for oxlint too :)
| `<vite-plus>/bin/oxlint` to `<vite-plus>/bin/vp`, update launch | ||
| args. | ||
|
|
||
| ## Conformance fixtures |
There was a problem hiding this comment.
What happens for workspaces where the root package.json does include oxlint, and only a sub-package includes vite-plus?
Should we prefer the vite-plus from the sub-package, even if other packages will use the roots oxlint?
Maybe related: oxc-project/oxc-vscode#145
There was a problem hiding this comment.
In this case, if VS Code opens the root workspace, it won’t be treated as a Vite+ project. We also plan to provide a capability similar to #1456: ship a rule via Vite+’s built-in oxlint plugin to prevent mixing oxlint and vite-plus within the same monorepo.
| 1. **Publish the detector as a shared npm package?** The current | ||
| proposal is `@voidzero-dev/detect-vite-plus` at | ||
| `packages/detect-vite-plus/`, consumed as a bundled devDependency | ||
| by `oxc-vscode` and `coc-oxc`. The alternative is to let each | ||
| Node-capable extension copy the ~50-line snippet directly. | ||
| Decision deferred to the maintainers. |
There was a problem hiding this comment.
The package makes sense for me. But I am not sure if this will be useful for other editors as they don't have JS runtime and needs to recreate it anyway by hand.
There was a problem hiding this comment.
I think coc-oxc can also use this package too, it seem to support Node.js runtime https://github.com/oxc-project/coc-oxc/blob/main/src/common.ts
| - `$PATH`, user's global `node_modules`, or | ||
| `oxc.<tool>.binPath` settings (the last is for oxlint/oxfmt, not | ||
| `vp`). |
There was a problem hiding this comment.
I would like to search for global node_modules and $PATH variable too.
Users want a global editor formatter, which they can use without forcing the repo-dependency.
VP-Users needs to know that oxfmt is used and manual install it globally.
There was a problem hiding this comment.
For Vite+ projects, this might not be possible. If the project doesn't have a dependency on vite-plus, the oxc IDE extension probably won't recognize it as a Vite+ project, and therefore won't look for the vp binary in the global path.
There was a problem hiding this comment.
We need to be careful, formatting in editor is very common. It should work every time, if possible.
IDE extension probably won't recognize it as a Vite+ project
Then it should not find oxlint & oxfmt too, and fallback to global search. Here we can search for vite-plus first, and then fallback to oxlint?
Scenario A:
User opens a project where no formatter or linter is installed. It is an old project where he edits one or two files every year. He still wants the formatting capability of the editor without creating a new setup.
Scenario B:
User opens a single file, there is no workspace/root concept anymore. The extension should fall back to globally installed packages, to allow users to format the file (with the default config).
| | `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 | | ||
| | `parent-vite-plus-nested-repo` | Outer dir declares + installs `vite-plus`; inner subdir is its own workspace root and does not | From inside the nested workspace: `null` | | ||
| | `plain-non-vite-plus` | A normal Node project, no `vite-plus` anywhere | `null` | | ||
| | `yarn4-pnp` | Berry/PnP, no `node_modules`, root `package.json` declares `vite-plus` | `{ root: "<repo>" }` — install hint | |
There was a problem hiding this comment.
No Yarn PnP support for vp? For VSCode oxlint and oxfmt it is supported:
https://github.com/oxc-project/oxc-vscode/blob/aeeb69c6a7990738d31e53dd21b73dce9776f63c/client/findBinary.ts#L171-L196
There was a problem hiding this comment.
If the current project is detected to be a Vite+ project, the vp binary can be located following the same rules as the oxlint binary, which supports Yarn PnP.
"Workspace root" collides with the VS Code "workspace" concept, which caused confusion in PR review (#1614). The monorepo-level concept this RFC describes is more precisely called the "root workspace" — the directory at the top of the monorepo containing pnpm-workspace.yaml / package.json#workspaces / lerna.json. Updates the prose, the Mermaid diagram labels, the pseudocode helper name, and the reference TypeScript helper (isWorkspaceRoot → isRootWorkspace). The literal Rust function name `find_workspace_root` in vite-task is preserved as a code citation. Also adds a blockquote in the "rule" section explicitly distinguishing the monorepo concept from the editor "workspace folder" concept so future readers don't hit the same confusion.
Defines a portable rule the four oxc editor extensions
(oxc-vscode, oxc-zed, oxc-intellij-plugin, coc-oxc) use to decide
whether to launch `vp lint --lsp` / `vp fmt --lsp` instead of plain
oxlint / oxfmt, and to resolve which executable to spawn.
Core algorithm (two phases, both bounded by the workspace root):
1. Phase 1 — find the package.json that DIRECTLY declares vite-plus
in dependencies or devDependencies. A transitively hoisted
`node_modules/vite-plus/` does not count.
2. Phase 2 — walk up from the declaring ancestor looking for a real
`node_modules/vite-plus/bin/vp` with a sibling `package.json` that
parses and has `name === "vite-plus"`.
Result is a tri-state: `null` (not Vite+), `{ root, vpPath }`
(Vite+ and runnable), or `{ root }` (declared but not yet installed —
editors fall back to plain oxlint/oxfmt and never launch a bare vp).
Distribution: for the Node-capable extensions (oxc-vscode, coc-oxc)
the detector ships as a shared bundled-devDependency package
(@voidzero-dev/detect-vite-plus, name TBD) at
`packages/detect-vite-plus/`. oxc-zed (Rust) and
oxc-intellij-plugin (Kotlin) port the same algorithm against the
shared conformance fixtures.
The RFC also documents:
- The four extensions' existing bin-resolution chains with code
excerpts and file:line citations.
- A Mermaid flowchart of the two-phase algorithm for cross-language
porters.
- The workspace-root marker set (`pnpm-workspace.yaml`,
`package.json#workspaces`, `lerna.json`) and a known parity gap
with vite-task that this RFC does not block on.
- Per-extension migration plans and a conformance fixture table that
pins down behaviour across every implementation.
Refs #1557
… isn't .vp-root was a fabricated example. Replace with .vite-hooks, which is the real vite-plus hooks directory (packages/cli/src/config/hooks.ts:67) and a more useful illustration of "incidental vite-plus artifact that must not stop the walk-up." Refs #1557
The detector now reads node_modules/vite-plus/package.json#version alongside name validation and returns it in DetectResult as vpVersion. Editors compare against MIN_VP_VERSION_FOR_LSP (a constant exported by the shared detector package, filled in when #1557's wrapper removal lands) to choose between launching `vp lint --lsp` and falling through to the legacy bin/oxlint wrapper chain shipped by older vite-plus versions. Three benefits: - Smooth rollout: workspaces pinned to older vite-plus keep working via the existing wrapper, no breakage on upgrade of the editor extension alone. - Upgrade hint: editors can surface "upgrade vite-plus to enable native LSP" when the installed version is too old. - Forward compatibility: future LSP-only features can raise the threshold by bumping the detector package, not by changing every consumer. Changes: - DetectResult gains optional vpVersion (set whenever vpPath is set). - resolveVpAt() rejects installs whose package.json has no string version, treating them as orphan. - Public API adds MIN_VP_VERSION_FOR_LSP constant and supportsLsp() helper for both consumers to use. - Per-extension migration plan gains a fourth branch for the "installed but version too old" case. - New conformance fixtures: installed-but-version-too-old, installed-no-version-field. - New open question: the concrete value of MIN_VP_VERSION_FOR_LSP (TBD until #1557 ships).
826 lines → 302 lines. Reviewer flagged repetition and pushed back on the version-gating mechanism. Changes: - Drop MIN_VP_VERSION_FOR_LSP / supportsLsp / vpVersion entirely. Editors just attempt `vp lint --lsp`; if it fails on an old vp, the editor surfaces an upgrade hint at that point. - Change "declared but not installed" UX: surface an install hint instead of silently falling back to plain oxlint. The reviewer is right that plain oxlint without VP_VERSION isn't Vite+-aware anyway, so silent fallback misleads more than it helps. - Heavy trim of repetition: drop "Insight", drop the long bin-resolution code excerpts (kept brief file:line refs), fold "Why this rule" into the rule section, collapse the Decisions section, drop "Downstream coordination" (overlapped with the migration plan), drop the "Verification plan" subheading. - Move "publish as a shared npm package?" from a locked Decision to an Open Question — the reviewer asked us to make the call; making it visibly open instead of locked invites the reviewer to weigh in. - Shrink the conformance fixture table: drop entries that document non-features (global-vp-on-path, user-binpath-override) now that the rule plainly says we don't check $PATH or user settings, and drop the version-related fixtures.
"Workspace root" collides with the VS Code "workspace" concept, which caused confusion in PR review (#1614). The monorepo-level concept this RFC describes is more precisely called the "root workspace" — the directory at the top of the monorepo containing pnpm-workspace.yaml / package.json#workspaces / lerna.json. Updates the prose, the Mermaid diagram labels, the pseudocode helper name, and the reference TypeScript helper (isWorkspaceRoot → isRootWorkspace). The literal Rust function name `find_workspace_root` in vite-task is preserved as a code citation. Also adds a blockquote in the "rule" section explicitly distinguishing the monorepo concept from the editor "workspace folder" concept so future readers don't hit the same confusion.
…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.
Package managers create `.cmd` shims on Windows for JS-based bins. Trying `vp.exe` and bare `vp` as fallbacks is over-engineered noise: - `vp.exe` would only be created for native binaries; `vp` is a JS shim, so no package manager produces a .exe. - Bare `vp` (no extension) on Windows isn't materialized on disk; it works under shell environments via PATHEXT, but existsSync checks filesystem entries and would miss it anyway. Collapses the candidates loop to a single conditional path, drops the "`.cmd`/`.exe`" mention from the prose.
Three cleanups from /simplify review: - Per-extension migration section opened with a re-statement of the three result outcomes already enumerated in § Result. Replaced with a one-line pointer. - Open question #2 (final package name) was contingent on #1, not a standalone question. Folded the name into the package-publish question as a parenthetical. - resolveVpAt's doc comment restated what the code already says; collapsed to one short JSDoc line.
2 participants
Summary
Draft RFC defining how the four oxc editor extensions —
oxc-vscode,oxc-zed,oxc-intellij-plugin,coc-oxc— determine whether a workspace is a Vite+ project and whichvpexecutable to spawn for--lsp. This is the prerequisite identified in #1557 for removing the per-packagebin/oxlint/bin/oxfmtwrappers.Core rule
A workspace is Vite+ iff some walked-up
package.jsondirectly declaresvite-plusindependenciesordevDependencies(bounded by the workspace root). The runnablevpbinary is resolved separately, also bounded by the workspace root, and validated against a realvite-pluspackage (name === "vite-plus").Detector returns a tri-state:
null— not Vite+; editor uses plain oxlint/oxfmt.{ root, vpPath }— Vite+ and runnable; editor launchesvp lint --lsp/vp fmt --lsp.{ root }— declared but not yet installed (fresh clone, Berry PnP, broken install); editor falls back to plain oxlint/oxfmt and never spawns a barevp.A transitively hoisted
node_modules/vite-plus/, a globalvp, avpon\$PATH, and a configuredoxc.<tool>.binPathall returnnullby construction.Distribution
oxc-vscodeandcoc-oxcconsume a shared bundled-devDependency npm package (proposed@voidzero-dev/detect-vite-plus) atpackages/detect-vite-plus/.oxc-zed(Rust/WASM) andoxc-intellij-plugin(Kotlin) port the same algorithm against the shared conformance fixtures inside this RFC.Status
Draft for discussion. Open points called out in the RFC:
vite-task(recognized as a known follow-up; deliberately not blocked on).Refs #1557