From 05a8ef8c0a77ef9147a27ef862cddde2a36e2d4e Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 00:41:40 +0800 Subject: [PATCH 01/15] feat: support Vite+ detection and explicit configuration --- .vscode-test.mjs | 2 + README.md | 60 +++++-- client/ConfigService.ts | 66 +++++++- client/VSCodeConfig.ts | 2 + client/detectVitePlus.ts | 71 ++++++++ client/extension.ts | 20 +++ client/findBinary.ts | 51 +++++- client/tools/ToolInterface.ts | 2 +- client/tools/formatter.ts | 75 ++++++-- client/tools/linter.ts | 80 +++++++-- client/tools/lsp_helper.ts | 11 +- package.json | 16 ++ tests/unit/VSCodeConfig.spec.ts | 6 + tests/unit/detectVitePlus.spec.ts | 150 ++++++++++++++++ tests/unit/findBinary.spec.ts | 18 ++ tests/unit/lsp_helper.spec.ts | 53 +++++- tests/unit/vitePlus.spec.ts | 244 +++++++++++++++++++++++++++ tests/unit/vitePlusLifecycle.spec.ts | 92 ++++++++++ 18 files changed, 963 insertions(+), 56 deletions(-) create mode 100644 client/detectVitePlus.ts create mode 100644 tests/unit/detectVitePlus.spec.ts create mode 100644 tests/unit/vitePlus.spec.ts create mode 100644 tests/unit/vitePlusLifecycle.spec.ts diff --git a/.vscode-test.mjs b/.vscode-test.mjs index 2cddb861..fd645fe4 100644 --- a/.vscode-test.mjs +++ b/.vscode-test.mjs @@ -45,6 +45,8 @@ const allTestSuites = new Map([ workspaceFolder: multiRootWorkspaceFile, env: { MULTI_FOLDER_WORKSPACE: "true", + SKIP_LINTER_TEST: "true", + SKIP_FORMATTER_TEST: "true", YARN_FOUND_BIN: path.resolve(import.meta.dirname, "node_modules/oxlint/dist/cli.js"), }, }, diff --git a/README.md b/README.md index 3720e1c5..2e46b964 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,32 @@ The extension does not bundle the Oxc tools. For the recommended setup, install See the official [Oxlint editor setup](https://oxc.rs/docs/guide/usage/linter/editors.html) and [Oxfmt editor setup](https://oxc.rs/docs/guide/usage/formatter/editors.html) guides for installation details. +## Vite+ + +For projects that declare `vite-plus` in `dependencies` or `devDependencies`, the extension runs `vp lint --lsp` and `vp fmt --lsp`. Detection starts from the active file's directory and stops at the monorepo root (`pnpm-workspace.yaml`, `package.json` with `workspaces`, or `lerna.json`). With no active workspace file, the extension checks the workspace folders in order. It checks the local `node_modules/.bin/vp` shim first, then `PATH` and global package installations. + +To select Vite+ without automatic dependency detection, add this to your workspace's `.vscode/settings.json`: + +```json +{ + "oxc.vitePlus.enable": true +} +``` + +You can also set `oxc.path.vp` to an absolute path or a path relative to the workspace folder. This setting selects Vite+ without dependency detection: + +```json +{ + "oxc.path.vp": "./node_modules/.bin/vp" +} +``` + +On Windows, use `./node_modules/.bin/vp.cmd`. To run the JavaScript entry point with `oxc.path.node` or `oxc.useExecPath`, set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp`. + +Both settings can differ between workspace folders. `oxc.vitePlus.enable` defaults to `null` (automatic detection); `false` disables Vite+ integration, including `oxc.path.vp`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority for their respective tools. + +The extension rechecks Vite+ when you switch files and restarts a server if its executable or project directory changes. It uses one server per tool for the window. If Vite+ is selected but `vp` is unavailable, the status item and output channels show an install hint. Install your dependencies and run the **Oxc: Restart oxlint Server** and **Oxc: Restart oxfmt Server** commands. A failed Vite+ launch shows an install or upgrade hint. Vite+ integration requires a trusted workspace, and `oxc.requireConfig` does not require a separate Oxlint configuration when Vite+ is selected. + ## Oxlint This is the linter for Oxc. The currently supported features are listed below. @@ -96,22 +122,24 @@ Following configurations are supported via `settings.json` and affect the window Following configurations are supported via `settings.json` and can be changed for each workspace: -| Key | Default Value | Possible Values | Description | -| ----------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `oxc.configPath` | `null` | `` \| `` | Path to oxlint configuration. Keep it empty to enable nested configuration. | -| `oxc.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used. | -| `oxc.fixKind` | `null` | `safe_fix` \| `safe_fix_or_suggestion` \| `dangerous_fix` \| `dangerous_fix_or_suggestion` \| `none` \| `all` | Specify the kind of fixes to suggest/apply. | -| `oxc.fmt.configPath` | `null` | `` \| `` | Path to an oxfmt configuration file | -| `oxc.fmt.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used. | -| `oxc.lint.customization` | `null` | `Record` \| `` | Customizes linting rules behavior. See for details. | -| `oxc.lint.run` | `onType` | `onSave` \| `onType` | Run the linter on save (onSave) or on type (onType) | -| `oxc.requireConfig` | `false` | `true` \| `false` | Start the language server only when a `.oxlintrc.json(c)` or `oxlint.config.ts` file exists in one of the workspaces. | -| `oxc.tsConfigPath` | `null` | `` \| `` | Path to the project's TypeScript config file. If your `tsconfig.json` is not at the root, you will need this set for the `import` plugin rules to resolve imports correctly. | -| `oxc.typeAware` | `null` | `true` \| `false` \| `` | Forces type-aware linting. Requires the `oxlint-tsgolint` package. It is preferred to use `options.typeAware` in your configuration file | -| `oxc.unusedDisableDirectives` | `null` | `allow` \| `warn` \| `deny` | Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway. It is preferred to use `options.reportUnusedDisableDirectives` in your configuration file | -| Deprecated | | | | -| `oxc.flags` | `{}` | `Record` | Specific Oxlint flags to pass to the language server. | -| `oxc.fmt.experimental` | `true` | `true` \| `false` | Enable Oxfmt formatting support. | +| Key | Default Value | Possible Values | Description | +| ----------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `oxc.configPath` | `null` | `` \| `` | Path to oxlint configuration. Keep it empty to enable nested configuration. | +| `oxc.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used. | +| `oxc.fixKind` | `null` | `safe_fix` \| `safe_fix_or_suggestion` \| `dangerous_fix` \| `dangerous_fix_or_suggestion` \| `none` \| `all` | Specify the kind of fixes to suggest/apply. | +| `oxc.fmt.configPath` | `null` | `` \| `` | Path to an oxfmt configuration file | +| `oxc.fmt.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used. | +| `oxc.lint.customization` | `null` | `Record` \| `` | Customizes linting rules behavior. See for details. | +| `oxc.lint.run` | `onType` | `onSave` \| `onType` | Run the linter on save (onSave) or on type (onType) | +| `oxc.path.vp` | - | `` | Path to a `vp` executable. Relative paths use the workspace folder. Setting this path selects Vite+ without automatic detection, unless `oxc.vitePlus.enable` is `false`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority. | +| `oxc.requireConfig` | `false` | `true` \| `false` | Start the language server only when a `.oxlintrc.json(c)` or `oxlint.config.ts` file exists in one of the workspaces. | +| `oxc.tsConfigPath` | `null` | `` \| `` | Path to the project's TypeScript config file. If your `tsconfig.json` is not at the root, you will need this set for the `import` plugin rules to resolve imports correctly. | +| `oxc.typeAware` | `null` | `true` \| `false` \| `` | Forces type-aware linting. Requires the `oxlint-tsgolint` package. It is preferred to use `options.typeAware` in your configuration file | +| `oxc.unusedDisableDirectives` | `null` | `allow` \| `warn` \| `deny` | Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway. It is preferred to use `options.reportUnusedDisableDirectives` in your configuration file | +| `oxc.vitePlus.enable` | `null` | `true` \| `false` \| `` | Use `vp lint --lsp` and `vp fmt --lsp`. Set to `true` to select Vite+ without detecting a dependency, `false` to disable Vite+ integration, or `null` to detect a direct `vite-plus` dependency. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority. | +| Deprecated | | | | +| `oxc.flags` | `{}` | `Record` | Specific Oxlint flags to pass to the language server. | +| `oxc.fmt.experimental` | `true` | `true` \| `false` | Enable Oxfmt formatting support. | #### FixKind diff --git a/client/ConfigService.ts b/client/ConfigService.ts index f488a02b..7cf6499e 100644 --- a/client/ConfigService.ts +++ b/client/ConfigService.ts @@ -1,4 +1,6 @@ -import { ConfigurationChangeEvent, Uri, workspace, WorkspaceFolder } from "vscode"; +import * as path from "node:path"; +import { ConfigurationChangeEvent, Uri, window, workspace, WorkspaceFolder } from "vscode"; +import { detectVitePlusProject, VitePlusError } from "./detectVitePlus"; import { DiagnosticPullMode } from "vscode-languageclient"; import { BinarySearchResult, @@ -23,6 +25,7 @@ export class ConfigService implements IDisposable { public vsCodeConfig: VSCodeConfig; private workspaceConfigs: Map = new Map(); + private vitePlusSearch: Promise | undefined; public onConfigChange: | ((this: ConfigService, config: ConfigurationChangeEvent) => Promise) @@ -116,12 +119,17 @@ export class ConfigService implements IDisposable { private async searchBinaryPath( settingsBinary: string | undefined, - defaultBinaryName: string, + defaultBinaryName: "oxlint" | "oxfmt", ): Promise { if (settingsBinary) { return searchSettingsBin(defaultBinaryName, settingsBinary); } + const vitePlus = await this.searchVitePlus(); + if (vitePlus) { + return { ...vitePlus, vitePlus: defaultBinaryName === "oxlint" ? "lint" : "fmt" }; + } + return ( (await searchProjectNodeModulesBin(defaultBinaryName)) ?? (await searchYarnPnpBin(defaultBinaryName)) ?? @@ -130,6 +138,60 @@ export class ConfigService implements IDisposable { ); } + private async searchVitePlus(): Promise { + // Lint and fmt share concurrent discovery, but restarts always re-read disk. + if (this.vitePlusSearch) return this.vitePlusSearch; + const search = this.resolveVitePlus(); + this.vitePlusSearch = search; + try { + return await search; + } finally { + this.vitePlusSearch = undefined; + } + } + + private async resolveVitePlus(): Promise { + if (!workspace.isTrusted) return null; + + const documentUri = window.activeTextEditor?.document.uri; + const activeFolder = + documentUri?.scheme === "file" ? workspace.getWorkspaceFolder(documentUri) : undefined; + const folders = activeFolder ? [activeFolder] : (workspace.workspaceFolders ?? []); + + for (const folder of folders) { + const config = workspace.getConfiguration(ConfigService.namespace, folder.uri); + const enabled = config.get("vitePlus.enable"); + if (enabled === false) continue; + + const configuredPath = config.get("path.vp"); + const start = + activeFolder && documentUri ? path.dirname(documentUri.fsPath) : folder.uri.fsPath; + if (configuredPath) { + // An explicit vp path opts in without requiring a dependency declaration. + // oxlint-disable-next-line no-await-in-loop -- workspace folder order is significant + const binary = await searchSettingsBin("vp", configuredPath, folder.uri.fsPath); + if (!binary) + throw new VitePlusError(`Invalid Vite+ binary: ${configuredPath}. Check oxc.path.vp.`); + return { ...binary, cwd: folder.uri.fsPath }; + } + + const project = detectVitePlusProject(start, enabled === true); + if (!project) continue; + // Global vp is eligible only after detection or explicit opt-in. + const binary: BinarySearchResult | undefined = project.vpPath + ? { path: project.vpPath, loader: "native" } + : // oxlint-disable-next-line no-await-in-loop -- global lookup requires a Vite+ project + ((await searchEnvPath("vp")) ?? (await searchGlobalNodeModulesBin("vp", "vite-plus"))); + if (!binary) { + throw new VitePlusError( + `Vite+ selected in ${project.root}, but no vp binary was found. Run your package manager's install command (for example, pnpm install), or set oxc.path.vp, then restart the Oxc servers.`, + ); + } + return { ...binary, cwd: project.root }; + } + return null; + } + private async onVscodeConfigChange(event: ConfigurationChangeEvent): Promise { let isConfigChanged = false; diff --git a/client/VSCodeConfig.ts b/client/VSCodeConfig.ts index 9dfc4923..17d11ab5 100644 --- a/client/VSCodeConfig.ts +++ b/client/VSCodeConfig.ts @@ -154,6 +154,8 @@ export class VSCodeConfig implements VSCodeConfigInterface { private effectsGeneralLSPConnection(event: ConfigurationChangeEvent): boolean { return ( event.affectsConfiguration(`${ConfigService.namespace}.path.node`) || + event.affectsConfiguration(`${ConfigService.namespace}.path.vp`) || + event.affectsConfiguration(`${ConfigService.namespace}.vitePlus.enable`) || event.affectsConfiguration(`${ConfigService.namespace}.useExecPath`) ); } diff --git a/client/detectVitePlus.ts b/client/detectVitePlus.ts new file mode 100644 index 00000000..cdb48239 --- /dev/null +++ b/client/detectVitePlus.ts @@ -0,0 +1,71 @@ +import { existsSync, readFileSync, statSync } from "node:fs"; +import * as path from "node:path"; + +export interface VitePlusProject { + /** The ancestor that declares vite-plus, or the explicitly enabled directory. */ + root: string; + /** Undefined means Vite+ is selected but is not installed locally. */ + vpPath?: string; +} + +interface PackageJson { + dependencies?: Record; + devDependencies?: Record; + workspaces?: unknown; +} + +function readPackageJson(dir: string): PackageJson | null { + try { + return JSON.parse(readFileSync(path.join(dir, "package.json"), "utf8")); + } catch { + return null; + } +} + +function isRootWorkspace(dir: string, pkg: PackageJson | null): boolean { + return ( + existsSync(path.join(dir, "pnpm-workspace.yaml")) || + existsSync(path.join(dir, "lerna.json")) || + Boolean(pkg?.workspaces) + ); +} + +/** + * Implements Phases 1 and 2 of the editor detection RFC. The bound is the + * monorepo root, which can be above the folder opened in the editor. + * Global lookup belongs to the caller and must only run for a non-null result. + * https://github.com/voidzero-dev/vite-plus/pull/1614 + */ +export function detectVitePlusProject(start: string, enabled = false): VitePlusProject | null { + let dir = path.resolve(start); + try { + if (statSync(dir).isFile()) dir = path.dirname(dir); + } catch { + // The caller can also pass a directory that does not exist yet. + } + + let pkg = readPackageJson(dir); + if (!enabled) { + while (!pkg?.dependencies?.["vite-plus"] && !pkg?.devDependencies?.["vite-plus"]) { + if (isRootWorkspace(dir, pkg) || dir === path.dirname(dir)) return null; + dir = path.dirname(dir); + pkg = readPackageJson(dir); + } + } + + const root = dir; + while (true) { + const vpPath = path.join( + dir, + "node_modules", + ".bin", + process.platform === "win32" ? "vp.cmd" : "vp", + ); + if (existsSync(vpPath)) return { root, vpPath }; + if (isRootWorkspace(dir, pkg) || dir === path.dirname(dir)) return { root }; + dir = path.dirname(dir); + pkg = readPackageJson(dir); + } +} + +export class VitePlusError extends Error {} diff --git a/client/extension.ts b/client/extension.ts index ed09e84b..e7635b37 100644 --- a/client/extension.ts +++ b/client/extension.ts @@ -118,6 +118,26 @@ export async function activate(context: ExtensionContext) { }), ); + // A window has one client per tool. Re-resolve on navigation, and restart + // only when the executable, Vite+ command, or project directory changes. + context.subscriptions.push( + window.onDidChangeActiveTextEditor((editor) => { + if ( + editor?.document.uri.scheme !== "file" || + !workspace.getWorkspaceFolder(editor.document.uri) + ) + return; + for (const tool of tools) { + void tool.restart(true).catch((error) => { + const output = tool instanceof Linter ? outputChannelLint : outputChannelFormat; + output.error( + `Failed to switch language server: ${error instanceof Error ? error.message : String(error)}`, + ); + }); + } + }), + ); + // Finally show the status bar item. statusBarItemHandler.show(); } diff --git a/client/findBinary.ts b/client/findBinary.ts index 1f179c98..8dc55de8 100644 --- a/client/findBinary.ts +++ b/client/findBinary.ts @@ -11,6 +11,8 @@ export type BinarySearchResult = { path: string; loader: "node" | "native"; yarnPnpLoaderPath?: string; // only set if loader is 'node' and found via Yarn PnP + vitePlus?: "lint" | "fmt"; + cwd?: string; }; /** @internal only used for testing */ @@ -44,7 +46,11 @@ async function searchNodeModulesDefaultBinPath( ): Promise { const candidates = folders.flatMap((folder) => { const basePath = path.join(folder, ".bin", binaryName); - return process.platform === "win32" ? [basePath, `${basePath}.exe`] : [basePath]; + return process.platform === "win32" + ? binaryName === "vp" + ? [`${basePath}.cmd`, `${basePath}.exe`, basePath] + : [basePath, `${basePath}.exe`] + : [basePath]; }); const exists = await Promise.all( @@ -202,6 +208,7 @@ export async function searchYarnPnpBin( */ export async function searchGlobalNodeModulesBin( binaryName: string, + packageName = binaryName, ): Promise { const globalPaths = await globalNodeModulesPaths(); @@ -213,6 +220,23 @@ export async function searchGlobalNodeModulesBin( if (result) { return result; } + // A package's executable can have a different name (vite-plus provides vp). + // Read only the global package directories, without resolving into a parent. + if (packageName !== binaryName) { + for (const globalPath of globalPaths) { + try { + const packageDir = path.join(globalPath, packageName); + const pkg = JSON.parse(readFileSync(path.join(packageDir, "package.json"), "utf8")); + const binEntry = typeof pkg.bin === "string" ? pkg.bin : pkg.bin?.[binaryName]; + if (pkg.name !== packageName || typeof binEntry !== "string") continue; + const binPath = path.resolve(packageDir, binEntry); + // oxlint-disable-next-line no-await-in-loop -- preserve global lookup priority + await workspace.fs.stat(Uri.file(binPath)); + return { path: binPath, loader: "node" }; + } catch {} + } + return undefined; + } // fallback to direct binary lookup via require.resolve try { const resolvedPath = replaceTargetFromMainToBin( @@ -244,7 +268,11 @@ export async function searchEnvPath( return []; } const basePath = path.join(folder, defaultBinaryName); - return process.platform === "win32" ? [basePath, `${basePath}.exe`] : [basePath]; + return process.platform === "win32" + ? defaultBinaryName === "vp" + ? [`${basePath}.cmd`, `${basePath}.exe`, basePath] + : [basePath, `${basePath}.exe`] + : [basePath]; }); const binary = await Promise.all( @@ -270,6 +298,7 @@ export async function searchEnvPath( export async function searchSettingsBin( defaultBinaryName: string, settingsBinary: string, + cwd = workspace.workspaceFolders?.[0]?.uri.fsPath, ): Promise { if (!workspace.isTrusted) { return; @@ -281,7 +310,6 @@ export async function searchSettingsBin( } if (!path.isAbsolute(settingsBinary)) { - const cwd = workspace.workspaceFolders?.[0]?.uri.fsPath; if (!cwd) { return undefined; } @@ -298,7 +326,22 @@ export async function searchSettingsBin( settingsBinary.endsWith(".js") || settingsBinary.endsWith(".cjs") || settingsBinary.endsWith(".mjs") || - settingsBinary.endsWith(`${defaultBinaryName}${path.sep}bin${path.sep}${defaultBinaryName}`); + settingsBinary.endsWith(`${defaultBinaryName}${path.sep}bin${path.sep}${defaultBinaryName}`) || + (defaultBinaryName === "vp" && settingsBinary.endsWith(path.join("vite-plus", "bin", "vp"))); + + // npm creates both a POSIX shim and a .cmd shim on Windows. Prefer the + // latter when the configured vp path omits the extension. + if ( + process.platform === "win32" && + defaultBinaryName === "vp" && + !isNode && + !path.extname(settingsBinary) + ) { + try { + await workspace.fs.stat(Uri.file(`${settingsBinary}.cmd`)); + return { path: `${settingsBinary}.cmd`, loader: "native" }; + } catch {} + } try { await workspace.fs.stat(Uri.file(settingsBinary)); diff --git a/client/tools/ToolInterface.ts b/client/tools/ToolInterface.ts index 73eb8b13..dad14b2b 100644 --- a/client/tools/ToolInterface.ts +++ b/client/tools/ToolInterface.ts @@ -29,7 +29,7 @@ export default interface ToolInterface { /** * Restarts the tool, cleaning up resources and reinitializing with the current configuration. */ - restart(): Promise; + restart(onlyIfBinaryChanged?: boolean): Promise; /** * Handles configuration changes. diff --git a/client/tools/formatter.ts b/client/tools/formatter.ts index 2b0b5f65..6bac68df 100644 --- a/client/tools/formatter.ts +++ b/client/tools/formatter.ts @@ -1,4 +1,6 @@ import { promises as fsPromises } from "node:fs"; +import { isDeepStrictEqual } from "node:util"; +import { VitePlusError } from "../detectVitePlus"; import { CodeAction, @@ -247,6 +249,9 @@ const supportedLanguageIds = [ export default class FormatterTool implements ToolInterface { // LSP client instance private client: LanguageClient | undefined; + private binary: BinarySearchResult | undefined; + private binaryError: string | undefined; + private restartQueue: Promise = Promise.resolve(); private documentSelectors = [ { @@ -317,11 +322,19 @@ export default class FormatterTool implements ToolInterface { } async getBinary(): Promise { + this.binaryError = undefined; if (process.env.SERVER_PATH_DEV_OXFMT) { const path = process.env.SERVER_PATH_DEV_OXFMT; return { path, loader: path.endsWith(".js") ? "node" : "native" }; } - const bin = await this.configService.getOxfmtServerBinPath(); + let bin: BinarySearchResult | undefined; + try { + bin = await this.configService.getOxfmtServerBinPath(); + } catch (error) { + if (!(error instanceof VitePlusError)) throw error; + this.binaryError = error.message; + return undefined; + } if (bin) { try { await fsPromises.access(bin.path); @@ -333,12 +346,12 @@ export default class FormatterTool implements ToolInterface { } async activate(binary?: BinarySearchResult) { + this.binary = binary; // No valid binary found for the formatter. if (!binary) { - this.statusBarItemHandler.updateTool("formatter", false, "No valid oxfmt binary found."); - this.outputChannel.appendLine( - "No valid oxfmt binary found. Formatter will not be activated.", - ); + const message = this.binaryError ?? "No valid oxfmt binary found."; + this.statusBarItemHandler.updateTool("formatter", false, message); + this.outputChannel.warn(message); return Promise.resolve(); } @@ -405,12 +418,17 @@ export default class FormatterTool implements ToolInterface { }; if (this.configService.vsCodeConfig.enableOxfmt) { - await this.client.start(); + await this.startClient(); } this.updateStatusBar(); } async deactivate(): Promise { + await this.restartQueue; + await this.stopClient(); + } + + private async stopClient(): Promise { try { await this.client?.stop(); } catch { @@ -421,10 +439,32 @@ export default class FormatterTool implements ToolInterface { this.client = undefined; } - async restart(): Promise { - await this.deactivate(); - const newBinaryPath = await this.getBinary(); - await this.activate(newBinaryPath); + restart(onlyIfBinaryChanged = false): Promise { + const restart = this.restartQueue.then(async () => { + const previousError = this.binaryError; + const newBinary = await this.getBinary(); + if ( + onlyIfBinaryChanged && + isDeepStrictEqual(this.binary, newBinary) && + previousError === this.binaryError + ) + return; + await this.stopClient(); + await this.activate(newBinary); + }); + this.restartQueue = restart.catch(() => {}); + return restart; + } + + private async startClient(): Promise { + try { + await this.client?.start(); + this.binaryError = undefined; + } catch (error) { + if (!this.binary?.vitePlus) throw error; + this.binaryError = `Failed to start Vite+ ${this.binary.vitePlus} --lsp. Install or upgrade vite-plus in ${this.binary.cwd}, then restart the Oxc servers. ${error instanceof Error ? error.message : String(error)}`; + this.outputChannel.error(this.binaryError); + } } async toggleClient(): Promise { @@ -438,12 +478,18 @@ export default class FormatterTool implements ToolInterface { } } else { if (this.configService.vsCodeConfig.enableOxfmt) { - await this.client.start(); + await this.startClient(); } } } - async onConfigChange(event: ConfigurationChangeEvent): Promise { + onConfigChange(event: ConfigurationChangeEvent): Promise { + const change = this.restartQueue.then(() => this.applyConfigChange(event)); + this.restartQueue = change.catch(() => {}); + return change; + } + + private async applyConfigChange(event: ConfigurationChangeEvent): Promise { if ( event.affectsConfiguration(`${ConfigService.namespace}.enable`) || event.affectsConfiguration(`${ConfigService.namespace}.enable.oxfmt`) @@ -485,14 +531,15 @@ export default class FormatterTool implements ToolInterface { text += `[$(play) Start Server](command:${OxcCommands.ToggleEnableFmt})\n\n`; } - const tooltipText = enable ? undefined : "`oxc.enable.oxfmt` or `oxc.enable` is false"; + const tooltipText = + this.binaryError ?? (enable ? undefined : "`oxc.enable.oxfmt` or `oxc.enable` is false"); if (tooltipText) { text = `${tooltipText}\n\n` + text; } this.statusBarItemHandler.updateTool( "formatter", - enable, + enable && !this.binaryError, text, this.client?.initializeResult?.serverInfo?.version, ); diff --git a/client/tools/linter.ts b/client/tools/linter.ts index 8f88a323..b777ed07 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -1,4 +1,6 @@ import { promises as fsPromises } from "node:fs"; +import { isDeepStrictEqual } from "node:util"; +import { VitePlusError } from "../detectVitePlus"; import { CodeActionKind, @@ -144,6 +146,9 @@ export default class LinterTool implements ToolInterface { // LSP client instance private client: LanguageClient | undefined; + private binary: BinarySearchResult | undefined; + private binaryError: string | undefined; + private restartQueue: Promise = Promise.resolve(); private disposeResources: (() => Promise) | undefined; @@ -205,11 +210,19 @@ export default class LinterTool implements ToolInterface { } async getBinary(): Promise { + this.binaryError = undefined; if (process.env.SERVER_PATH_DEV_OXLINT) { const path = process.env.SERVER_PATH_DEV_OXLINT; return { path, loader: path.endsWith(".js") ? "node" : "native" }; } - const bin = await this.configService.getOxlintServerBinPath(); + let bin: BinarySearchResult | undefined; + try { + bin = await this.configService.getOxlintServerBinPath(); + } catch (error) { + if (!(error instanceof VitePlusError)) throw error; + this.binaryError = error.message; + return undefined; + } if (bin) { try { await fsPromises.access(bin.path); @@ -221,16 +234,19 @@ export default class LinterTool implements ToolInterface { } async activate(binary?: BinarySearchResult): Promise { + this.binary = binary; if (!binary) { - this.statusBarItemHandler.updateTool("linter", false, "No valid oxlint binary found."); - this.outputChannel.appendLine("No valid oxlint binary found. Linter will not be activated."); + const message = this.binaryError ?? "No valid oxlint binary found."; + this.statusBarItemHandler.updateTool("linter", false, message); + this.outputChannel.warn(message); return Promise.resolve(); } - this.allowedToStartServer = this.configService.vsCodeConfig.requireConfig - ? (await workspace.findFiles(oxlintConfigDefaultFilePattern, "**/node_modules/**", 1)) - .length > 0 - : true; + this.allowedToStartServer = + !binary.vitePlus && this.configService.vsCodeConfig.requireConfig + ? (await workspace.findFiles(oxlintConfigDefaultFilePattern, "**/node_modules/**", 1)) + .length > 0 + : true; const run: Executable = await runExecutable( binary, @@ -356,7 +372,7 @@ export default class LinterTool implements ToolInterface { let activatorDispatcher: { dispose: () => void } | undefined; if (this.allowedToStartServer) { if (this.configService.vsCodeConfig.enableOxlint) { - await this.client.start(); + await this.startClient(); } } else { activatorDispatcher = this.generateActivatorByConfig(this.configService.vsCodeConfig); @@ -377,6 +393,11 @@ export default class LinterTool implements ToolInterface { } async deactivate(): Promise { + await this.restartQueue; + await this.stopClient(); + } + + private async stopClient(): Promise { try { await this.client?.stop(); } catch { @@ -404,18 +425,46 @@ export default class LinterTool implements ToolInterface { } } else { if (configService.vsCodeConfig.enableOxlint) { - await this.client.start(); + await this.startClient(); } } } - async restart(): Promise { - await this.deactivate(); - const newBinaryPath = await this.getBinary(); - await this.activate(newBinaryPath); + restart(onlyIfBinaryChanged = false): Promise { + const restart = this.restartQueue.then(async () => { + const previousError = this.binaryError; + const newBinary = await this.getBinary(); + if ( + onlyIfBinaryChanged && + isDeepStrictEqual(this.binary, newBinary) && + previousError === this.binaryError + ) + return; + await this.stopClient(); + await this.activate(newBinary); + }); + this.restartQueue = restart.catch(() => {}); + return restart; + } + + private async startClient(): Promise { + try { + await this.client?.start(); + this.binaryError = undefined; + } catch (error) { + if (!this.binary?.vitePlus) throw error; + this.binaryError = `Failed to start Vite+ ${this.binary.vitePlus} --lsp. Install or upgrade vite-plus in ${this.binary.cwd}, then restart the Oxc servers. ${error instanceof Error ? error.message : String(error)}`; + this.outputChannel.error(this.binaryError); + } + } + + onConfigChange(event: ConfigurationChangeEvent): Promise { + const change = this.restartQueue.then(() => this.applyConfigChange(event)); + this.restartQueue = change.catch(() => {}); + return change; } - async onConfigChange(event: ConfigurationChangeEvent): Promise { + private async applyConfigChange(event: ConfigurationChangeEvent): Promise { if ( event.affectsConfiguration(`${ConfigService.namespace}.enable`) || event.affectsConfiguration(`${ConfigService.namespace}.enable.oxlint`) @@ -449,6 +498,7 @@ export default class LinterTool implements ToolInterface { isEnabled: boolean; tooltipText?: string; } { + if (this.binaryError) return { isEnabled: false, tooltipText: this.binaryError }; if (!this.allowedToStartServer) { return { isEnabled: false, @@ -502,7 +552,7 @@ export default class LinterTool implements ToolInterface { this.allowedToStartServer = true; this.updateStatusBar(config.enableOxlint); if (this.client && !this.client.isRunning() && config.enableOxlint) { - await this.client.start(); + await this.startClient(); } }); diff --git a/client/tools/lsp_helper.ts b/client/tools/lsp_helper.ts index 18ff6c40..119d2442 100644 --- a/client/tools/lsp_helper.ts +++ b/client/tools/lsp_helper.ts @@ -47,6 +47,7 @@ export async function runExecutable( } const isWindows = process.platform === "win32"; + const args = binary.vitePlus ? [binary.vitePlus, "--lsp"] : ["--lsp"]; // In Yarn PnP environments, inject the PnP loaders so that both CJS require() // and ESM import calls can resolve dependencies through PnP. @@ -59,19 +60,23 @@ export async function runExecutable( pnpArgs.push("--loader", pathToFileURL(esmLoaderPath).href); } - return isNode || useExecPath + // vp can be a package-manager shell shim or a native executable. Neither + // can be interpreted as JavaScript, even when useExecPath is enabled. + return isNode || (useExecPath && !binary.vitePlus) ? { command: nodeCommand, - args: [...pnpArgs, binary.path, "--lsp"], + args: [...pnpArgs, binary.path, ...args], options: { + cwd: binary.cwd, env: serverEnv, }, } : { // On Windows with shell, quote the command path to handle spaces in usernames/paths command: isWindows ? `"${binary.path}"` : binary.path, - args: ["--lsp"], + args, options: { + cwd: binary.cwd, // On Windows we need to run the binary in a shell to be able to execute the shell npm bin script. // Searching for the right `.exe` file inside `node_modules/` is not reliable as it depends on // the package manager used (npm, yarn, pnpm, etc) and the package version. diff --git a/package.json b/package.json index f8d8cbcb..a29f33a7 100644 --- a/package.json +++ b/package.json @@ -326,6 +326,20 @@ "scope": "window", "markdownDescription": "Path to an Oxc formatter binary. Default: auto detection in `node_modules`." }, + "oxc.vitePlus.enable": { + "type": [ + "boolean", + "null" + ], + "scope": "resource", + "default": null, + "markdownDescription": "Use `vp lint --lsp` and `vp fmt --lsp`. Set to `true` to select Vite+ without detecting a dependency, `false` to disable Vite+ integration, or `null` to detect a direct `vite-plus` dependency. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority." + }, + "oxc.path.vp": { + "type": "string", + "scope": "resource", + "markdownDescription": "Path to a `vp` executable. Relative paths use the workspace folder. Setting this path selects Vite+ without automatic detection, unless `oxc.vitePlus.enable` is `false`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority." + }, "oxc.path.tsgolint": { "type": "string", "scope": "window", @@ -410,6 +424,8 @@ "oxc.path.oxfmt", "oxc.path.tsgolint", "oxc.path.node", + "oxc.path.vp", + "oxc.vitePlus.enable", "oxc.useExecPath" ] } diff --git a/tests/unit/VSCodeConfig.spec.ts b/tests/unit/VSCodeConfig.spec.ts index 7622992c..53df6811 100644 --- a/tests/unit/VSCodeConfig.spec.ts +++ b/tests/unit/VSCodeConfig.spec.ts @@ -18,6 +18,8 @@ suite("VSCodeConfig", () => { "path.node", "useExecPath", "suppressProgramErrors", + "path.vp", + "vitePlus.enable", ]; setup(async () => { await Promise.all(keys.map((key) => conf.update(key, undefined))); @@ -109,6 +111,8 @@ suite("VSCodeConfig", () => { { key: "path.tsgolint", affects: true }, { key: "path.node", affects: true }, { key: "useExecPath", affects: true }, + { key: "path.vp", affects: true }, + { key: "vitePlus.enable", affects: true }, { key: "requireConfig", affects: false }, { key: "path.oxfmt", affects: false }, ]; @@ -136,6 +140,8 @@ suite("VSCodeConfig", () => { { key: "path.oxfmt", affects: true }, { key: "path.node", affects: true }, { key: "useExecPath", affects: true }, + { key: "path.vp", affects: true }, + { key: "vitePlus.enable", affects: true }, { key: "path.tsgolint", affects: false }, { key: "requireConfig", affects: false }, { key: "path.oxlint", affects: false }, diff --git a/tests/unit/detectVitePlus.spec.ts b/tests/unit/detectVitePlus.spec.ts new file mode 100644 index 00000000..8afc85f0 --- /dev/null +++ b/tests/unit/detectVitePlus.spec.ts @@ -0,0 +1,150 @@ +import { deepStrictEqual, strictEqual } from "assert"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import * as path from "node:path"; +import { detectVitePlusProject } from "../../client/detectVitePlus"; + +suite("detectVitePlusProject", () => { + let root: string; + const originalPlatform = process.platform; + + function file(relative: string, content = ""): string { + const target = path.join(root, relative); + mkdirSync(path.dirname(target), { recursive: true }); + writeFileSync(target, content); + return target; + } + + function pkg(relative = "", value: object = { devDependencies: { "vite-plus": "latest" } }) { + return file(path.join(relative, "package.json"), JSON.stringify(value)); + } + + function shim(relative = ""): string { + return file( + path.join(relative, "node_modules/.bin", process.platform === "win32" ? "vp.cmd" : "vp"), + ); + } + + setup(() => { + root = mkdtempSync(path.join(tmpdir(), "detect-vite-plus-")); + file("pnpm-workspace.yaml"); + }); + + teardown(() => { + Object.defineProperty(process, "platform", { value: originalPlatform }); + rmSync(root, { recursive: true, force: true }); + }); + + test("root-declared-and-installed, starting from a file", () => { + pkg(); + const vpPath = shim(); + const start = file("src/index.ts"); + deepStrictEqual(detectVitePlusProject(start), { root, vpPath }); + }); + + test("pnpm-subpackage-declared-root-hoisted", () => { + pkg("packages/app"); + const vpPath = shim(); + const start = file("packages/app/src/index.ts"); + deepStrictEqual(detectVitePlusProject(start), { + root: path.join(root, "packages/app"), + vpPath, + }); + }); + + test("npm-subpackage-direct-dep-unhoisted", () => { + rmSync(path.join(root, "pnpm-workspace.yaml")); + pkg("", { workspaces: ["packages/*"] }); + pkg("packages/app", { dependencies: { "vite-plus": "latest" } }); + const vpPath = shim("packages/app"); + deepStrictEqual(detectVitePlusProject(path.join(root, "packages/app")), { + root: path.join(root, "packages/app"), + vpPath, + }); + }); + + test("root-declared-no-local-no-global", () => { + pkg(); + deepStrictEqual(detectVitePlusProject(root), { root }); + }); + + test("transitive-install", () => { + pkg("", { dependencies: { other: "latest" } }); + pkg("node_modules/vite-plus", { name: "vite-plus" }); + shim(); + strictEqual(detectVitePlusProject(root), null); + }); + + test("plain-non-vite-plus", () => { + pkg("", { + name: "plain", + peerDependencies: { "vite-plus": "latest" }, + optionalDependencies: { "vite-plus": "latest" }, + }); + strictEqual(detectVitePlusProject(root), null); + }); + + test("yarn4-pnp", () => { + pkg(); + file(".pnp.cjs", "throw new Error('the detector must not execute PnP code')"); + deepStrictEqual(detectVitePlusProject(root), { root }); + }); + + for (const marker of ["pnpm", "npm", "lerna"]) { + test(`parent-vite-plus-nested-repo (${marker})`, () => { + pkg(); + shim(); + pkg("nested", marker === "npm" ? { workspaces: ["packages/*"] } : {}); + if (marker !== "npm") + file(`nested/${marker === "pnpm" ? "pnpm-workspace.yaml" : "lerna.json"}`); + const start = file("nested/src/index.ts"); + strictEqual(detectVitePlusProject(start), null); + }); + + test(`does not resolve a local binary above a ${marker} workspace`, () => { + pkg(); + shim(); + pkg("nested", { + dependencies: { "vite-plus": "latest" }, + ...(marker === "npm" ? { workspaces: [] } : {}), + }); + if (marker !== "npm") + file(`nested/${marker === "pnpm" ? "pnpm-workspace.yaml" : "lerna.json"}`); + deepStrictEqual(detectVitePlusProject(path.join(root, "nested")), { + root: path.join(root, "nested"), + }); + }); + } + + test("continues past malformed package.json files", () => { + pkg(); + const vpPath = shim(); + const start = file("src/package.json", "{"); + deepStrictEqual(detectVitePlusProject(start), { root, vpPath }); + }); + + test("explicit opt-in does not require a dependency declaration", () => { + pkg("", {}); + const vpPath = shim(); + deepStrictEqual(detectVitePlusProject(root, true), { root, vpPath }); + }); + + test("prefers the nearest declaring package and its install", () => { + pkg(); + shim(); + pkg("packages/app"); + const vpPath = shim("packages/app"); + deepStrictEqual(detectVitePlusProject(path.join(root, "packages/app")), { + root: path.join(root, "packages/app"), + vpPath, + }); + }); + + test("selects vp.cmd on Windows", () => { + Object.defineProperty(process, "platform", { value: "win32" }); + pkg(); + file("node_modules/.bin/vp"); + const vpPath = shim(); + deepStrictEqual(detectVitePlusProject(root), { root, vpPath }); + }); +}); diff --git a/tests/unit/findBinary.spec.ts b/tests/unit/findBinary.spec.ts index fb07be96..cf0474d5 100644 --- a/tests/unit/findBinary.spec.ts +++ b/tests/unit/findBinary.spec.ts @@ -10,12 +10,30 @@ import { searchEnvPath, searchProjectNodeModulesBin, searchYarnPnpBin, + searchSettingsBin, } from "../../client/findBinary"; import { WORKSPACE_FOLDER } from "../test-helpers.js"; suite("findBinary", () => { const binaryName = "oxlint"; + test("prefers a Windows vp.cmd shim over the POSIX shim for a configured path", async () => { + const originalPlatform = process.platform; + const dir = mkdtempSync(path.join(tmpdir(), "test-vp-cmd-")); + const vpPath = path.join(dir, "vp"); + writeFileSync(vpPath, ""); + writeFileSync(`${vpPath}.cmd`, ""); + try { + Object.defineProperty(process, "platform", { value: "win32" }); + const result = await searchSettingsBin("vp", vpPath); + strictEqual(result?.path, `${vpPath}.cmd`); + strictEqual(result?.loader, "native"); + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform }); + rmSync(dir, { recursive: true, force: true }); + } + }); + suite("replaceTargetFromMainToBin", () => { let tmpDir: string; diff --git a/tests/unit/lsp_helper.spec.ts b/tests/unit/lsp_helper.spec.ts index 5c442637..6d2181b7 100644 --- a/tests/unit/lsp_helper.spec.ts +++ b/tests/unit/lsp_helper.spec.ts @@ -1,4 +1,4 @@ -import { strictEqual } from "assert"; +import { deepStrictEqual, strictEqual } from "assert"; import { runExecutable } from "../../client/tools/lsp_helper"; import * as path from "node:path"; import { pathToFileURL } from "node:url"; @@ -12,6 +12,57 @@ suite("runExecutable", () => { process.env = originalEnv; }); + for (const command of ["lint", "fmt"] as const) { + test(`runs vp ${command} --lsp in the project directory`, async () => { + const result = await runExecutable({ + path: "/project/node_modules/.bin/vp", + loader: "native", + vitePlus: command, + cwd: "/project", + }); + deepStrictEqual(result.args, [command, "--lsp"]); + strictEqual(result.options?.cwd, "/project"); + }); + + test(`runs the vp JavaScript entry point with ${command} --lsp and the configured runtime`, async () => { + const result = await runExecutable( + { + path: "/project/node_modules/vite-plus/bin/vp", + loader: "node", + vitePlus: command, + cwd: "/project", + }, + true, + ); + strictEqual(result.command, process.execPath); + deepStrictEqual(result.args, ["/project/node_modules/vite-plus/bin/vp", command, "--lsp"]); + strictEqual(result.options?.cwd, "/project"); + strictEqual(result.options?.env?.ELECTRON_RUN_AS_NODE, "1"); + }); + } + + test("does not interpret a vp shell shim as JavaScript with useExecPath", async () => { + Object.defineProperty(process, "platform", { value: "linux" }); + const result = await runExecutable( + { path: "/project/node_modules/.bin/vp", loader: "native", vitePlus: "lint" }, + true, + ); + strictEqual(result.command, "/project/node_modules/.bin/vp"); + deepStrictEqual(result.args, ["lint", "--lsp"]); + }); + + test("quotes Windows vp.cmd paths and passes the subcommand through the shell", async () => { + Object.defineProperty(process, "platform", { value: "win32" }); + const result = await runExecutable({ + path: "C:\\My Project\\node_modules\\.bin\\vp.cmd", + loader: "native", + vitePlus: "fmt", + }); + strictEqual(result.command, '"C:\\My Project\\node_modules\\.bin\\vp.cmd"'); + strictEqual(result.options?.shell, true); + deepStrictEqual(result.args, ["fmt", "--lsp"]); + }); + test("should create Node.js executable for .js files", async () => { const result = await runExecutable({ path: "/path/to/server.js", diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts new file mode 100644 index 00000000..360e5cf4 --- /dev/null +++ b/tests/unit/vitePlus.spec.ts @@ -0,0 +1,244 @@ +import { deepStrictEqual, rejects, strictEqual } from "assert"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import * as path from "node:path"; +import { mock } from "node:test"; +import { commands, ConfigurationTarget, Uri, window, workspace } from "vscode"; +import { ConfigService } from "../../client/ConfigService"; +import { WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER } from "../test-helpers"; + +suite("Vite+ server selection", () => { + const root = path.join(WORKSPACE_FOLDER.uri.fsPath, "vite-plus-tests"); + const secondRoot = + WORKSPACE_SECOND_FOLDER && path.join(WORKSPACE_SECOND_FOLDER.uri.fsPath, "vite-plus-tests"); + const conf = workspace.getConfiguration("oxc", WORKSPACE_FOLDER.uri); + const originalPath = process.env.PATH; + let service: ConfigService; + + function file(relative: string, content = "", dir = root): string { + const target = path.join(dir, relative); + mkdirSync(path.dirname(target), { recursive: true }); + writeFileSync(target, content); + return target; + } + + function declare(dir = root) { + file("package.json", JSON.stringify({ devDependencies: { "vite-plus": "latest" } }), dir); + } + + function shim(dir = root) { + return file( + path.join("node_modules/.bin", process.platform === "win32" ? "vp.cmd" : "vp"), + "", + dir, + ); + } + + async function open(dir = root) { + await window.showTextDocument(Uri.file(file("index.txt", "", dir))); + } + + setup(async () => { + file("pnpm-workspace.yaml"); + await open(); + service = new ConfigService(); + }); + + teardown(async () => { + service.dispose(); + mock.restoreAll(); + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + await commands.executeCommand("workbench.action.closeAllEditors"); + for (const folder of [WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER]) { + if (!folder) continue; + const config = workspace.getConfiguration("oxc", folder.uri); + // oxlint-disable-next-line no-await-in-loop -- reset each workspace folder + await Promise.all( + ["path.vp", "vitePlus.enable"].map((key) => + config.update(key, undefined, ConfigurationTarget.WorkspaceFolder), + ), + ); + } + await workspace.getConfiguration("oxc").update("path.oxlint", undefined); + await workspace.getConfiguration("oxc").update("path.oxfmt", undefined); + rmSync(root, { recursive: true, force: true }); + if (secondRoot) rmSync(secondRoot, { recursive: true, force: true }); + }); + + test("uses vp lint/fmt for an active nested package", async () => { + declare(); + const vpPath = shim(); + const [lint, fmt] = await Promise.all([ + service.getOxlintServerBinPath(), + service.getOxfmtServerBinPath(), + ]); + deepStrictEqual(lint, { path: vpPath, loader: "native", cwd: root, vitePlus: "lint" }); + deepStrictEqual(fmt, { path: vpPath, loader: "native", cwd: root, vitePlus: "fmt" }); + }); + + test("explicit enable works without a package.json dependency", async () => { + const vpPath = shim(); + await conf.update("vitePlus.enable", true, ConfigurationTarget.WorkspaceFolder); + strictEqual((await service.getOxlintServerBinPath())?.path, vpPath); + strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, "fmt"); + }); + + test("an explicit vp path opts in and resolves against the workspace folder", async () => { + const vpPath = file("bin/custom-vp.js"); + await conf.update( + "path.vp", + "./vite-plus-tests/bin/custom-vp.js", + ConfigurationTarget.WorkspaceFolder, + ); + deepStrictEqual(await service.getOxlintServerBinPath(), { + path: vpPath, + loader: "node", + cwd: WORKSPACE_FOLDER.uri.fsPath, + vitePlus: "lint", + }); + await commands.executeCommand("workbench.action.closeAllEditors"); + strictEqual( + (await service.getOxfmtServerBinPath())?.path, + vpPath, + "uses workspace folders when no document is active", + ); + }); + + test("recognizes the extensionless vite-plus JavaScript entry point", async () => { + const vpPath = file("node_modules/vite-plus/bin/vp"); + await conf.update("path.vp", vpPath, ConfigurationTarget.WorkspaceFolder); + strictEqual((await service.getOxfmtServerBinPath())?.loader, "node"); + }); + + test("invalid explicit vp paths give an error instead of falling back", async () => { + declare(); + shim(); + await conf.update("path.vp", "./missing-vp", ConfigurationTarget.WorkspaceFolder); + await rejects(service.getOxlintServerBinPath(), /Invalid Vite\+ binary.*oxc.path.vp/); + await conf.update("path.vp", "../unsafe-vp", ConfigurationTarget.WorkspaceFolder); + await rejects(service.getOxfmtServerBinPath(), /Invalid Vite\+ binary/); + }); + + test("explicit tool paths take priority over Vite+ for each tool", async () => { + declare(); + shim(); + const lintPath = file("custom/oxlint.js"); + const fmtPath = file("custom/oxfmt.js"); + await workspace.getConfiguration("oxc").update("path.oxlint", lintPath); + strictEqual((await service.getOxlintServerBinPath())?.path, lintPath); + strictEqual((await service.getOxlintServerBinPath())?.vitePlus, undefined); + strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, "fmt"); + await workspace.getConfiguration("oxc").update("path.oxfmt", fmtPath); + strictEqual((await service.getOxfmtServerBinPath())?.path, fmtPath); + strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, undefined); + }); + + test("false disables automatic detection and an explicit vp path", async () => { + declare(); + const vpPath = shim(); + await conf.update("path.vp", vpPath, ConfigurationTarget.WorkspaceFolder); + await conf.update("vitePlus.enable", false, ConfigurationTarget.WorkspaceFolder); + strictEqual((await service.getOxlintServerBinPath())?.vitePlus, undefined); + strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, undefined); + }); + + test("root-declared-no-local-global-on-path", async () => { + declare(); + const binDir = path.join(root, "global-bin"); + const vpPath = file(process.platform === "win32" ? "vp.cmd" : "vp", "", binDir); + process.env.PATH = binDir; + deepStrictEqual(await service.getOxlintServerBinPath(), { + path: vpPath, + loader: "native", + cwd: root, + vitePlus: "lint", + }); + const localPath = shim(); + strictEqual( + (await service.getOxlintServerBinPath())?.path, + localPath, + "local install must take priority after a restart", + ); + }); + + test("global-vp-without-declaration", async () => { + const binDir = path.join(root, "global-bin"); + file(process.platform === "win32" ? "vp.cmd" : "vp", "", binDir); + process.env.PATH = binDir; + strictEqual((await service.getOxlintServerBinPath())?.vitePlus, undefined); + strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, undefined); + await conf.update("vitePlus.enable", true, ConfigurationTarget.WorkspaceFolder); + strictEqual( + (await service.getOxlintServerBinPath())?.vitePlus, + "lint", + "explicit opt-in permits global resolution without a dependency", + ); + }); + + test("missing local and global installs give an install hint, even if plain tools exist", async () => { + declare(); + process.env.PATH = root; + mock.method(require("node:child_process"), "spawnSync", () => ({ status: 1 })); + mock.method(require("node:os"), "homedir", () => root); + await rejects(service.getOxlintServerBinPath(), /Vite\+ selected.*pnpm install/); + await rejects(service.getOxfmtServerBinPath(), /Vite\+ selected.*pnpm install/); + const vpPath = shim(); + strictEqual( + (await service.getOxlintServerBinPath())?.path, + vpPath, + "missing installs must not be cached", + ); + }); + + test("resolves vp from a global vite-plus package, not a package named vp", async () => { + declare(); + process.env.PATH = root; + const globalModules = path.join(root, "global/node_modules"); + file( + "vite-plus/package.json", + JSON.stringify({ name: "vite-plus", bin: { vp: "bin/vp" } }), + globalModules, + ); + const vpPath = file("vite-plus/bin/vp", "", globalModules); + mock.method(require("node:child_process"), "spawnSync", () => ({ + status: 0, + stdout: globalModules, + })); + deepStrictEqual(await service.getOxfmtServerBinPath(), { + path: vpPath, + loader: "node", + cwd: root, + vitePlus: "fmt", + }); + }); + + test("reselects when navigating between Vite+ and plain workspace folders", async function () { + if (!secondRoot) this.skip(); + declare(); + const vpPath = shim(); + strictEqual((await service.getOxlintServerBinPath())?.path, vpPath); + file("pnpm-workspace.yaml", "", secondRoot!); + await open(secondRoot!); + strictEqual((await service.getOxlintServerBinPath())?.vitePlus, undefined); + await open(); + strictEqual((await service.getOxlintServerBinPath())?.path, vpPath); + }); + + test("explicit settings are scoped to the active workspace folder", async function () { + if (!secondRoot || !WORKSPACE_SECOND_FOLDER) this.skip(); + const firstPath = file("bin/vp.js"); + const secondPath = file("bin/vp.js", "", secondRoot!); + await conf.update( + "path.vp", + "./vite-plus-tests/bin/vp.js", + ConfigurationTarget.WorkspaceFolder, + ); + await workspace + .getConfiguration("oxc", WORKSPACE_SECOND_FOLDER!.uri) + .update("path.vp", "./vite-plus-tests/bin/vp.js", ConfigurationTarget.WorkspaceFolder); + strictEqual((await service.getOxlintServerBinPath())?.path, firstPath); + await open(secondRoot!); + strictEqual((await service.getOxlintServerBinPath())?.path, secondPath); + strictEqual((await service.getOxfmtServerBinPath())?.cwd, WORKSPACE_SECOND_FOLDER!.uri.fsPath); + }); +}); diff --git a/tests/unit/vitePlusLifecycle.spec.ts b/tests/unit/vitePlusLifecycle.spec.ts new file mode 100644 index 00000000..3bf3c316 --- /dev/null +++ b/tests/unit/vitePlusLifecycle.spec.ts @@ -0,0 +1,92 @@ +import { strictEqual } from "assert"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import * as path from "node:path"; +import { mock } from "node:test"; +import { window, workspace } from "vscode"; +import { ConfigService } from "../../client/ConfigService"; +import type { BinarySearchResult } from "../../client/findBinary"; +import StatusBarItemHandler from "../../client/StatusBarItemHandler"; +import Formatter from "../../client/tools/formatter"; +import Linter from "../../client/tools/linter"; +import { WORKSPACE_FOLDER } from "../test-helpers"; + +for (const [Tool, command, getter] of [ + [Linter, "lint", "getOxlintServerBinPath"], + [Formatter, "fmt", "getOxfmtServerBinPath"], +] as const) { + suite(`Vite+ ${command} lifecycle`, () => { + const root = path.join(WORKSPACE_FOLDER.uri.fsPath, `vp-${command}-lifecycle`); + let service: ConfigService; + let tool: Linter | Formatter; + let output: ReturnType; + let status: StatusBarItemHandler; + let selected: BinarySearchResult; + + setup(async () => { + // No server needs to run to exercise client replacement on navigation. + await workspace.getConfiguration("oxc").update("enable", false); + mkdirSync(root, { recursive: true }); + const vpPath = path.join(root, "vp.cjs"); + writeFileSync(vpPath, ""); + selected = { path: vpPath, loader: "node", vitePlus: command, cwd: root }; + service = new ConfigService(); + mock.method(service, getter, async () => selected); + const channel = window.createOutputChannel(`Vite+ ${command} lifecycle`, { log: true }); + output = channel; + status = new StatusBarItemHandler("test"); + tool = new Tool(channel, service, status); + await tool.activate(selected); + }); + + teardown(async () => { + await tool.deactivate(); + tool.dispose(); + service.dispose(); + output.dispose(); + status.dispose(); + mock.restoreAll(); + await workspace.getConfiguration("oxc").update("enable", undefined); + rmSync(root, { recursive: true, force: true }); + }); + + test("keeps the client for unchanged binaries and replaces it when the project or mode changes", async () => { + const activation = mock.method(tool, "activate", tool.activate.bind(tool)); + await tool.restart(true); + strictEqual(activation.mock.callCount(), 0); + + selected = { ...selected, cwd: path.join(root, "another-project") }; + await tool.restart(true); + strictEqual(activation.mock.callCount(), 1); + + selected = { path: selected.path, loader: "node" }; + await tool.restart(true); + strictEqual(activation.mock.callCount(), 2); + + await tool.restart(); + strictEqual( + activation.mock.callCount(), + 3, + "the restart command must allow an unchanged binary", + ); + }); + + test("waits for an ongoing restart before processing another restart or shutdown", async () => { + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const activate = tool.activate.bind(tool); + const activation = mock.method(tool, "activate", async (binary?: BinarySearchResult) => { + await gate; + await activate(binary); + }); + const first = tool.restart(); + const second = tool.restart(); + const shutdown = tool.deactivate(); + release(); + await Promise.all([first, second, shutdown]); + strictEqual(activation.mock.callCount(), 2); + strictEqual(tool.getLspVersion(), undefined); + }); + }); +} From 1e4eb400cb8f0bed5c6fa2ba09f631e8edb8b3d8 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 01:11:59 +0800 Subject: [PATCH 02/15] fix: address Vite+ runtime and discovery review findings --- README.md | 6 +- client/ConfigService.ts | 9 ++- client/bundledNode.ts | 35 +++++++++++ client/detectVitePlus.ts | 24 +++++++- client/extension.ts | 2 + client/findBinary.ts | 3 +- client/resolveVitePlusNodeEntry.ts | 41 +++++++++++++ client/tools/lsp_helper.ts | 11 +++- tests/unit/detectVitePlus.spec.ts | 22 +++++++ tests/unit/lsp_helper.spec.ts | 94 +++++++++++++++++++++++++++--- tests/unit/vitePlus.spec.ts | 35 +++++++++++ 11 files changed, 265 insertions(+), 17 deletions(-) create mode 100644 client/bundledNode.ts create mode 100644 client/resolveVitePlusNodeEntry.ts diff --git a/README.md b/README.md index 2e46b964..601b35ce 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ See the official [Oxlint editor setup](https://oxc.rs/docs/guide/usage/linter/ed ## Vite+ -For projects that declare `vite-plus` in `dependencies` or `devDependencies`, the extension runs `vp lint --lsp` and `vp fmt --lsp`. Detection starts from the active file's directory and stops at the monorepo root (`pnpm-workspace.yaml`, `package.json` with `workspaces`, or `lerna.json`). With no active workspace file, the extension checks the workspace folders in order. It checks the local `node_modules/.bin/vp` shim first, then `PATH` and global package installations. +For projects that declare `vite-plus` in `dependencies` or `devDependencies`, the extension runs `vp lint --lsp` and `vp fmt --lsp`. Detection starts from the active file's directory and stops at the monorepo root (`pnpm-workspace.yaml`, `package.json` with `workspaces`, or `lerna.json`). With no active workspace file, the extension checks the workspace folders in order. It checks the local `node_modules/.bin/vp` shim first, then the shell's `PATH` and global package installations. To select Vite+ without automatic dependency detection, add this to your workspace's `.vscode/settings.json`: @@ -33,10 +33,12 @@ You can also set `oxc.path.vp` to an absolute path or a path relative to the wor } ``` -On Windows, use `./node_modules/.bin/vp.cmd`. To run the JavaScript entry point with `oxc.path.node` or `oxc.useExecPath`, set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp`. +On Windows, use `./node_modules/.bin/vp.cmd`. With `oxc.useExecPath`, the extension runs npm and pnpm project entries and their Node subprocesses with VS Code's bundled Node. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. Both settings can differ between workspace folders. `oxc.vitePlus.enable` defaults to `null` (automatic detection); `false` disables Vite+ integration, including `oxc.path.vp`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority for their respective tools. +Forced mode uses the nearest `package.json` or monorepo root as its working directory, with the workspace folder as a fallback. Switching between source directories in the same package does not restart the servers. + The extension rechecks Vite+ when you switch files and restarts a server if its executable or project directory changes. It uses one server per tool for the window. If Vite+ is selected but `vp` is unavailable, the status item and output channels show an install hint. Install your dependencies and run the **Oxc: Restart oxlint Server** and **Oxc: Restart oxfmt Server** commands. A failed Vite+ launch shows an install or upgrade hint. Vite+ integration requires a trusted workspace, and `oxc.requireConfig` does not require a separate Oxlint configuration when Vite+ is selected. ## Oxlint diff --git a/client/ConfigService.ts b/client/ConfigService.ts index 7cf6499e..b3811e15 100644 --- a/client/ConfigService.ts +++ b/client/ConfigService.ts @@ -1,6 +1,7 @@ import * as path from "node:path"; import { ConfigurationChangeEvent, Uri, window, workspace, WorkspaceFolder } from "vscode"; import { detectVitePlusProject, VitePlusError } from "./detectVitePlus"; +import { getShellEnv } from "./getShellEnv"; import { DiagnosticPullMode } from "vscode-languageclient"; import { BinarySearchResult, @@ -175,13 +176,15 @@ export class ConfigService implements IDisposable { return { ...binary, cwd: folder.uri.fsPath }; } - const project = detectVitePlusProject(start, enabled === true); + const project = detectVitePlusProject(start, enabled === true, folder.uri.fsPath); if (!project) continue; // Global vp is eligible only after detection or explicit opt-in. + // oxlint-disable no-await-in-loop -- global lookup requires a Vite+ project const binary: BinarySearchResult | undefined = project.vpPath ? { path: project.vpPath, loader: "native" } - : // oxlint-disable-next-line no-await-in-loop -- global lookup requires a Vite+ project - ((await searchEnvPath("vp")) ?? (await searchGlobalNodeModulesBin("vp", "vite-plus"))); + : ((await searchEnvPath("vp", await getShellEnv())) ?? + (await searchGlobalNodeModulesBin("vp", "vite-plus"))); + // oxlint-enable no-await-in-loop if (!binary) { throw new VitePlusError( `Vite+ selected in ${project.root}, but no vp binary was found. Run your package manager's install command (for example, pnpm install), or set oxc.path.vp, then restart the Oxc servers.`, diff --git a/client/bundledNode.ts b/client/bundledNode.ts new file mode 100644 index 00000000..f082d21f --- /dev/null +++ b/client/bundledNode.ts @@ -0,0 +1,35 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import * as path from "node:path"; + +let shimDirectory: string | undefined; + +/** Make the bundled runtime available to vp's child processes as `node`. */ +export function bundledNodeDirectory(): string { + if (/^node(?:\.exe)?$/i.test(path.basename(process.execPath))) { + return path.dirname(process.execPath); + } + if (shimDirectory) return shimDirectory; + + const directory = mkdtempSync(path.join(tmpdir(), "oxc-node-")); + const isWindows = process.platform === "win32"; + // Invoke Electron at its original location so it can find its resources. + const script = isWindows + ? `@echo off\r\nsetlocal DisableDelayedExpansion\r\n"${process.execPath.replaceAll("%", "%%")}" %*\r\n` + : `#!/bin/sh\nexec '${process.execPath.replaceAll("'", "'\\''")}' "$@"\n`; + try { + writeFileSync(path.join(directory, isWindows ? "node.cmd" : "node"), script, { mode: 0o700 }); + } catch (error) { + rmSync(directory, { recursive: true, force: true }); + throw error; + } + shimDirectory = directory; + return directory; +} + +export function disposeBundledNode(): void { + if (shimDirectory) { + rmSync(shimDirectory, { recursive: true, force: true }); + shimDirectory = undefined; + } +} diff --git a/client/detectVitePlus.ts b/client/detectVitePlus.ts index cdb48239..5b4723f9 100644 --- a/client/detectVitePlus.ts +++ b/client/detectVitePlus.ts @@ -2,7 +2,7 @@ import { existsSync, readFileSync, statSync } from "node:fs"; import * as path from "node:path"; export interface VitePlusProject { - /** The ancestor that declares vite-plus, or the explicitly enabled directory. */ + /** The declaring ancestor, or the nearest package/workspace root in forced mode. */ root: string; /** Undefined means Vite+ is selected but is not installed locally. */ vpPath?: string; @@ -36,7 +36,11 @@ function isRootWorkspace(dir: string, pkg: PackageJson | null): boolean { * Global lookup belongs to the caller and must only run for a non-null result. * https://github.com/voidzero-dev/vite-plus/pull/1614 */ -export function detectVitePlusProject(start: string, enabled = false): VitePlusProject | null { +export function detectVitePlusProject( + start: string, + enabled = false, + workspaceFolder?: string, +): VitePlusProject | null { let dir = path.resolve(start); try { if (statSync(dir).isFile()) dir = path.dirname(dir); @@ -45,7 +49,21 @@ export function detectVitePlusProject(start: string, enabled = false): VitePlusP } let pkg = readPackageJson(dir); - if (!enabled) { + if (enabled) { + // Explicit opt-in skips dependency detection, but still needs a stable cwd + // when the active document moves between source directories. + const fallbackRoot = workspaceFolder ? path.resolve(workspaceFolder) : dir; + while (!pkg && !isRootWorkspace(dir, pkg)) { + const parent = path.dirname(dir); + if ((workspaceFolder && dir === fallbackRoot) || parent === dir) { + dir = fallbackRoot; + pkg = readPackageJson(dir); + break; + } + dir = parent; + pkg = readPackageJson(dir); + } + } else { while (!pkg?.dependencies?.["vite-plus"] && !pkg?.devDependencies?.["vite-plus"]) { if (isRootWorkspace(dir, pkg) || dir === path.dirname(dir)) return null; dir = path.dirname(dir); diff --git a/client/extension.ts b/client/extension.ts index e7635b37..9f358cd3 100644 --- a/client/extension.ts +++ b/client/extension.ts @@ -6,6 +6,7 @@ import StatusBarItemHandler from "./StatusBarItemHandler"; import Formatter from "./tools/formatter"; import Linter from "./tools/linter"; import ToolInterface from "./tools/ToolInterface"; +import { disposeBundledNode } from "./bundledNode"; const outputChannelName = "Oxc"; const tools: ToolInterface[] = []; @@ -145,4 +146,5 @@ export async function activate(context: ExtensionContext) { export async function deactivate(): Promise { await Promise.all(tools.map((tool) => tool.deactivate())); tools.length = 0; + disposeBundledNode(); } diff --git a/client/findBinary.ts b/client/findBinary.ts index 8dc55de8..9c9b4369 100644 --- a/client/findBinary.ts +++ b/client/findBinary.ts @@ -253,8 +253,9 @@ export async function searchGlobalNodeModulesBin( */ export async function searchEnvPath( defaultBinaryName: string, + environment: Record = env, ): Promise { - const envPath = env.PATH; + const envPath = environment.PATH; if (!envPath) { return undefined; diff --git a/client/resolveVitePlusNodeEntry.ts b/client/resolveVitePlusNodeEntry.ts new file mode 100644 index 00000000..c6a72e19 --- /dev/null +++ b/client/resolveVitePlusNodeEntry.ts @@ -0,0 +1,41 @@ +import { closeSync, openSync, readSync } from "node:fs"; +import * as path from "node:path"; + +/** Resolve Node entry points without interpreting shell shims or native vp binaries as JS. */ +export function resolveVitePlusNodeEntry(vpPath: string): string | undefined { + const binDir = path.dirname(vpPath); + const candidates = [vpPath]; + if (path.basename(binDir) === ".bin") { + // pnpm and Windows npm shims sit next to the installed package. + candidates.push(path.resolve(binDir, "..", "vite-plus", "bin", "vp")); + } else if (path.extname(vpPath) === ".cmd") { + // Global npm shims on Windows sit next to node_modules. + candidates.push(path.join(binDir, "node_modules", "vite-plus", "bin", "vp")); + } + + for (const candidate of candidates) { + let fd: number | undefined; + try { + fd = openSync(candidate, "r"); + // Read only the shebang: a standalone native vp can be a large file. + const buffer = Buffer.alloc(256); + const length = readSync(fd, buffer, 0, buffer.length, 0); + const shebang = buffer.toString("utf8", 0, length).split(/\r?\n/, 1)[0]; + if (/^#!\s*(?:\S*\/node|\S*\/env\s+(?:-S\s+)?node)(?:\s|$)/.test(shebang)) return candidate; + if ( + candidate === vpPath && + path.extname(vpPath) !== ".cmd" && + !/^#!.*(?:\/|\s)(?:sh|bash|dash|zsh|ksh)(?:\s|$)/.test(shebang) + ) { + // A native vp in .bin must not be replaced by an adjacent JS package. + return undefined; + } + } catch { + // The candidate is absent or unreadable; keep the original executable. + return undefined; + } finally { + if (fd !== undefined) closeSync(fd); + } + } + return undefined; +} diff --git a/client/tools/lsp_helper.ts b/client/tools/lsp_helper.ts index 119d2442..16597a86 100644 --- a/client/tools/lsp_helper.ts +++ b/client/tools/lsp_helper.ts @@ -4,6 +4,8 @@ import { LogOutputChannel, window } from "vscode"; import { Executable, MessageType, ShowMessageParams } from "vscode-languageclient/node"; import type { BinarySearchResult } from "../findBinary"; import { getShellEnv } from "../getShellEnv"; +import { resolveVitePlusNodeEntry } from "../resolveVitePlusNodeEntry"; +import { bundledNodeDirectory } from "../bundledNode"; export async function runExecutable( binary: BinarySearchResult, @@ -12,6 +14,10 @@ export async function runExecutable( tsgolintPath?: string, suppressProgramErrors?: boolean, ): Promise { + if (binary.vitePlus && useExecPath && binary.loader === "native") { + const nodeEntry = resolveVitePlusNodeEntry(binary.path); + if (nodeEntry) binary = { ...binary, path: nodeEntry, loader: "node" }; + } const shellEnv = await getShellEnv(); const serverEnv: Record = { @@ -42,7 +48,10 @@ export async function runExecutable( } if (path.isAbsolute(nodeCommand)) { - const nodeDir = path.dirname(nodeCommand); + // vp also starts Node by name internally. Electron's executable is usually + // named Code/Code Helper, so its directory alone does not provide `node`. + const nodeDir = + binary.vitePlus && useExecPath ? bundledNodeDirectory() : path.dirname(nodeCommand); serverEnv.PATH = `${nodeDir}${path.delimiter}${serverEnv.PATH ?? ""}`; } diff --git a/tests/unit/detectVitePlus.spec.ts b/tests/unit/detectVitePlus.spec.ts index 8afc85f0..b0bfb5c3 100644 --- a/tests/unit/detectVitePlus.spec.ts +++ b/tests/unit/detectVitePlus.spec.ts @@ -129,6 +129,28 @@ suite("detectVitePlusProject", () => { deepStrictEqual(detectVitePlusProject(root, true), { root, vpPath }); }); + for (const marker of ["package", "workspace", "none"]) { + test(`forced mode has a stable project root with ${marker} metadata`, () => { + if (marker !== "workspace") rmSync(path.join(root, "pnpm-workspace.yaml")); + if (marker === "package") pkg("", {}); + const vpPath = shim(); + const firstFile = file("src/pages/index.ts"); + const secondFile = file("src/components/button.ts"); + deepStrictEqual(detectVitePlusProject(firstFile, true, root), { root, vpPath }); + deepStrictEqual(detectVitePlusProject(secondFile, true, root), { root, vpPath }); + }); + } + + test("forced mode retains a nested package root and a hoisted install", () => { + pkg("packages/app", {}); + const vpPath = shim(); + const start = file("packages/app/src/index.ts"); + deepStrictEqual(detectVitePlusProject(start, true, root), { + root: path.join(root, "packages/app"), + vpPath, + }); + }); + test("prefers the nearest declaring package and its install", () => { pkg(); shim(); diff --git a/tests/unit/lsp_helper.spec.ts b/tests/unit/lsp_helper.spec.ts index 6d2181b7..26378186 100644 --- a/tests/unit/lsp_helper.spec.ts +++ b/tests/unit/lsp_helper.spec.ts @@ -1,13 +1,33 @@ import { deepStrictEqual, strictEqual } from "assert"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { mock } from "node:test"; import { runExecutable } from "../../client/tools/lsp_helper"; +import { disposeBundledNode } from "../../client/bundledNode"; import * as path from "node:path"; import { pathToFileURL } from "node:url"; +// Mock the shared CommonJS exports used by the compiled test modules. +const shellEnv: typeof import("../../client/getShellEnv") = require( + path.join(__dirname, "../client/getShellEnv.js"), +); + suite("runExecutable", () => { const originalPlatform = process.platform; const originalEnv = process.env; + let tempDir: string; + + setup(() => { + process.env = { ...originalEnv }; + tempDir = mkdtempSync(path.join(tmpdir(), "vp-runtime-")); + mock.method(shellEnv, "getShellEnv", async () => ({ PATH: process.env.PATH })); + }); teardown(() => { + mock.restoreAll(); + disposeBundledNode(); + rmSync(tempDir, { recursive: true, force: true }); Object.defineProperty(process, "platform", { value: originalPlatform }); process.env = originalEnv; }); @@ -41,13 +61,73 @@ suite("runExecutable", () => { }); } - test("does not interpret a vp shell shim as JavaScript with useExecPath", async () => { - Object.defineProperty(process, "platform", { value: "linux" }); - const result = await runExecutable( - { path: "/project/node_modules/.bin/vp", loader: "native", vitePlus: "lint" }, - true, - ); - strictEqual(result.command, "/project/node_modules/.bin/vp"); + for (const shimType of ["npm", "pnpm", "cmd", "global-cmd"] as const) { + for (const command of ["lint", "fmt"] as const) { + test(`runs ${shimType} vp ${command} with bundled Node and no system Node`, async function () { + if (shimType === "npm" && originalPlatform === "win32") this.skip(); + const nodeEntry = path.join(tempDir, "node_modules", "vite-plus", "bin", "vp"); + const shim = + shimType === "global-cmd" + ? path.join(tempDir, "vp.cmd") + : path.join(tempDir, "node_modules", ".bin", shimType === "cmd" ? "vp.cmd" : "vp"); + mkdirSync(path.dirname(nodeEntry), { recursive: true }); + mkdirSync(path.dirname(shim), { recursive: true }); + writeFileSync( + path.join(path.dirname(nodeEntry), "child.cjs"), + "process.stdout.write(JSON.stringify(process.argv.slice(2)));\n", + ); + writeFileSync( + nodeEntry, + `#!/usr/bin/env node +const { spawnSync } = require("node:child_process"); +const args = process.argv.slice(2); +const script = require("node:path").join(__dirname, "child.cjs"); +const child = spawnSync("node", [process.platform === "win32" ? '"' + script + '"' : script, ...args], { + encoding: "utf8", + shell: process.platform === "win32", +}); +if (child.error) throw child.error; +process.stdout.write(child.stdout); +process.stderr.write(child.stderr); +process.exit(child.status ?? 1); +`, + ); + if (shimType === "npm") symlinkSync(nodeEntry, shim); + else + writeFileSync( + shim, + shimType === "pnpm" ? '#!/bin/sh\nexec node "$@"\n' : "@echo off\r\nnode %*\r\n", + ); + process.env.PATH = path.join(tempDir, "no-system-node"); + + const result = await runExecutable( + { path: shim, loader: "native", vitePlus: command, cwd: tempDir }, + true, + ); + strictEqual(result.command, process.execPath); + deepStrictEqual(result.args, [shimType === "npm" ? shim : nodeEntry, command, "--lsp"]); + strictEqual(result.options?.env?.ELECTRON_RUN_AS_NODE, "1"); + // Start both the entry point and its Node subprocess, as vp does. + const child = spawnSync(result.command, result.args, { + ...result.options, + encoding: "utf8", + timeout: 5000, + }); + strictEqual(child.status, 0, child.stderr || child.error?.message); + deepStrictEqual(JSON.parse(child.stdout), [command, "--lsp"]); + }); + } + } + + test("keeps a standalone native vp executable with useExecPath", async () => { + const vpPath = path.join(tempDir, "node_modules", ".bin", "vp"); + const nodeEntry = path.join(tempDir, "node_modules", "vite-plus", "bin", "vp"); + mkdirSync(path.dirname(vpPath), { recursive: true }); + mkdirSync(path.dirname(nodeEntry), { recursive: true }); + writeFileSync(vpPath, Buffer.from([0x7f, 0x45, 0x4c, 0x46])); + writeFileSync(nodeEntry, "#!/usr/bin/env node\n"); + const result = await runExecutable({ path: vpPath, loader: "native", vitePlus: "lint" }, true); + strictEqual(result.command, process.platform === "win32" ? `"${vpPath}"` : vpPath); deepStrictEqual(result.args, ["lint", "--lsp"]); }); diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts index 360e5cf4..a1c5e3af 100644 --- a/tests/unit/vitePlus.spec.ts +++ b/tests/unit/vitePlus.spec.ts @@ -4,8 +4,14 @@ import * as path from "node:path"; import { mock } from "node:test"; import { commands, ConfigurationTarget, Uri, window, workspace } from "vscode"; import { ConfigService } from "../../client/ConfigService"; +import { runExecutable } from "../../client/tools/lsp_helper"; import { WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER } from "../test-helpers"; +// Mock the shared CommonJS exports used by the compiled test modules. +const shellEnv: typeof import("../../client/getShellEnv") = require( + path.join(__dirname, "../client/getShellEnv.js"), +); + suite("Vite+ server selection", () => { const root = path.join(WORKSPACE_FOLDER.uri.fsPath, "vite-plus-tests"); const secondRoot = @@ -38,6 +44,7 @@ suite("Vite+ server selection", () => { } setup(async () => { + mock.method(shellEnv, "getShellEnv", async () => ({ PATH: process.env.PATH })); file("pnpm-workspace.yaml"); await open(); service = new ConfigService(); @@ -83,6 +90,18 @@ suite("Vite+ server selection", () => { strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, "fmt"); }); + test("explicit enable keeps the same executable and cwd across source directories", async () => { + file("package.json", "{}"); + const vpPath = shim(); + await conf.update("vitePlus.enable", true, ConfigurationTarget.WorkspaceFolder); + await open(path.join(root, "src/pages")); + const first = await service.getOxlintServerBinPath(); + await open(path.join(root, "src/components")); + deepStrictEqual(await service.getOxlintServerBinPath(), first); + deepStrictEqual(first, { path: vpPath, loader: "native", cwd: root, vitePlus: "lint" }); + strictEqual((await service.getOxfmtServerBinPath())?.cwd, root); + }); + test("an explicit vp path opts in and resolves against the workspace folder", async () => { const vpPath = file("bin/custom-vp.js"); await conf.update( @@ -175,6 +194,22 @@ suite("Vite+ server selection", () => { ); }); + test("discovers global vp using the same shell PATH as the launcher", async () => { + declare(); + const shellBin = path.join(root, "shell-bin"); + const vpPath = file(process.platform === "win32" ? "vp.cmd" : "vp", "", shellBin); + process.env.PATH = path.join(root, "inherited-bin"); + mock.method(shellEnv, "getShellEnv", async () => ({ PATH: shellBin })); + const binary = await service.getOxlintServerBinPath(); + strictEqual(binary?.path, vpPath); + strictEqual((await runExecutable(binary!)).options?.env?.PATH, shellBin); + strictEqual( + process.env.PATH, + path.join(root, "inherited-bin"), + "shell discovery must not mutate the extension host environment", + ); + }); + test("missing local and global installs give an install hint, even if plain tools exist", async () => { declare(); process.env.PATH = root; From d50a84fd1f5f510c631b57c76944feba66e683f0 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 01:40:24 +0800 Subject: [PATCH 03/15] fix: remove Node shims and handle Vite+ project changes --- README.md | 4 +- client/ConfigService.ts | 63 +++++++++++++--------- client/bundledNode.ts | 35 ------------ client/detectVitePlus.ts | 12 ++--- client/extension.ts | 31 +++++++---- client/tools/lsp_helper.ts | 6 +-- tests/unit/detectVitePlus.spec.ts | 12 +++++ tests/unit/extension.spec.ts | 90 +++++++++++++++++++++++++++++++ tests/unit/lsp_helper.spec.ts | 7 +-- tests/unit/vitePlus.spec.ts | 48 +++++++++++++++++ 10 files changed, 221 insertions(+), 87 deletions(-) delete mode 100644 client/bundledNode.ts create mode 100644 tests/unit/extension.spec.ts diff --git a/README.md b/README.md index 601b35ce..7cec1a4d 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,9 @@ You can also set `oxc.path.vp` to an absolute path or a path relative to the wor } ``` -On Windows, use `./node_modules/.bin/vp.cmd`. With `oxc.useExecPath`, the extension runs npm and pnpm project entries and their Node subprocesses with VS Code's bundled Node. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. +On Windows, use `./node_modules/.bin/vp.cmd` for npm or pnpm, or `./node_modules/.bin/vp.exe` for Bun. + +With `oxc.useExecPath`, the extension runs npm and pnpm project entries with VS Code's bundled Node. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. Running without a system Node installation requires a Vite+ release that includes [the runtime reuse fix](https://github.com/voidzero-dev/vite-plus/pull/2673). Earlier releases still require `node` on `PATH` for their subprocesses. Both settings can differ between workspace folders. `oxc.vitePlus.enable` defaults to `null` (automatic detection); `false` disables Vite+ integration, including `oxc.path.vp`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority for their respective tools. diff --git a/client/ConfigService.ts b/client/ConfigService.ts index b3811e15..94f1dbb6 100644 --- a/client/ConfigService.ts +++ b/client/ConfigService.ts @@ -19,6 +19,13 @@ import { WorkspaceConfig, } from "./WorkspaceConfig"; +interface VitePlusSearchFolder { + root: string; + start: string; + enabled: boolean | null | undefined; + configuredPath: string | undefined; +} + export class ConfigService implements IDisposable { public static readonly namespace = "oxc"; private readonly _disposables: IDisposable[] = []; @@ -26,7 +33,7 @@ export class ConfigService implements IDisposable { public vsCodeConfig: VSCodeConfig; private workspaceConfigs: Map = new Map(); - private vitePlusSearch: Promise | undefined; + private readonly vitePlusSearches = new Map>(); public onConfigChange: | ((this: ConfigService, config: ConfigurationChangeEvent) => Promise) @@ -140,43 +147,51 @@ export class ConfigService implements IDisposable { } private async searchVitePlus(): Promise { - // Lint and fmt share concurrent discovery, but restarts always re-read disk. - if (this.vitePlusSearch) return this.vitePlusSearch; - const search = this.resolveVitePlus(); - this.vitePlusSearch = search; - try { - return await search; - } finally { - this.vitePlusSearch = undefined; - } - } - - private async resolveVitePlus(): Promise { if (!workspace.isTrusted) return null; const documentUri = window.activeTextEditor?.document.uri; const activeFolder = documentUri?.scheme === "file" ? workspace.getWorkspaceFolder(documentUri) : undefined; - const folders = activeFolder ? [activeFolder] : (workspace.workspaceFolders ?? []); + const folders = (activeFolder ? [activeFolder] : (workspace.workspaceFolders ?? [])).map( + (folder): VitePlusSearchFolder => { + const config = workspace.getConfiguration(ConfigService.namespace, folder.uri); + return { + root: folder.uri.fsPath, + start: activeFolder && documentUri ? path.dirname(documentUri.fsPath) : folder.uri.fsPath, + enabled: config.get("vitePlus.enable"), + configuredPath: config.get("path.vp"), + }; + }, + ); + // Share only searches with the same document context and settings. A slow + // search for another project must not select its binary after navigation. + const key = JSON.stringify(folders); + const pending = this.vitePlusSearches.get(key); + if (pending) return pending; + const search = this.resolveVitePlus(folders); + this.vitePlusSearches.set(key, search); + try { + return await search; + } finally { + this.vitePlusSearches.delete(key); + } + } - for (const folder of folders) { - const config = workspace.getConfiguration(ConfigService.namespace, folder.uri); - const enabled = config.get("vitePlus.enable"); + private async resolveVitePlus( + folders: VitePlusSearchFolder[], + ): Promise { + for (const { root, start, enabled, configuredPath } of folders) { if (enabled === false) continue; - - const configuredPath = config.get("path.vp"); - const start = - activeFolder && documentUri ? path.dirname(documentUri.fsPath) : folder.uri.fsPath; if (configuredPath) { // An explicit vp path opts in without requiring a dependency declaration. // oxlint-disable-next-line no-await-in-loop -- workspace folder order is significant - const binary = await searchSettingsBin("vp", configuredPath, folder.uri.fsPath); + const binary = await searchSettingsBin("vp", configuredPath, root); if (!binary) throw new VitePlusError(`Invalid Vite+ binary: ${configuredPath}. Check oxc.path.vp.`); - return { ...binary, cwd: folder.uri.fsPath }; + return { ...binary, cwd: root }; } - const project = detectVitePlusProject(start, enabled === true, folder.uri.fsPath); + const project = detectVitePlusProject(start, enabled === true, root); if (!project) continue; // Global vp is eligible only after detection or explicit opt-in. // oxlint-disable no-await-in-loop -- global lookup requires a Vite+ project diff --git a/client/bundledNode.ts b/client/bundledNode.ts deleted file mode 100644 index f082d21f..00000000 --- a/client/bundledNode.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import * as path from "node:path"; - -let shimDirectory: string | undefined; - -/** Make the bundled runtime available to vp's child processes as `node`. */ -export function bundledNodeDirectory(): string { - if (/^node(?:\.exe)?$/i.test(path.basename(process.execPath))) { - return path.dirname(process.execPath); - } - if (shimDirectory) return shimDirectory; - - const directory = mkdtempSync(path.join(tmpdir(), "oxc-node-")); - const isWindows = process.platform === "win32"; - // Invoke Electron at its original location so it can find its resources. - const script = isWindows - ? `@echo off\r\nsetlocal DisableDelayedExpansion\r\n"${process.execPath.replaceAll("%", "%%")}" %*\r\n` - : `#!/bin/sh\nexec '${process.execPath.replaceAll("'", "'\\''")}' "$@"\n`; - try { - writeFileSync(path.join(directory, isWindows ? "node.cmd" : "node"), script, { mode: 0o700 }); - } catch (error) { - rmSync(directory, { recursive: true, force: true }); - throw error; - } - shimDirectory = directory; - return directory; -} - -export function disposeBundledNode(): void { - if (shimDirectory) { - rmSync(shimDirectory, { recursive: true, force: true }); - shimDirectory = undefined; - } -} diff --git a/client/detectVitePlus.ts b/client/detectVitePlus.ts index 5b4723f9..be578acf 100644 --- a/client/detectVitePlus.ts +++ b/client/detectVitePlus.ts @@ -73,13 +73,11 @@ export function detectVitePlusProject( const root = dir; while (true) { - const vpPath = path.join( - dir, - "node_modules", - ".bin", - process.platform === "win32" ? "vp.cmd" : "vp", - ); - if (existsSync(vpPath)) return { root, vpPath }; + const binNames = process.platform === "win32" ? ["vp.cmd", "vp.exe"] : ["vp"]; + for (const name of binNames) { + const vpPath = path.join(dir, "node_modules", ".bin", name); + if (existsSync(vpPath)) return { root, vpPath }; + } if (isRootWorkspace(dir, pkg) || dir === path.dirname(dir)) return { root }; dir = path.dirname(dir); pkg = readPackageJson(dir); diff --git a/client/extension.ts b/client/extension.ts index 9f358cd3..c9cfa580 100644 --- a/client/extension.ts +++ b/client/extension.ts @@ -6,7 +6,6 @@ import StatusBarItemHandler from "./StatusBarItemHandler"; import Formatter from "./tools/formatter"; import Linter from "./tools/linter"; import ToolInterface from "./tools/ToolInterface"; -import { disposeBundledNode } from "./bundledNode"; const outputChannelName = "Oxc"; const tools: ToolInterface[] = []; @@ -110,6 +109,7 @@ export async function activate(context: ExtensionContext) { outputChannelFormat.info("Searching for oxfmt binary."); outputChannelLint.info("Searching for oxlint binary."); + const initialDocument = window.activeTextEditor?.document.uri.toString(); const binaryPaths = await Promise.all(tools.map((tool) => tool.getBinary())); await Promise.all( @@ -121,6 +121,20 @@ export async function activate(context: ExtensionContext) { // A window has one client per tool. Re-resolve on navigation, and restart // only when the executable, Vite+ command, or project directory changes. + const switchProject = async () => { + await Promise.all( + tools.map(async (tool) => { + try { + await tool.restart(true); + } catch (error) { + const output = tool instanceof Linter ? outputChannelLint : outputChannelFormat; + output.error( + `Failed to switch language server: ${error instanceof Error ? error.message : String(error)}`, + ); + } + }), + ); + }; context.subscriptions.push( window.onDidChangeActiveTextEditor((editor) => { if ( @@ -128,17 +142,15 @@ export async function activate(context: ExtensionContext) { !workspace.getWorkspaceFolder(editor.document.uri) ) return; - for (const tool of tools) { - void tool.restart(true).catch((error) => { - const output = tool instanceof Linter ? outputChannelLint : outputChannelFormat; - output.error( - `Failed to switch language server: ${error instanceof Error ? error.message : String(error)}`, - ); - }); - } + void switchProject(); }), ); + // Navigation during binary discovery or server startup predates the listener. + if (window.activeTextEditor?.document.uri.toString() !== initialDocument) { + await switchProject(); + } + // Finally show the status bar item. statusBarItemHandler.show(); } @@ -146,5 +158,4 @@ export async function activate(context: ExtensionContext) { export async function deactivate(): Promise { await Promise.all(tools.map((tool) => tool.deactivate())); tools.length = 0; - disposeBundledNode(); } diff --git a/client/tools/lsp_helper.ts b/client/tools/lsp_helper.ts index 16597a86..08ae8cb9 100644 --- a/client/tools/lsp_helper.ts +++ b/client/tools/lsp_helper.ts @@ -5,7 +5,6 @@ import { Executable, MessageType, ShowMessageParams } from "vscode-languageclien import type { BinarySearchResult } from "../findBinary"; import { getShellEnv } from "../getShellEnv"; import { resolveVitePlusNodeEntry } from "../resolveVitePlusNodeEntry"; -import { bundledNodeDirectory } from "../bundledNode"; export async function runExecutable( binary: BinarySearchResult, @@ -48,10 +47,7 @@ export async function runExecutable( } if (path.isAbsolute(nodeCommand)) { - // vp also starts Node by name internally. Electron's executable is usually - // named Code/Code Helper, so its directory alone does not provide `node`. - const nodeDir = - binary.vitePlus && useExecPath ? bundledNodeDirectory() : path.dirname(nodeCommand); + const nodeDir = path.dirname(nodeCommand); serverEnv.PATH = `${nodeDir}${path.delimiter}${serverEnv.PATH ?? ""}`; } diff --git a/tests/unit/detectVitePlus.spec.ts b/tests/unit/detectVitePlus.spec.ts index b0bfb5c3..a3887868 100644 --- a/tests/unit/detectVitePlus.spec.ts +++ b/tests/unit/detectVitePlus.spec.ts @@ -166,7 +166,19 @@ suite("detectVitePlusProject", () => { Object.defineProperty(process, "platform", { value: "win32" }); pkg(); file("node_modules/.bin/vp"); + file("node_modules/.bin/vp.exe"); const vpPath = shim(); deepStrictEqual(detectVitePlusProject(root), { root, vpPath }); }); + + test("selects a local vp.exe shim on Windows before a hoisted vp.cmd", () => { + Object.defineProperty(process, "platform", { value: "win32" }); + pkg("packages/app"); + shim(); + const vpPath = file("packages/app/node_modules/.bin/vp.exe"); + deepStrictEqual(detectVitePlusProject(path.join(root, "packages/app")), { + root: path.join(root, "packages/app"), + vpPath, + }); + }); }); diff --git a/tests/unit/extension.spec.ts b/tests/unit/extension.spec.ts new file mode 100644 index 00000000..74828572 --- /dev/null +++ b/tests/unit/extension.spec.ts @@ -0,0 +1,90 @@ +import { deepStrictEqual, strictEqual } from "assert"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import * as path from "node:path"; +import { mock } from "node:test"; +import { commands, ExtensionContext, Uri, window } from "vscode"; +import { activate, deactivate } from "../../client/extension"; +import type { BinarySearchResult } from "../../client/findBinary"; +import Formatter from "../../client/tools/formatter"; +import Linter from "../../client/tools/linter"; +import { WORKSPACE_FOLDER } from "../test-helpers"; + +suite("navigation during extension activation", () => { + const root = path.join(WORKSPACE_FOLDER.uri.fsPath, "startup-navigation"); + const originalEnv = process.env; + let context: ExtensionContext; + + setup(() => { + process.env = { ...originalEnv, SKIP_LINTER_TEST: "false", SKIP_FORMATTER_TEST: "false" }; + mkdirSync(root, { recursive: true }); + for (const name of ["a", "b"]) writeFileSync(path.join(root, `${name}.txt`), ""); + // The test host has already registered the extension's commands. + mock.method(commands, "registerCommand", () => ({ dispose() {} })); + context = { + extension: { packageJSON: { version: "test" } }, + subscriptions: [], + } as unknown as ExtensionContext; + }); + + teardown(async () => { + await deactivate(); + for (const disposable of context.subscriptions) disposable.dispose(); + mock.restoreAll(); + process.env = originalEnv; + await commands.executeCommand("workbench.action.closeAllEditors"); + rmSync(root, { recursive: true, force: true }); + }); + + for (const phase of ["discovery", "startup"] as const) { + test(`reconciles a project switch during ${phase}`, async () => { + const first = Uri.file(path.join(root, "a.txt")); + const second = Uri.file(path.join(root, "b.txt")); + await window.showTextDocument(first); + let release!: () => void; + let entered!: () => void; + let waiting = 0; + const gate = new Promise((resolve) => { + release = resolve; + }); + const ready = new Promise((resolve) => { + entered = resolve; + }); + const wait = async () => { + if (++waiting === 2) entered(); + await gate; + }; + const selected = new Map(); + for (const [Tool, name] of [ + [Linter, "lint"], + [Formatter, "fmt"], + ] as const) { + mock.method(Tool.prototype, "getBinary", async () => { + const binary = { + path: window.activeTextEditor!.document.uri.fsPath, + loader: "native" as const, + }; + if (phase === "discovery") await wait(); + return binary; + }); + mock.method(Tool.prototype, "activate", async (binary?: BinarySearchResult) => { + selected.set(name, binary!.path); + if (phase === "startup") await wait(); + }); + mock.method(Tool.prototype, "restart", async (onlyIfBinaryChanged?: boolean) => { + strictEqual(onlyIfBinaryChanged, true); + selected.set(name, window.activeTextEditor!.document.uri.fsPath); + }); + mock.method(Tool.prototype, "deactivate", async () => {}); + } + const activation = activate(context); + try { + await ready; + await window.showTextDocument(second); + } finally { + release(); + } + await activation; + deepStrictEqual([...selected.values()], [second.fsPath, second.fsPath]); + }); + } +}); diff --git a/tests/unit/lsp_helper.spec.ts b/tests/unit/lsp_helper.spec.ts index 26378186..c5cbdff5 100644 --- a/tests/unit/lsp_helper.spec.ts +++ b/tests/unit/lsp_helper.spec.ts @@ -4,7 +4,6 @@ import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node import { tmpdir } from "node:os"; import { mock } from "node:test"; import { runExecutable } from "../../client/tools/lsp_helper"; -import { disposeBundledNode } from "../../client/bundledNode"; import * as path from "node:path"; import { pathToFileURL } from "node:url"; @@ -26,7 +25,6 @@ suite("runExecutable", () => { teardown(() => { mock.restoreAll(); - disposeBundledNode(); rmSync(tempDir, { recursive: true, force: true }); Object.defineProperty(process, "platform", { value: originalPlatform }); process.env = originalEnv; @@ -82,9 +80,8 @@ suite("runExecutable", () => { const { spawnSync } = require("node:child_process"); const args = process.argv.slice(2); const script = require("node:path").join(__dirname, "child.cjs"); -const child = spawnSync("node", [process.platform === "win32" ? '"' + script + '"' : script, ...args], { +const child = spawnSync(process.execPath, [script, ...args], { encoding: "utf8", - shell: process.platform === "win32", }); if (child.error) throw child.error; process.stdout.write(child.stdout); @@ -107,7 +104,7 @@ process.exit(child.status ?? 1); strictEqual(result.command, process.execPath); deepStrictEqual(result.args, [shimType === "npm" ? shim : nodeEntry, command, "--lsp"]); strictEqual(result.options?.env?.ELECTRON_RUN_AS_NODE, "1"); - // Start both the entry point and its Node subprocess, as vp does. + // vp must reuse process.execPath for its subprocess, without PATH shims. const child = spawnSync(result.command, result.args, { ...result.options, encoding: "utf8", diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts index a1c5e3af..98e66f43 100644 --- a/tests/unit/vitePlus.spec.ts +++ b/tests/unit/vitePlus.spec.ts @@ -11,6 +11,9 @@ import { WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER } from "../test-helpers"; const shellEnv: typeof import("../../client/getShellEnv") = require( path.join(__dirname, "../client/getShellEnv.js"), ); +const findBinary: typeof import("../../client/findBinary") = require( + path.join(__dirname, "../client/findBinary.js"), +); suite("Vite+ server selection", () => { const root = path.join(WORKSPACE_FOLDER.uri.fsPath, "vite-plus-tests"); @@ -276,4 +279,49 @@ suite("Vite+ server selection", () => { strictEqual((await service.getOxlintServerBinPath())?.path, secondPath); strictEqual((await service.getOxfmtServerBinPath())?.cwd, WORKSPACE_SECOND_FOLDER!.uri.fsPath); }); + + for (const change of ["none", "folder", "setting"] as const) { + test(`shares an ongoing search only when its context is unchanged (${change})`, async function () { + if (change === "folder" && (!secondRoot || !WORKSPACE_SECOND_FOLDER)) this.skip(); + const firstPath = file("bin/first-vp.js"); + const secondPath = file("bin/second-vp.js", "", change === "folder" ? secondRoot! : root); + await conf.update("path.vp", firstPath, ConfigurationTarget.WorkspaceFolder); + if (change === "folder") { + await workspace + .getConfiguration("oxc", WORKSPACE_SECOND_FOLDER!.uri) + .update("path.vp", secondPath, ConfigurationTarget.WorkspaceFolder); + } + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const searchSettingsBin = findBinary.searchSettingsBin; + let firstPathLookups = 0; + mock.method( + findBinary, + "searchSettingsBin", + async (name: string, configuredPath: string, cwd?: string) => { + if (configuredPath === firstPath) { + firstPathLookups++; + await gate; + } + return searchSettingsBin(name, configuredPath, cwd); + }, + ); + const first = service.getOxlintServerBinPath(); + let second: ReturnType; + try { + if (change === "folder") await open(secondRoot!); + if (change === "setting") { + await conf.update("path.vp", secondPath, ConfigurationTarget.WorkspaceFolder); + } + second = service.getOxfmtServerBinPath(); + } finally { + release(); + } + strictEqual((await first)?.path, firstPath); + strictEqual((await second!)?.path, change === "none" ? firstPath : secondPath); + strictEqual(firstPathLookups, 1, "matching lint/fmt requests should share discovery"); + }); + } }); From d8f16214e3e59afd116cf348bd83e0a9cf8fe57a Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 09:36:22 +0800 Subject: [PATCH 04/15] refactor: select lint and format binary sources independently --- README.md | 55 ++++++----- client/ConfigService.ts | 19 ++-- client/VSCodeConfig.ts | 3 +- package.json | 30 ++++-- tests/unit/VSCodeConfig.spec.ts | 11 ++- tests/unit/vitePlus.spec.ts | 157 +++++++++++++++++++++++++++++--- 6 files changed, 219 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 7cec1a4d..f3854baa 100644 --- a/README.md +++ b/README.md @@ -21,11 +21,21 @@ To select Vite+ without automatic dependency detection, add this to your workspa ```json { - "oxc.vitePlus.enable": true + "oxc.lint.binarySource": "vite-plus", + "oxc.fmt.binarySource": "vite-plus" } ``` -You can also set `oxc.path.vp` to an absolute path or a path relative to the workspace folder. This setting selects Vite+ without dependency detection: +Each tool has its own source setting. Both default to `"auto"`, which detects a direct `vite-plus` dependency and otherwise uses the standalone tool. Set a source to `"oxc"` to use the standalone tool even in a Vite+ project. For example, to use your own Oxlint installation with Vite+ formatting: + +```json +{ + "oxc.lint.binarySource": "oxc", + "oxc.fmt.binarySource": "vite-plus" +} +``` + +You can also set `oxc.path.vp` to an absolute path or a path relative to the workspace folder. This setting selects Vite+ without dependency detection for tools whose source is `"auto"` or `"vite-plus"`. A source of `"oxc"` ignores this path: ```json { @@ -37,9 +47,9 @@ On Windows, use `./node_modules/.bin/vp.cmd` for npm or pnpm, or `./node_modules With `oxc.useExecPath`, the extension runs npm and pnpm project entries with VS Code's bundled Node. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. Running without a system Node installation requires a Vite+ release that includes [the runtime reuse fix](https://github.com/voidzero-dev/vite-plus/pull/2673). Earlier releases still require `node` on `PATH` for their subprocesses. -Both settings can differ between workspace folders. `oxc.vitePlus.enable` defaults to `null` (automatic detection); `false` disables Vite+ integration, including `oxc.path.vp`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority for their respective tools. +The source settings and `oxc.path.vp` can differ between workspace folders. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority over source selection for their respective tools. Changing a source setting restarts only its tool. -Forced mode uses the nearest `package.json` or monorepo root as its working directory, with the workspace folder as a fallback. Switching between source directories in the same package does not restart the servers. +The `"vite-plus"` source uses the nearest `package.json` or monorepo root as its working directory, with the workspace folder as a fallback. Switching between source directories in the same package does not restart the servers. The extension rechecks Vite+ when you switch files and restarts a server if its executable or project directory changes. It uses one server per tool for the window. If Vite+ is selected but `vp` is unavailable, the status item and output channels show an install hint. Install your dependencies and run the **Oxc: Restart oxlint Server** and **Oxc: Restart oxfmt Server** commands. A failed Vite+ launch shows an install or upgrade hint. Vite+ integration requires a trusted workspace, and `oxc.requireConfig` does not require a separate Oxlint configuration when Vite+ is selected. @@ -126,24 +136,25 @@ Following configurations are supported via `settings.json` and affect the window Following configurations are supported via `settings.json` and can be changed for each workspace: -| Key | Default Value | Possible Values | Description | -| ----------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `oxc.configPath` | `null` | `` \| `` | Path to oxlint configuration. Keep it empty to enable nested configuration. | -| `oxc.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used. | -| `oxc.fixKind` | `null` | `safe_fix` \| `safe_fix_or_suggestion` \| `dangerous_fix` \| `dangerous_fix_or_suggestion` \| `none` \| `all` | Specify the kind of fixes to suggest/apply. | -| `oxc.fmt.configPath` | `null` | `` \| `` | Path to an oxfmt configuration file | -| `oxc.fmt.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used. | -| `oxc.lint.customization` | `null` | `Record` \| `` | Customizes linting rules behavior. See for details. | -| `oxc.lint.run` | `onType` | `onSave` \| `onType` | Run the linter on save (onSave) or on type (onType) | -| `oxc.path.vp` | - | `` | Path to a `vp` executable. Relative paths use the workspace folder. Setting this path selects Vite+ without automatic detection, unless `oxc.vitePlus.enable` is `false`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority. | -| `oxc.requireConfig` | `false` | `true` \| `false` | Start the language server only when a `.oxlintrc.json(c)` or `oxlint.config.ts` file exists in one of the workspaces. | -| `oxc.tsConfigPath` | `null` | `` \| `` | Path to the project's TypeScript config file. If your `tsconfig.json` is not at the root, you will need this set for the `import` plugin rules to resolve imports correctly. | -| `oxc.typeAware` | `null` | `true` \| `false` \| `` | Forces type-aware linting. Requires the `oxlint-tsgolint` package. It is preferred to use `options.typeAware` in your configuration file | -| `oxc.unusedDisableDirectives` | `null` | `allow` \| `warn` \| `deny` | Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway. It is preferred to use `options.reportUnusedDisableDirectives` in your configuration file | -| `oxc.vitePlus.enable` | `null` | `true` \| `false` \| `` | Use `vp lint --lsp` and `vp fmt --lsp`. Set to `true` to select Vite+ without detecting a dependency, `false` to disable Vite+ integration, or `null` to detect a direct `vite-plus` dependency. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority. | -| Deprecated | | | | -| `oxc.flags` | `{}` | `Record` | Specific Oxlint flags to pass to the language server. | -| `oxc.fmt.experimental` | `true` | `true` \| `false` | Enable Oxfmt formatting support. | +| Key | Default Value | Possible Values | Description | +| ----------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `oxc.configPath` | `null` | `` \| `` | Path to oxlint configuration. Keep it empty to enable nested configuration. | +| `oxc.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used. | +| `oxc.fixKind` | `null` | `safe_fix` \| `safe_fix_or_suggestion` \| `dangerous_fix` \| `dangerous_fix_or_suggestion` \| `none` \| `all` | Specify the kind of fixes to suggest/apply. | +| `oxc.fmt.binarySource` | `auto` | `auto` \| `vite-plus` \| `oxc` | Select the formatter source. `auto` detects a direct `vite-plus` dependency or uses `oxc.path.vp`, otherwise it finds standalone Oxfmt. `vite-plus` selects `vp fmt --lsp` without dependency detection. `oxc` finds standalone Oxfmt and ignores `oxc.path.vp`. An explicit `oxc.path.oxfmt` takes priority. | +| `oxc.fmt.configPath` | `null` | `` \| `` | Path to an oxfmt configuration file | +| `oxc.fmt.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used. | +| `oxc.lint.binarySource` | `auto` | `auto` \| `vite-plus` \| `oxc` | Select the linter source. `auto` detects a direct `vite-plus` dependency or uses `oxc.path.vp`, otherwise it finds standalone Oxlint. `vite-plus` selects `vp lint --lsp` without dependency detection. `oxc` finds standalone Oxlint and ignores `oxc.path.vp`. An explicit `oxc.path.oxlint` takes priority. | +| `oxc.lint.customization` | `null` | `Record` \| `` | Customizes linting rules behavior. See for details. | +| `oxc.lint.run` | `onType` | `onSave` \| `onType` | Run the linter on save (onSave) or on type (onType) | +| `oxc.path.vp` | - | `` | Path to a `vp` executable. Relative paths use the workspace folder. Setting this path selects Vite+ without dependency detection for tools whose `binarySource` is `auto` or `vite-plus`. Tools whose `binarySource` is `oxc` ignore this path. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority. | +| `oxc.requireConfig` | `false` | `true` \| `false` | Start the language server only when a `.oxlintrc.json(c)` or `oxlint.config.ts` file exists in one of the workspaces. | +| `oxc.tsConfigPath` | `null` | `` \| `` | Path to the project's TypeScript config file. If your `tsconfig.json` is not at the root, you will need this set for the `import` plugin rules to resolve imports correctly. | +| `oxc.typeAware` | `null` | `true` \| `false` \| `` | Forces type-aware linting. Requires the `oxlint-tsgolint` package. It is preferred to use `options.typeAware` in your configuration file | +| `oxc.unusedDisableDirectives` | `null` | `allow` \| `warn` \| `deny` | Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway. It is preferred to use `options.reportUnusedDisableDirectives` in your configuration file | +| Deprecated | | | | +| `oxc.flags` | `{}` | `Record` | Specific Oxlint flags to pass to the language server. | +| `oxc.fmt.experimental` | `true` | `true` \| `false` | Enable Oxfmt formatting support. | #### FixKind diff --git a/client/ConfigService.ts b/client/ConfigService.ts index 94f1dbb6..1280042f 100644 --- a/client/ConfigService.ts +++ b/client/ConfigService.ts @@ -19,10 +19,12 @@ import { WorkspaceConfig, } from "./WorkspaceConfig"; +type BinarySource = "auto" | "vite-plus" | "oxc"; + interface VitePlusSearchFolder { root: string; start: string; - enabled: boolean | null | undefined; + source: BinarySource; configuredPath: string | undefined; } @@ -133,9 +135,10 @@ export class ConfigService implements IDisposable { return searchSettingsBin(defaultBinaryName, settingsBinary); } - const vitePlus = await this.searchVitePlus(); + const command = defaultBinaryName === "oxlint" ? "lint" : "fmt"; + const vitePlus = await this.searchVitePlus(command); if (vitePlus) { - return { ...vitePlus, vitePlus: defaultBinaryName === "oxlint" ? "lint" : "fmt" }; + return { ...vitePlus, vitePlus: command }; } return ( @@ -146,7 +149,7 @@ export class ConfigService implements IDisposable { ); } - private async searchVitePlus(): Promise { + private async searchVitePlus(command: "lint" | "fmt"): Promise { if (!workspace.isTrusted) return null; const documentUri = window.activeTextEditor?.document.uri; @@ -158,7 +161,7 @@ export class ConfigService implements IDisposable { return { root: folder.uri.fsPath, start: activeFolder && documentUri ? path.dirname(documentUri.fsPath) : folder.uri.fsPath, - enabled: config.get("vitePlus.enable"), + source: config.get(`${command}.binarySource`) ?? "auto", configuredPath: config.get("path.vp"), }; }, @@ -180,8 +183,8 @@ export class ConfigService implements IDisposable { private async resolveVitePlus( folders: VitePlusSearchFolder[], ): Promise { - for (const { root, start, enabled, configuredPath } of folders) { - if (enabled === false) continue; + for (const { root, start, source, configuredPath } of folders) { + if (source === "oxc") continue; if (configuredPath) { // An explicit vp path opts in without requiring a dependency declaration. // oxlint-disable-next-line no-await-in-loop -- workspace folder order is significant @@ -191,7 +194,7 @@ export class ConfigService implements IDisposable { return { ...binary, cwd: root }; } - const project = detectVitePlusProject(start, enabled === true, root); + const project = detectVitePlusProject(start, source === "vite-plus", root); if (!project) continue; // Global vp is eligible only after detection or explicit opt-in. // oxlint-disable no-await-in-loop -- global lookup requires a Vite+ project diff --git a/client/VSCodeConfig.ts b/client/VSCodeConfig.ts index 17d11ab5..86609019 100644 --- a/client/VSCodeConfig.ts +++ b/client/VSCodeConfig.ts @@ -155,7 +155,6 @@ export class VSCodeConfig implements VSCodeConfigInterface { return ( event.affectsConfiguration(`${ConfigService.namespace}.path.node`) || event.affectsConfiguration(`${ConfigService.namespace}.path.vp`) || - event.affectsConfiguration(`${ConfigService.namespace}.vitePlus.enable`) || event.affectsConfiguration(`${ConfigService.namespace}.useExecPath`) ); } @@ -163,6 +162,7 @@ export class VSCodeConfig implements VSCodeConfigInterface { effectsOxlintConnection(event: ConfigurationChangeEvent): boolean { return ( event.affectsConfiguration(`${ConfigService.namespace}.path.oxlint`) || + event.affectsConfiguration(`${ConfigService.namespace}.lint.binarySource`) || event.affectsConfiguration(`${ConfigService.namespace}.path.tsgolint`) || this.effectsGeneralLSPConnection(event) ); @@ -171,6 +171,7 @@ export class VSCodeConfig implements VSCodeConfigInterface { effectsOxfmtConnection(event: ConfigurationChangeEvent): boolean { return ( event.affectsConfiguration(`${ConfigService.namespace}.path.oxfmt`) || + event.affectsConfiguration(`${ConfigService.namespace}.fmt.binarySource`) || this.effectsGeneralLSPConnection(event) ); } diff --git a/package.json b/package.json index a29f33a7..db4377d9 100644 --- a/package.json +++ b/package.json @@ -326,19 +326,32 @@ "scope": "window", "markdownDescription": "Path to an Oxc formatter binary. Default: auto detection in `node_modules`." }, - "oxc.vitePlus.enable": { - "type": [ - "boolean", - "null" + "oxc.lint.binarySource": { + "type": "string", + "enum": [ + "auto", + "vite-plus", + "oxc" ], "scope": "resource", - "default": null, - "markdownDescription": "Use `vp lint --lsp` and `vp fmt --lsp`. Set to `true` to select Vite+ without detecting a dependency, `false` to disable Vite+ integration, or `null` to detect a direct `vite-plus` dependency. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority." + "default": "auto", + "markdownDescription": "Select the linter source. `auto` detects a direct `vite-plus` dependency or uses `oxc.path.vp`, otherwise it finds standalone Oxlint. `vite-plus` selects `vp lint --lsp` without dependency detection. `oxc` finds standalone Oxlint and ignores `oxc.path.vp`. An explicit `oxc.path.oxlint` takes priority." + }, + "oxc.fmt.binarySource": { + "type": "string", + "enum": [ + "auto", + "vite-plus", + "oxc" + ], + "scope": "resource", + "default": "auto", + "markdownDescription": "Select the formatter source. `auto` detects a direct `vite-plus` dependency or uses `oxc.path.vp`, otherwise it finds standalone Oxfmt. `vite-plus` selects `vp fmt --lsp` without dependency detection. `oxc` finds standalone Oxfmt and ignores `oxc.path.vp`. An explicit `oxc.path.oxfmt` takes priority." }, "oxc.path.vp": { "type": "string", "scope": "resource", - "markdownDescription": "Path to a `vp` executable. Relative paths use the workspace folder. Setting this path selects Vite+ without automatic detection, unless `oxc.vitePlus.enable` is `false`. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority." + "markdownDescription": "Path to a `vp` executable. Relative paths use the workspace folder. Setting this path selects Vite+ without dependency detection for tools whose `binarySource` is `auto` or `vite-plus`. Tools whose `binarySource` is `oxc` ignore this path. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority." }, "oxc.path.tsgolint": { "type": "string", @@ -425,7 +438,8 @@ "oxc.path.tsgolint", "oxc.path.node", "oxc.path.vp", - "oxc.vitePlus.enable", + "oxc.lint.binarySource", + "oxc.fmt.binarySource", "oxc.useExecPath" ] } diff --git a/tests/unit/VSCodeConfig.spec.ts b/tests/unit/VSCodeConfig.spec.ts index 53df6811..d7722396 100644 --- a/tests/unit/VSCodeConfig.spec.ts +++ b/tests/unit/VSCodeConfig.spec.ts @@ -19,7 +19,8 @@ suite("VSCodeConfig", () => { "useExecPath", "suppressProgramErrors", "path.vp", - "vitePlus.enable", + "lint.binarySource", + "fmt.binarySource", ]; setup(async () => { await Promise.all(keys.map((key) => conf.update(key, undefined))); @@ -41,6 +42,8 @@ suite("VSCodeConfig", () => { strictEqual(config.binPathTsGoLint, ""); strictEqual(config.nodePath, ""); strictEqual(config.useExecPath, false); + strictEqual(conf.get("lint.binarySource"), "auto"); + strictEqual(conf.get("fmt.binarySource"), "auto"); strictEqual( config.suppressProgramErrors, false, @@ -112,7 +115,8 @@ suite("VSCodeConfig", () => { { key: "path.node", affects: true }, { key: "useExecPath", affects: true }, { key: "path.vp", affects: true }, - { key: "vitePlus.enable", affects: true }, + { key: "lint.binarySource", affects: true }, + { key: "fmt.binarySource", affects: false }, { key: "requireConfig", affects: false }, { key: "path.oxfmt", affects: false }, ]; @@ -141,7 +145,8 @@ suite("VSCodeConfig", () => { { key: "path.node", affects: true }, { key: "useExecPath", affects: true }, { key: "path.vp", affects: true }, - { key: "vitePlus.enable", affects: true }, + { key: "fmt.binarySource", affects: true }, + { key: "lint.binarySource", affects: false }, { key: "path.tsgolint", affects: false }, { key: "requireConfig", affects: false }, { key: "path.oxlint", affects: false }, diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts index 98e66f43..823418fb 100644 --- a/tests/unit/vitePlus.spec.ts +++ b/tests/unit/vitePlus.spec.ts @@ -42,6 +42,26 @@ suite("Vite+ server selection", () => { ); } + function standaloneTools() { + const binaries = { + oxlint: { path: file("tools/oxlint.js"), loader: "node" as const }, + oxfmt: { path: file("tools/oxfmt.js"), loader: "node" as const }, + }; + mock.method( + findBinary, + "searchProjectNodeModulesBin", + async (name: "oxlint" | "oxfmt") => binaries[name], + ); + return binaries; + } + + async function sources(lint: string, fmt: string) { + await Promise.all([ + conf.update("lint.binarySource", lint, ConfigurationTarget.WorkspaceFolder), + conf.update("fmt.binarySource", fmt, ConfigurationTarget.WorkspaceFolder), + ]); + } + async function open(dir = root) { await window.showTextDocument(Uri.file(file("index.txt", "", dir))); } @@ -64,7 +84,7 @@ suite("Vite+ server selection", () => { const config = workspace.getConfiguration("oxc", folder.uri); // oxlint-disable-next-line no-await-in-loop -- reset each workspace folder await Promise.all( - ["path.vp", "vitePlus.enable"].map((key) => + ["path.vp", "lint.binarySource", "fmt.binarySource"].map((key) => config.update(key, undefined, ConfigurationTarget.WorkspaceFolder), ), ); @@ -86,17 +106,17 @@ suite("Vite+ server selection", () => { deepStrictEqual(fmt, { path: vpPath, loader: "native", cwd: root, vitePlus: "fmt" }); }); - test("explicit enable works without a package.json dependency", async () => { + test("explicit sources work without a package.json dependency", async () => { const vpPath = shim(); - await conf.update("vitePlus.enable", true, ConfigurationTarget.WorkspaceFolder); + await sources("vite-plus", "vite-plus"); strictEqual((await service.getOxlintServerBinPath())?.path, vpPath); strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, "fmt"); }); - test("explicit enable keeps the same executable and cwd across source directories", async () => { + test("explicit sources keep the same executable and cwd across source directories", async () => { file("package.json", "{}"); const vpPath = shim(); - await conf.update("vitePlus.enable", true, ConfigurationTarget.WorkspaceFolder); + await sources("vite-plus", "vite-plus"); await open(path.join(root, "src/pages")); const first = await service.getOxlintServerBinPath(); await open(path.join(root, "src/components")); @@ -144,6 +164,7 @@ suite("Vite+ server selection", () => { test("explicit tool paths take priority over Vite+ for each tool", async () => { declare(); shim(); + await sources("vite-plus", "vite-plus"); const lintPath = file("custom/oxlint.js"); const fmtPath = file("custom/oxfmt.js"); await workspace.getConfiguration("oxc").update("path.oxlint", lintPath); @@ -155,15 +176,84 @@ suite("Vite+ server selection", () => { strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, undefined); }); - test("false disables automatic detection and an explicit vp path", async () => { + test("standalone sources ignore detection and an invalid explicit vp path", async () => { declare(); - const vpPath = shim(); - await conf.update("path.vp", vpPath, ConfigurationTarget.WorkspaceFolder); - await conf.update("vitePlus.enable", false, ConfigurationTarget.WorkspaceFolder); - strictEqual((await service.getOxlintServerBinPath())?.vitePlus, undefined); - strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, undefined); + shim(); + const binaries = standaloneTools(); + await conf.update("path.vp", "./missing-vp", ConfigurationTarget.WorkspaceFolder); + await sources("oxc", "oxc"); + deepStrictEqual(await service.getOxlintServerBinPath(), binaries.oxlint); + deepStrictEqual(await service.getOxfmtServerBinPath(), binaries.oxfmt); }); + for (const lintSource of ["auto", "vite-plus", "oxc"]) { + for (const fmtSource of ["auto", "vite-plus", "oxc"]) { + test(`selects tools independently: lint=${lintSource}, fmt=${fmtSource}`, async () => { + declare(); + const vpPath = shim(); + const binaries = standaloneTools(); + await sources(lintSource, fmtSource); + const [lint, fmt] = await Promise.all([ + service.getOxlintServerBinPath(), + service.getOxfmtServerBinPath(), + ]); + deepStrictEqual( + lint, + lintSource === "oxc" + ? binaries.oxlint + : { path: vpPath, loader: "native", cwd: root, vitePlus: "lint" }, + ); + deepStrictEqual( + fmt, + fmtSource === "oxc" + ? binaries.oxfmt + : { path: vpPath, loader: "native", cwd: root, vitePlus: "fmt" }, + ); + }); + } + } + + for (const command of ["lint", "fmt"] as const) { + test(`forcing ${command} does not opt the other tool in without a dependency`, async () => { + const vpPath = shim(); + const binaries = standaloneTools(); + await conf.update( + `${command}.binarySource`, + "vite-plus", + ConfigurationTarget.WorkspaceFolder, + ); + const [lint, fmt] = await Promise.all([ + service.getOxlintServerBinPath(), + service.getOxfmtServerBinPath(), + ]); + if (command === "lint") { + strictEqual(lint?.path, vpPath); + deepStrictEqual(fmt, binaries.oxfmt); + } else { + deepStrictEqual(lint, binaries.oxlint); + strictEqual(fmt?.path, vpPath); + } + }); + + test(`an explicit vp path respects the standalone ${command} source`, async () => { + const vpPath = file("bin/custom-vp.js"); + const binaries = standaloneTools(); + await conf.update("path.vp", vpPath, ConfigurationTarget.WorkspaceFolder); + await conf.update(`${command}.binarySource`, "oxc", ConfigurationTarget.WorkspaceFolder); + const [lint, fmt] = await Promise.all([ + service.getOxlintServerBinPath(), + service.getOxfmtServerBinPath(), + ]); + if (command === "lint") { + deepStrictEqual(lint, binaries.oxlint); + strictEqual(fmt?.path, vpPath); + } else { + strictEqual(lint?.path, vpPath); + deepStrictEqual(fmt, binaries.oxfmt); + } + }); + } + test("root-declared-no-local-global-on-path", async () => { declare(); const binDir = path.join(root, "global-bin"); @@ -189,12 +279,13 @@ suite("Vite+ server selection", () => { process.env.PATH = binDir; strictEqual((await service.getOxlintServerBinPath())?.vitePlus, undefined); strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, undefined); - await conf.update("vitePlus.enable", true, ConfigurationTarget.WorkspaceFolder); + await conf.update("lint.binarySource", "vite-plus", ConfigurationTarget.WorkspaceFolder); strictEqual( (await service.getOxlintServerBinPath())?.vitePlus, "lint", "explicit opt-in permits global resolution without a dependency", ); + strictEqual((await service.getOxfmtServerBinPath())?.vitePlus, undefined); }); test("discovers global vp using the same shell PATH as the launcher", async () => { @@ -228,6 +319,16 @@ suite("Vite+ server selection", () => { ); }); + test("explicit Vite+ sources report a missing install without falling back to standalone tools", async () => { + standaloneTools(); + process.env.PATH = root; + mock.method(require("node:child_process"), "spawnSync", () => ({ status: 1 })); + mock.method(require("node:os"), "homedir", () => root); + await sources("vite-plus", "vite-plus"); + await rejects(service.getOxlintServerBinPath(), /Vite\+ selected.*pnpm install/); + await rejects(service.getOxfmtServerBinPath(), /Vite\+ selected.*pnpm install/); + }); + test("resolves vp from a global vite-plus package, not a package named vp", async () => { declare(); process.env.PATH = root; @@ -280,11 +381,33 @@ suite("Vite+ server selection", () => { strictEqual((await service.getOxfmtServerBinPath())?.cwd, WORKSPACE_SECOND_FOLDER!.uri.fsPath); }); - for (const change of ["none", "folder", "setting"] as const) { + test("source settings follow the active workspace folder independently for each tool", async function () { + if (!secondRoot || !WORKSPACE_SECOND_FOLDER) this.skip(); + declare(); + declare(secondRoot!); + const firstPath = shim(); + const secondPath = shim(secondRoot!); + const binaries = standaloneTools(); + await sources("oxc", "vite-plus"); + const secondConf = workspace.getConfiguration("oxc", WORKSPACE_SECOND_FOLDER!.uri); + await secondConf.update("lint.binarySource", "vite-plus", ConfigurationTarget.WorkspaceFolder); + await secondConf.update("fmt.binarySource", "oxc", ConfigurationTarget.WorkspaceFolder); + deepStrictEqual(await service.getOxlintServerBinPath(), binaries.oxlint); + strictEqual((await service.getOxfmtServerBinPath())?.path, firstPath); + await open(secondRoot!); + strictEqual((await service.getOxlintServerBinPath())?.path, secondPath); + deepStrictEqual(await service.getOxfmtServerBinPath(), binaries.oxfmt); + await open(); + deepStrictEqual(await service.getOxlintServerBinPath(), binaries.oxlint); + strictEqual((await service.getOxfmtServerBinPath())?.path, firstPath); + }); + + for (const change of ["none", "folder", "setting", "source"] as const) { test(`shares an ongoing search only when its context is unchanged (${change})`, async function () { if (change === "folder" && (!secondRoot || !WORKSPACE_SECOND_FOLDER)) this.skip(); const firstPath = file("bin/first-vp.js"); const secondPath = file("bin/second-vp.js", "", change === "folder" ? secondRoot! : root); + const standalone = change === "source" ? standaloneTools() : undefined; await conf.update("path.vp", firstPath, ConfigurationTarget.WorkspaceFolder); if (change === "folder") { await workspace @@ -315,12 +438,18 @@ suite("Vite+ server selection", () => { if (change === "setting") { await conf.update("path.vp", secondPath, ConfigurationTarget.WorkspaceFolder); } + if (change === "source") { + await conf.update("fmt.binarySource", "oxc", ConfigurationTarget.WorkspaceFolder); + } second = service.getOxfmtServerBinPath(); } finally { release(); } strictEqual((await first)?.path, firstPath); - strictEqual((await second!)?.path, change === "none" ? firstPath : secondPath); + strictEqual( + (await second!)?.path, + standalone?.oxfmt.path ?? (change === "none" ? firstPath : secondPath), + ); strictEqual(firstPathLookups, 1, "matching lint/fmt requests should share discovery"); }); } From 9a4e49431d28efd0fc178dde58490c67dcfeaf2d Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 09:53:15 +0800 Subject: [PATCH 05/15] fix: cache global package locations across file switches --- client/ConfigService.ts | 8 ++- client/findBinary.ts | 15 +++++- client/tools/formatter.ts | 1 + client/tools/linter.ts | 1 + tests/unit/findBinary.spec.ts | 79 +++++++++++++++++++++++++++- tests/unit/vitePlus.spec.ts | 42 +++++++++++++++ tests/unit/vitePlusLifecycle.spec.ts | 42 ++++++++++++++- 7 files changed, 184 insertions(+), 4 deletions(-) diff --git a/client/ConfigService.ts b/client/ConfigService.ts index 1280042f..c65fba13 100644 --- a/client/ConfigService.ts +++ b/client/ConfigService.ts @@ -5,6 +5,7 @@ import { getShellEnv } from "./getShellEnv"; import { DiagnosticPullMode } from "vscode-languageclient"; import { BinarySearchResult, + clearGlobalNodeModulesPathsCache, searchGlobalNodeModulesBin, searchEnvPath, searchProjectNodeModulesBin, @@ -110,6 +111,11 @@ export class ConfigService implements IDisposable { return this.searchBinaryPath(this.vsCodeConfig.binPathOxfmt, "oxfmt"); } + public clearBinarySearchCaches(): void { + clearGlobalNodeModulesPathsCache(); + this.vitePlusSearches.clear(); + } + public shouldRequestDiagnostics( textDocumentUri: Uri, diagnosticPullMode: DiagnosticPullMode, @@ -176,7 +182,7 @@ export class ConfigService implements IDisposable { try { return await search; } finally { - this.vitePlusSearches.delete(key); + if (this.vitePlusSearches.get(key) === search) this.vitePlusSearches.delete(key); } } diff --git a/client/findBinary.ts b/client/findBinary.ts index 9c9b4369..151d23b7 100644 --- a/client/findBinary.ts +++ b/client/findBinary.ts @@ -365,8 +365,21 @@ export async function searchSettingsBin( return undefined; } +let cachedGlobalNodeModulesPaths: Promise | undefined; + +/** Refresh package-manager locations on an explicit server restart. */ +export function clearGlobalNodeModulesPathsCache(): void { + cachedGlobalNodeModulesPaths = undefined; +} + +function globalNodeModulesPaths(): Promise { + // Lint and format share the pending lookup and its result across navigation. + // Only locations are cached; binary searches still check the filesystem. + return (cachedGlobalNodeModulesPaths ??= resolveGlobalNodeModulesPaths()); +} + // copied from: https://github.com/biomejs/biome-vscode/blob/ae9b6df2254d0ff8ee9d626554251600eb2ca118/src/locator.ts#L28-L49 -async function globalNodeModulesPaths(): Promise { +async function resolveGlobalNodeModulesPaths(): Promise { const npmGlobalNodeModulesPath = await safeSpawnSync("npm", ["root", "-g"]); const pnpmGlobalNodeModulesPath = await safeSpawnSync("pnpm", ["root", "-g"]); const bunGlobalNodeModulesPath = path.resolve(homedir(), ".bun/install/global/node_modules"); diff --git a/client/tools/formatter.ts b/client/tools/formatter.ts index 6bac68df..9196139b 100644 --- a/client/tools/formatter.ts +++ b/client/tools/formatter.ts @@ -441,6 +441,7 @@ export default class FormatterTool implements ToolInterface { restart(onlyIfBinaryChanged = false): Promise { const restart = this.restartQueue.then(async () => { + if (!onlyIfBinaryChanged) this.configService.clearBinarySearchCaches(); const previousError = this.binaryError; const newBinary = await this.getBinary(); if ( diff --git a/client/tools/linter.ts b/client/tools/linter.ts index b777ed07..694e53cc 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -432,6 +432,7 @@ export default class LinterTool implements ToolInterface { restart(onlyIfBinaryChanged = false): Promise { const restart = this.restartQueue.then(async () => { + if (!onlyIfBinaryChanged) this.configService.clearBinarySearchCaches(); const previousError = this.binaryError; const newBinary = await this.getBinary(); if ( diff --git a/tests/unit/findBinary.spec.ts b/tests/unit/findBinary.spec.ts index cf0474d5..45b3e2df 100644 --- a/tests/unit/findBinary.spec.ts +++ b/tests/unit/findBinary.spec.ts @@ -1,10 +1,12 @@ -import { strictEqual, throws } from "assert"; +import { deepStrictEqual, strictEqual, throws } from "assert"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import * as path from "node:path"; import { tmpdir } from "node:os"; +import { mock } from "node:test"; import { Uri, workspace } from "vscode"; import { clearWorkspacePackageJsonNodeModulesCache, + clearGlobalNodeModulesPathsCache, replaceTargetFromMainToBin, searchGlobalNodeModulesBin, searchEnvPath, @@ -14,6 +16,10 @@ import { } from "../../client/findBinary"; import { WORKSPACE_FOLDER } from "../test-helpers.js"; +const shellEnv: typeof import("../../client/getShellEnv") = require( + path.join(__dirname, "../client/getShellEnv.js"), +); + suite("findBinary", () => { const binaryName = "oxlint"; @@ -215,6 +221,77 @@ suite("findBinary", () => { }); suite("searchGlobalNodeModulesBin", () => { + let globalModules: string; + + setup(() => { + clearGlobalNodeModulesPathsCache(); + globalModules = mkdtempSync(path.join(tmpdir(), "test-global-modules-")); + }); + + teardown(() => { + mock.restoreAll(); + clearGlobalNodeModulesPathsCache(); + rmSync(globalModules, { recursive: true, force: true }); + }); + + test("shares package-manager probes across concurrent searches and repeated navigation", async () => { + const names = ["oxlint", "oxfmt", "vp"]; + const paths = names.map((name) => + path.join( + globalModules, + ".bin", + name === "vp" && process.platform === "win32" ? "vp.cmd" : name, + ), + ); + mkdirSync(path.join(globalModules, ".bin")); + for (const binPath of paths) writeFileSync(binPath, ""); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + mock.method(shellEnv, "getShellEnv", async () => { + await gate; + return {}; + }); + const probes = mock.method(require("node:child_process"), "spawnSync", () => ({ + status: 0, + stdout: globalModules, + })); + const pending = Promise.all(names.map((name) => searchGlobalNodeModulesBin(name))); + release(); + deepStrictEqual( + (await pending).map((binary) => binary?.path), + paths, + ); + strictEqual(probes.mock.callCount(), 2, "npm and pnpm should each run once"); + for (let navigation = 0; navigation < 3; navigation++) { + // oxlint-disable-next-line no-await-in-loop -- simulate successive file switches + const binaries = await Promise.all(names.map((name) => searchGlobalNodeModulesBin(name))); + deepStrictEqual( + binaries.map((binary) => binary?.path), + paths, + ); + } + strictEqual(probes.mock.callCount(), 2, "file switches must not repeat the probes"); + }); + + test("finds newly installed binaries without repeating package-manager probes", async () => { + mock.method(shellEnv, "getShellEnv", async () => ({})); + const probes = mock.method(require("node:child_process"), "spawnSync", () => ({ + status: 0, + stdout: globalModules, + })); + const name = "new-global-bin-test"; + strictEqual(await searchGlobalNodeModulesBin(name), undefined); + const binPath = path.join(globalModules, ".bin", name); + mkdirSync(path.dirname(binPath)); + writeFileSync(binPath, ""); + strictEqual((await searchGlobalNodeModulesBin(name))?.path, binPath); + rmSync(binPath); + strictEqual(await searchGlobalNodeModulesBin(name), undefined); + strictEqual(probes.mock.callCount(), 2); + }); + test("should return undefined when binary is not found in global node_modules", async () => { const result = await searchGlobalNodeModulesBin("non-existent-binary-package-name-12345"); strictEqual(result, undefined); diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts index 823418fb..0ccd23ef 100644 --- a/tests/unit/vitePlus.spec.ts +++ b/tests/unit/vitePlus.spec.ts @@ -67,6 +67,7 @@ suite("Vite+ server selection", () => { } setup(async () => { + findBinary.clearGlobalNodeModulesPathsCache(); mock.method(shellEnv, "getShellEnv", async () => ({ PATH: process.env.PATH })); file("pnpm-workspace.yaml"); await open(); @@ -75,6 +76,7 @@ suite("Vite+ server selection", () => { teardown(async () => { service.dispose(); + findBinary.clearGlobalNodeModulesPathsCache(); mock.restoreAll(); if (originalPath === undefined) delete process.env.PATH; else process.env.PATH = originalPath; @@ -402,6 +404,46 @@ suite("Vite+ server selection", () => { strictEqual((await service.getOxfmtServerBinPath())?.path, firstPath); }); + test("a refresh replaces pending discovery without the old search clearing the new one", async () => { + const vpPath = file("bin/vp.js"); + await conf.update("path.vp", vpPath, ConfigurationTarget.WorkspaceFolder); + let releaseFirst!: () => void; + let releaseSecond!: () => void; + const firstGate = new Promise((resolve) => { + releaseFirst = resolve; + }); + const secondGate = new Promise((resolve) => { + releaseSecond = resolve; + }); + const searchSettingsBin = findBinary.searchSettingsBin; + let lookups = 0; + mock.method( + findBinary, + "searchSettingsBin", + async (name: string, configuredPath: string, cwd?: string) => { + await (++lookups === 1 ? firstGate : secondGate); + return searchSettingsBin(name, configuredPath, cwd); + }, + ); + const first = service.getOxlintServerBinPath(); + service.clearBinarySearchCaches(); + const second = service.getOxlintServerBinPath(); + try { + strictEqual(lookups, 2, "refresh must start a new search"); + releaseFirst(); + await first; + const third = service.getOxfmtServerBinPath(); + strictEqual(lookups, 2, "lint and format must still share the refreshed search"); + releaseSecond(); + strictEqual((await second)?.path, vpPath); + strictEqual((await third)?.path, vpPath); + } finally { + releaseFirst(); + releaseSecond(); + await Promise.all([first, second]); + } + }); + for (const change of ["none", "folder", "setting", "source"] as const) { test(`shares an ongoing search only when its context is unchanged (${change})`, async function () { if (change === "folder" && (!secondRoot || !WORKSPACE_SECOND_FOLDER)) this.skip(); diff --git a/tests/unit/vitePlusLifecycle.spec.ts b/tests/unit/vitePlusLifecycle.spec.ts index 3bf3c316..c6835035 100644 --- a/tests/unit/vitePlusLifecycle.spec.ts +++ b/tests/unit/vitePlusLifecycle.spec.ts @@ -4,7 +4,11 @@ import * as path from "node:path"; import { mock } from "node:test"; import { window, workspace } from "vscode"; import { ConfigService } from "../../client/ConfigService"; -import type { BinarySearchResult } from "../../client/findBinary"; +import { + BinarySearchResult, + clearGlobalNodeModulesPathsCache, + searchGlobalNodeModulesBin, +} from "../../client/findBinary"; import StatusBarItemHandler from "../../client/StatusBarItemHandler"; import Formatter from "../../client/tools/formatter"; import Linter from "../../client/tools/linter"; @@ -45,6 +49,7 @@ for (const [Tool, command, getter] of [ output.dispose(); status.dispose(); mock.restoreAll(); + clearGlobalNodeModulesPathsCache(); await workspace.getConfiguration("oxc").update("enable", undefined); rmSync(root, { recursive: true, force: true }); }); @@ -70,6 +75,41 @@ for (const [Tool, command, getter] of [ ); }); + test("navigation reuses global locations and an explicit restart refreshes them", async () => { + const name = command === "lint" ? "oxlint" : "oxfmt"; + const firstModules = path.join(root, "first", "node_modules"); + const secondModules = path.join(root, "second", "node_modules"); + for (const dir of [firstModules, secondModules]) { + mkdirSync(path.join(dir, ".bin"), { recursive: true }); + writeFileSync(path.join(dir, ".bin", name), ""); + } + let globalModules = firstModules; + const probes = mock.method(require("node:child_process"), "spawnSync", () => ({ + status: 0, + stdout: globalModules, + })); + mock.method(service, getter, () => searchGlobalNodeModulesBin(name)); + const activation = mock.method(tool, "activate", tool.activate.bind(tool)); + await tool.restart(); + strictEqual( + activation.mock.calls[0].arguments[0]?.path, + path.join(firstModules, ".bin", name), + ); + strictEqual(probes.mock.callCount(), 2); + + globalModules = secondModules; + await tool.restart(true); + strictEqual(activation.mock.callCount(), 1); + strictEqual(probes.mock.callCount(), 2, "navigation must reuse the known locations"); + + await tool.restart(); + strictEqual( + activation.mock.calls[1].arguments[0]?.path, + path.join(secondModules, ".bin", name), + ); + strictEqual(probes.mock.callCount(), 4, "explicit restarts must query the locations again"); + }); + test("waits for an ongoing restart before processing another restart or shutdown", async () => { let release!: () => void; const gate = new Promise((resolve) => { From 9a90030131962a6cd0b7a96f20b9060d7d9fa0af Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 11:44:02 +0800 Subject: [PATCH 06/15] fix: disable nested configuration for Vite+ servers --- README.md | 6 +- client/ConfigService.ts | 8 +-- client/WorkspaceConfig.ts | 12 ++-- client/tools/formatter.ts | 12 ++-- client/tools/linter.ts | 12 ++-- package.json | 4 +- tests/unit/vitePlusLifecycle.spec.ts | 88 +++++++++++++++++++++++++++- 7 files changed, 117 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index f3854baa..00ab6f21 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,8 @@ With `oxc.useExecPath`, the extension runs npm and pnpm project entries with VS The source settings and `oxc.path.vp` can differ between workspace folders. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority over source selection for their respective tools. Changing a source setting restarts only its tool. +Vite+ does not support nested configuration. The extension automatically disables nested config discovery for each server started through `vp`. This applies to both linting and formatting without changing your saved settings. Standalone tools continue to use `oxc.disableNestedConfig` and `oxc.fmt.disableNestedConfig`. + The `"vite-plus"` source uses the nearest `package.json` or monorepo root as its working directory, with the workspace folder as a fallback. Switching between source directories in the same package does not restart the servers. The extension rechecks Vite+ when you switch files and restarts a server if its executable or project directory changes. It uses one server per tool for the window. If Vite+ is selected but `vp` is unavailable, the status item and output channels show an install hint. Install your dependencies and run the **Oxc: Restart oxlint Server** and **Oxc: Restart oxfmt Server** commands. A failed Vite+ launch shows an install or upgrade hint. Vite+ integration requires a trusted workspace, and `oxc.requireConfig` does not require a separate Oxlint configuration when Vite+ is selected. @@ -139,11 +141,11 @@ Following configurations are supported via `settings.json` and can be changed fo | Key | Default Value | Possible Values | Description | | ----------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `oxc.configPath` | `null` | `` \| `` | Path to oxlint configuration. Keep it empty to enable nested configuration. | -| `oxc.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used. | +| `oxc.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used. The extension always sends `true` when using `vp lint --lsp`. | | `oxc.fixKind` | `null` | `safe_fix` \| `safe_fix_or_suggestion` \| `dangerous_fix` \| `dangerous_fix_or_suggestion` \| `none` \| `all` | Specify the kind of fixes to suggest/apply. | | `oxc.fmt.binarySource` | `auto` | `auto` \| `vite-plus` \| `oxc` | Select the formatter source. `auto` detects a direct `vite-plus` dependency or uses `oxc.path.vp`, otherwise it finds standalone Oxfmt. `vite-plus` selects `vp fmt --lsp` without dependency detection. `oxc` finds standalone Oxfmt and ignores `oxc.path.vp`. An explicit `oxc.path.oxfmt` takes priority. | | `oxc.fmt.configPath` | `null` | `` \| `` | Path to an oxfmt configuration file | -| `oxc.fmt.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used. | +| `oxc.fmt.disableNestedConfig` | `false` | `true` \| `false` | Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used. The extension always sends `true` when using `vp fmt --lsp`. | | `oxc.lint.binarySource` | `auto` | `auto` \| `vite-plus` \| `oxc` | Select the linter source. `auto` detects a direct `vite-plus` dependency or uses `oxc.path.vp`, otherwise it finds standalone Oxlint. `vite-plus` selects `vp lint --lsp` without dependency detection. `oxc` finds standalone Oxlint and ignores `oxc.path.vp`. An explicit `oxc.path.oxlint` takes priority. | | `oxc.lint.customization` | `null` | `Record` \| `` | Customizes linting rules behavior. See for details. | | `oxc.lint.run` | `onType` | `onSave` \| `onType` | Run the linter on save (onSave) or on type (onType) | diff --git a/client/ConfigService.ts b/client/ConfigService.ts index c65fba13..684fc7e9 100644 --- a/client/ConfigService.ts +++ b/client/ConfigService.ts @@ -58,12 +58,12 @@ export class ConfigService implements IDisposable { this._disposables.push(disposeChangeListener); } - public get oxlintServerConfig(): { + public getOxlintServerConfig(isVitePlus = false): { workspaceUri: string; options: OxlintWorkspaceConfigInterface; }[] { return [...this.workspaceConfigs.entries()].map(([path, config]) => { - const options = config.toOxlintConfig(); + const options = config.toOxlintConfig(isVitePlus); return { workspaceUri: Uri.file(path).toString(), @@ -72,13 +72,13 @@ export class ConfigService implements IDisposable { }); } - public get formatterServerConfig(): { + public getFormatterServerConfig(isVitePlus = false): { workspaceUri: string; options: OxfmtWorkspaceConfigInterface; }[] { return [...this.workspaceConfigs.entries()].map(([path, config]) => ({ workspaceUri: Uri.file(path).toString(), - options: config.toOxfmtConfig(), + options: config.toOxfmtConfig(isVitePlus), })); } diff --git a/client/WorkspaceConfig.ts b/client/WorkspaceConfig.ts index ea84dad3..ca8c69a0 100644 --- a/client/WorkspaceConfig.ts +++ b/client/WorkspaceConfig.ts @@ -375,31 +375,33 @@ export class WorkspaceConfig { return diagnosticPullMode === this.runTrigger; } - public toOxlintConfig(): OxlintWorkspaceConfigInterface { + public toOxlintConfig(isVitePlus = false): OxlintWorkspaceConfigInterface { + // Vite+ uses one configuration, so nested Oxc configs must not override it. + const disableNestedConfig = isVitePlus || this.disableNestedConfig; return { configPath: this.configPath ?? undefined, tsConfigPath: this.tsConfigPath ?? undefined, unusedDisableDirectives: this.unusedDisableDirectives ?? undefined, typeAware: this.typeAware ?? undefined, - disableNestedConfig: this.disableNestedConfig, + disableNestedConfig, fixKind: this.fixKind ?? undefined, rulesCustomization: this.rulesCustomization ?? undefined, // keep for backward compatibility run: this.runTrigger, // deprecated, kept for backward compatibility flags: { - disable_nested_config: this.disableNestedConfig ? "true" : "false", + disable_nested_config: disableNestedConfig ? "true" : "false", ...(this.fixKind ? { fix_kind: this.fixKind } : {}), }, }; } - public toOxfmtConfig(): OxfmtWorkspaceConfigInterface { + public toOxfmtConfig(isVitePlus = false): OxfmtWorkspaceConfigInterface { return { // @ts-expect-error -- deprecated setting, kept for backward compatibility ["fmt.experimental"]: true, ["fmt.configPath"]: this.formattingConfigPath ?? undefined, - ["fmt.disableNestedConfig"]: this.formattingDisableNestedConfig, + ["fmt.disableNestedConfig"]: isVitePlus || this.formattingDisableNestedConfig, }; } } diff --git a/client/tools/formatter.ts b/client/tools/formatter.ts index 9196139b..473b465d 100644 --- a/client/tools/formatter.ts +++ b/client/tools/formatter.ts @@ -374,7 +374,7 @@ export default class FormatterTool implements ToolInterface { const clientOptions: LanguageClientOptions = { // Register the server for plain text documents documentSelector: this.documentSelectors, - initializationOptions: this.configService.formatterServerConfig, + initializationOptions: this.configService.getFormatterServerConfig(!!binary.vitePlus), outputChannel: this.outputChannel, traceOutputChannel: this.outputChannel, middleware: { @@ -389,8 +389,9 @@ export default class FormatterTool implements ToolInterface { } return ( - this.configService.getWorkspaceConfig(Uri.parse(item.scopeUri))?.toOxfmtConfig() ?? - null + this.configService + .getWorkspaceConfig(Uri.parse(item.scopeUri)) + ?.toOxfmtConfig(!!this.binary?.vitePlus) ?? null ); }); }, @@ -504,11 +505,12 @@ export default class FormatterTool implements ToolInterface { } // update the initializationOptions for a possible restart - this.client.clientOptions.initializationOptions = this.configService.formatterServerConfig; + const settings = this.configService.getFormatterServerConfig(!!this.binary?.vitePlus); + this.client.clientOptions.initializationOptions = settings; if (this.configService.effectsWorkspaceConfigChange(event) && this.client.isRunning()) { await this.client.sendNotification("workspace/didChangeConfiguration", { - settings: this.configService.formatterServerConfig, + settings, }); } } diff --git a/client/tools/linter.ts b/client/tools/linter.ts index 694e53cc..5a5d584c 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -288,7 +288,7 @@ export default class LinterTool implements ToolInterface { scheme: "file", }, ], - initializationOptions: this.configService.oxlintServerConfig, + initializationOptions: this.configService.getOxlintServerConfig(!!binary.vitePlus), outputChannel: this.outputChannel, traceOutputChannel: this.outputChannel, diagnosticPullOptions: { @@ -345,8 +345,9 @@ export default class LinterTool implements ToolInterface { } return ( - this.configService.getWorkspaceConfig(Uri.parse(item.scopeUri))?.toOxlintConfig() ?? - null + this.configService + .getWorkspaceConfig(Uri.parse(item.scopeUri)) + ?.toOxlintConfig(!!this.binary?.vitePlus) ?? null ); }); }, @@ -479,11 +480,12 @@ export default class LinterTool implements ToolInterface { } // update the initializationOptions for a possible restart - this.client.clientOptions.initializationOptions = this.configService.oxlintServerConfig; + const settings = this.configService.getOxlintServerConfig(!!this.binary?.vitePlus); + this.client.clientOptions.initializationOptions = settings; if (this.configService.effectsWorkspaceConfigChange(event) && this.client.isRunning()) { await this.client.sendNotification("workspace/didChangeConfiguration", { - settings: this.configService.oxlintServerConfig, + settings, }); } } diff --git a/package.json b/package.json index db4377d9..e00b7a59 100644 --- a/package.json +++ b/package.json @@ -216,7 +216,7 @@ "type": "boolean", "scope": "resource", "default": false, - "markdownDescription": "Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used." + "markdownDescription": "Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.configPath` (if any) will be used. The extension always sends `true` when using `vp lint --lsp`." }, "oxc.fixKind": { "type": [ @@ -301,7 +301,7 @@ "type": "boolean", "scope": "resource", "default": false, - "markdownDescription": "Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used." + "markdownDescription": "Disable searching for nested configuration files. When set to true, only the configuration file specified in `oxc.fmt.configPath` (if any) will be used. The extension always sends `true` when using `vp fmt --lsp`." }, "oxc.suppressProgramErrors": { "type": "boolean", diff --git a/tests/unit/vitePlusLifecycle.spec.ts b/tests/unit/vitePlusLifecycle.spec.ts index c6835035..4ed186f2 100644 --- a/tests/unit/vitePlusLifecycle.spec.ts +++ b/tests/unit/vitePlusLifecycle.spec.ts @@ -1,8 +1,9 @@ -import { strictEqual } from "assert"; +import { deepStrictEqual, ok, strictEqual } from "assert"; import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import * as path from "node:path"; import { mock } from "node:test"; -import { window, workspace } from "vscode"; +import { CancellationTokenSource, ConfigurationTarget, window, workspace } from "vscode"; +import type { LanguageClient } from "vscode-languageclient/node"; import { ConfigService } from "../../client/ConfigService"; import { BinarySearchResult, @@ -51,9 +52,92 @@ for (const [Tool, command, getter] of [ mock.restoreAll(); clearGlobalNodeModulesPathsCache(); await workspace.getConfiguration("oxc").update("enable", undefined); + await workspace + .getConfiguration("oxc", WORKSPACE_FOLDER.uri) + .update( + command === "lint" ? "disableNestedConfig" : "fmt.disableNestedConfig", + undefined, + ConfigurationTarget.WorkspaceFolder, + ); rmSync(root, { recursive: true, force: true }); }); + for (const userSetting of [false, true]) { + test(`disables nested configs only for Vite+ with user setting ${userSetting}`, async () => { + const key = command === "lint" ? "disableNestedConfig" : "fmt.disableNestedConfig"; + const config = workspace.getConfiguration("oxc", WORKSPACE_FOLDER.uri); + await config.update(key, userSetting, ConfigurationTarget.WorkspaceFolder); + service.getWorkspaceConfig(WORKSPACE_FOLDER.uri)!.refresh(); + const event = { affectsConfiguration: (section: string) => section === `oxc.${key}` }; + + const check = async (expected: boolean) => { + const client = (tool as unknown as { client: LanguageClient }).client; + const optionsForWorkspace = (settings: { workspaceUri: string; options: unknown }[]) => + settings.find(({ workspaceUri }) => workspaceUri === WORKSPACE_FOLDER.uri.toString())! + .options; + const assertOptions = (options: unknown) => { + const values = options as Record; + strictEqual(values[key], expected); + if (command === "lint") { + strictEqual( + (values.flags as Record).disable_nested_config, + String(expected), + ); + } + }; + + assertOptions(optionsForWorkspace(client.clientOptions.initializationOptions)); + const cancellation = new CancellationTokenSource(); + try { + const pulled = await client.clientOptions.middleware!.workspace!.configuration!( + { + items: [ + { section: "oxc_language_server", scopeUri: WORKSPACE_FOLDER.uri.toString() }, + { section: "unrelated", scopeUri: WORKSPACE_FOLDER.uri.toString() }, + { section: "oxc_language_server" }, + ], + }, + cancellation.token, + async () => [], + ); + ok(Array.isArray(pulled)); + assertOptions(pulled[0]); + deepStrictEqual(pulled.slice(1), [null, null]); + } finally { + cancellation.dispose(); + } + + const running = mock.method(client, "isRunning", () => true); + const notification = mock.method(client, "sendNotification", async () => {}); + try { + await tool.onConfigChange(event); + strictEqual(notification.mock.callCount(), 1); + const [method, params] = notification.mock.calls[0].arguments; + strictEqual(method, "workspace/didChangeConfiguration"); + assertOptions(optionsForWorkspace(params.settings)); + assertOptions(optionsForWorkspace(client.clientOptions.initializationOptions)); + } finally { + running.mock.restore(); + notification.mock.restore(); + } + strictEqual( + workspace.getConfiguration("oxc", WORKSPACE_FOLDER.uri).get(key), + userSetting, + "the saved setting must not change", + ); + }; + + await check(true); + const vitePlus = selected; + selected = { path: selected.path, loader: "node" }; + await tool.restart(true); + await check(userSetting); + selected = vitePlus; + await tool.restart(true); + await check(true); + }); + } + test("keeps the client for unchanged binaries and replaces it when the project or mode changes", async () => { const activation = mock.method(tool, "activate", tool.activate.bind(tool)); await tool.restart(true); From 6cd21a427223da5f5b7898739f1e4909eb7abac7 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 12:02:30 +0800 Subject: [PATCH 07/15] fix: normalize Windows PATH for Vite+ discovery --- client/getShellEnv.ts | 10 +++++++- tests/unit/getShellEnv.spec.ts | 22 ++++++++++++++--- tests/unit/vitePlus.spec.ts | 44 ++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/client/getShellEnv.ts b/client/getShellEnv.ts index 39d51537..999f4c60 100644 --- a/client/getShellEnv.ts +++ b/client/getShellEnv.ts @@ -22,7 +22,15 @@ export async function getShellEnv(): Promise> // windows electron app does not have the problem of individual shell environment, as it inherits the environment from the parent process. if (process.platform === "win32") { - cachedEnv = Promise.resolve({ ...process.env }); + // Plain objects lose process.env's case-insensitive lookup on Windows. + // Normalize PATH so discovery and launch use the same value. + const windowsEnv = Object.fromEntries( + Object.entries(process.env).map(([key, value]) => [ + key.toUpperCase() === "PATH" ? "PATH" : key, + value, + ]), + ); + cachedEnv = Promise.resolve(windowsEnv); return cachedEnv; } diff --git a/tests/unit/getShellEnv.spec.ts b/tests/unit/getShellEnv.spec.ts index 4529e536..1cfac5ee 100644 --- a/tests/unit/getShellEnv.spec.ts +++ b/tests/unit/getShellEnv.spec.ts @@ -7,10 +7,11 @@ type GetShellEnvModule = { getShellEnv: () => Promise>; }; +let moduleId = 0; + async function loadFreshGetShellEnvModule(): Promise { - const timestamp = Date.now(); // append a query parameter to force a fresh import of the module to reset the cachedEnv variable - const module = await import(`../../client/getShellEnv.ts?ts=${timestamp}`); + const module = await import(`../../client/getShellEnv.ts?testModule=${moduleId++}`); return module; } @@ -35,7 +36,7 @@ suite("getShellEnv", () => { process.env = originalEnv; }); - test("returns process.env directly on win32", async () => { + test("copies process.env on win32", async () => { Object.defineProperty(process, "platform", { value: "win32" }); process.env.GET_SHELL_ENV_TEST_KEY = "windows-fast-path"; process.env.SHELL = path.join(tempDir, "does-not-matter-on-win32"); @@ -46,6 +47,21 @@ suite("getShellEnv", () => { strictEqual(env.GET_SHELL_ENV_TEST_KEY, "windows-fast-path"); }); + for (const key of ["Path", "PATH", "pAtH"]) { + test(`normalizes ${key} in the Windows environment copy`, async () => { + Object.defineProperty(process, "platform", { value: "win32" }); + process.env = { [key]: tempDir, KEEP_ME: "unchanged" }; + const { getShellEnv } = await loadFreshGetShellEnvModule(); + const env = await getShellEnv(); + + strictEqual(env.PATH, tempDir); + strictEqual(env.KEEP_ME, "unchanged"); + strictEqual(Object.keys(env).filter((name) => name.toUpperCase() === "PATH").length, 1); + strictEqual(process.env[key], tempDir, "the original environment must not change"); + strictEqual(Object.keys(process.env)[0], key); + }); + } + test("parses shell output into env object", async function () { if (process.platform === "win32") { this.skip(); diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts index 0ccd23ef..ac0184f6 100644 --- a/tests/unit/vitePlus.spec.ts +++ b/tests/unit/vitePlus.spec.ts @@ -306,6 +306,50 @@ suite("Vite+ server selection", () => { ); }); + for (const extension of ["cmd", "exe"]) { + test(`discovers global vp.${extension} from a Windows Path environment key`, async () => { + declare(); + const shellBin = path.join(root, "shell-bin"); + const vpPath = file(`vp.${extension}`, "", shellBin); + const originalPlatform = process.platform; + const originalEnv = process.env; + try { + Object.defineProperty(process, "platform", { value: "win32" }); + process.env = { ...originalEnv }; + for (const key of Object.keys(process.env)) { + if (key.toUpperCase() === "PATH") delete process.env[key]; + } + process.env.Path = shellBin; + // Use a fresh instance of the real provider to exercise its environment copy. + const { getShellEnv } = await import( + `../../client/getShellEnv.ts?windowsPath=${extension}` + ); + mock.method(shellEnv, "getShellEnv", getShellEnv); + + const binaries = await Promise.all([ + service.getOxlintServerBinPath(), + service.getOxfmtServerBinPath(), + ]); + await Promise.all( + binaries.map(async (binary) => { + strictEqual(binary?.path, vpPath); + const executable = await runExecutable(binary!, true); + strictEqual( + executable.options?.env?.PATH, + `${path.dirname(process.execPath)}${path.delimiter}${shellBin}`, + ); + strictEqual(executable.options?.env?.Path, undefined); + }), + ); + strictEqual(process.env.Path, shellBin); + strictEqual(process.env.PATH, undefined); + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform }); + process.env = originalEnv; + } + }); + } + test("missing local and global installs give an install hint, even if plain tools exist", async () => { declare(); process.env.PATH = root; From e1debc26d5bfbcb3d16ad3184f5eb60ed76ab4bc Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 15:49:05 +0800 Subject: [PATCH 08/15] test: restore process overrides after each test --- tests/processMocks.ts | 24 +++++++++++ tests/unit/detectVitePlus.spec.ts | 8 ++-- tests/unit/extension.spec.ts | 7 ++-- tests/unit/findBinary.spec.ts | 6 +-- tests/unit/getShellEnv.spec.ts | 14 +++---- tests/unit/lsp_helper.spec.ts | 15 ++++--- tests/unit/vitePlus.spec.ts | 66 +++++++++++++------------------ 7 files changed, 77 insertions(+), 63 deletions(-) create mode 100644 tests/processMocks.ts diff --git a/tests/processMocks.ts b/tests/processMocks.ts new file mode 100644 index 00000000..dab44e7d --- /dev/null +++ b/tests/processMocks.ts @@ -0,0 +1,24 @@ +/** Register these helpers inside a Mocha suite to restore process state after each test. */ +export function mockProcessPlatform(): (platform: NodeJS.Platform) => void { + let descriptor: PropertyDescriptor; + setup(() => { + descriptor = Object.getOwnPropertyDescriptor(process, "platform")!; + }); + teardown(() => { + Object.defineProperty(process, "platform", descriptor); + }); + return (platform) => { + Object.defineProperty(process, "platform", { value: platform }); + }; +} + +export function mockProcessEnv(): void { + let descriptor: PropertyDescriptor; + setup(() => { + descriptor = Object.getOwnPropertyDescriptor(process, "env")!; + process.env = { ...process.env }; + }); + teardown(() => { + Object.defineProperty(process, "env", descriptor); + }); +} diff --git a/tests/unit/detectVitePlus.spec.ts b/tests/unit/detectVitePlus.spec.ts index a3887868..bfd8f38f 100644 --- a/tests/unit/detectVitePlus.spec.ts +++ b/tests/unit/detectVitePlus.spec.ts @@ -3,10 +3,11 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import * as path from "node:path"; import { detectVitePlusProject } from "../../client/detectVitePlus"; +import { mockProcessPlatform } from "../processMocks"; suite("detectVitePlusProject", () => { let root: string; - const originalPlatform = process.platform; + const setPlatform = mockProcessPlatform(); function file(relative: string, content = ""): string { const target = path.join(root, relative); @@ -31,7 +32,6 @@ suite("detectVitePlusProject", () => { }); teardown(() => { - Object.defineProperty(process, "platform", { value: originalPlatform }); rmSync(root, { recursive: true, force: true }); }); @@ -163,7 +163,7 @@ suite("detectVitePlusProject", () => { }); test("selects vp.cmd on Windows", () => { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); pkg(); file("node_modules/.bin/vp"); file("node_modules/.bin/vp.exe"); @@ -172,7 +172,7 @@ suite("detectVitePlusProject", () => { }); test("selects a local vp.exe shim on Windows before a hoisted vp.cmd", () => { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); pkg("packages/app"); shim(); const vpPath = file("packages/app/node_modules/.bin/vp.exe"); diff --git a/tests/unit/extension.spec.ts b/tests/unit/extension.spec.ts index 74828572..954d536f 100644 --- a/tests/unit/extension.spec.ts +++ b/tests/unit/extension.spec.ts @@ -8,14 +8,16 @@ import type { BinarySearchResult } from "../../client/findBinary"; import Formatter from "../../client/tools/formatter"; import Linter from "../../client/tools/linter"; import { WORKSPACE_FOLDER } from "../test-helpers"; +import { mockProcessEnv } from "../processMocks"; suite("navigation during extension activation", () => { const root = path.join(WORKSPACE_FOLDER.uri.fsPath, "startup-navigation"); - const originalEnv = process.env; + mockProcessEnv(); let context: ExtensionContext; setup(() => { - process.env = { ...originalEnv, SKIP_LINTER_TEST: "false", SKIP_FORMATTER_TEST: "false" }; + process.env.SKIP_LINTER_TEST = "false"; + process.env.SKIP_FORMATTER_TEST = "false"; mkdirSync(root, { recursive: true }); for (const name of ["a", "b"]) writeFileSync(path.join(root, `${name}.txt`), ""); // The test host has already registered the extension's commands. @@ -30,7 +32,6 @@ suite("navigation during extension activation", () => { await deactivate(); for (const disposable of context.subscriptions) disposable.dispose(); mock.restoreAll(); - process.env = originalEnv; await commands.executeCommand("workbench.action.closeAllEditors"); rmSync(root, { recursive: true, force: true }); }); diff --git a/tests/unit/findBinary.spec.ts b/tests/unit/findBinary.spec.ts index 45b3e2df..504070ff 100644 --- a/tests/unit/findBinary.spec.ts +++ b/tests/unit/findBinary.spec.ts @@ -15,6 +15,7 @@ import { searchSettingsBin, } from "../../client/findBinary"; import { WORKSPACE_FOLDER } from "../test-helpers.js"; +import { mockProcessPlatform } from "../processMocks"; const shellEnv: typeof import("../../client/getShellEnv") = require( path.join(__dirname, "../client/getShellEnv.js"), @@ -22,20 +23,19 @@ const shellEnv: typeof import("../../client/getShellEnv") = require( suite("findBinary", () => { const binaryName = "oxlint"; + const setPlatform = mockProcessPlatform(); test("prefers a Windows vp.cmd shim over the POSIX shim for a configured path", async () => { - const originalPlatform = process.platform; const dir = mkdtempSync(path.join(tmpdir(), "test-vp-cmd-")); const vpPath = path.join(dir, "vp"); writeFileSync(vpPath, ""); writeFileSync(`${vpPath}.cmd`, ""); try { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); const result = await searchSettingsBin("vp", vpPath); strictEqual(result?.path, `${vpPath}.cmd`); strictEqual(result?.loader, "native"); } finally { - Object.defineProperty(process, "platform", { value: originalPlatform }); rmSync(dir, { recursive: true, force: true }); } }); diff --git a/tests/unit/getShellEnv.spec.ts b/tests/unit/getShellEnv.spec.ts index 1cfac5ee..d3650d0f 100644 --- a/tests/unit/getShellEnv.spec.ts +++ b/tests/unit/getShellEnv.spec.ts @@ -1,7 +1,8 @@ import { strictEqual } from "assert"; -import { mkdtempSync, writeFileSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import * as path from "node:path"; +import { mockProcessEnv, mockProcessPlatform } from "../processMocks"; type GetShellEnvModule = { getShellEnv: () => Promise>; @@ -24,20 +25,19 @@ function createMockShellScript(dir: string, name: string, scriptBody: string): s suite("getShellEnv", () => { let tempDir: string; - const originalPlatform = process.platform; - const originalEnv = process.env; + const setPlatform = mockProcessPlatform(); + mockProcessEnv(); setup(() => { tempDir = mkdtempSync(path.join(tmpdir(), "get-shell-env-test-")); }); teardown(() => { - Object.defineProperty(process, "platform", { value: originalPlatform }); - process.env = originalEnv; + rmSync(tempDir, { recursive: true, force: true }); }); test("copies process.env on win32", async () => { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); process.env.GET_SHELL_ENV_TEST_KEY = "windows-fast-path"; process.env.SHELL = path.join(tempDir, "does-not-matter-on-win32"); @@ -49,7 +49,7 @@ suite("getShellEnv", () => { for (const key of ["Path", "PATH", "pAtH"]) { test(`normalizes ${key} in the Windows environment copy`, async () => { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); process.env = { [key]: tempDir, KEEP_ME: "unchanged" }; const { getShellEnv } = await loadFreshGetShellEnvModule(); const env = await getShellEnv(); diff --git a/tests/unit/lsp_helper.spec.ts b/tests/unit/lsp_helper.spec.ts index c5cbdff5..b20fe117 100644 --- a/tests/unit/lsp_helper.spec.ts +++ b/tests/unit/lsp_helper.spec.ts @@ -6,6 +6,7 @@ import { mock } from "node:test"; import { runExecutable } from "../../client/tools/lsp_helper"; import * as path from "node:path"; import { pathToFileURL } from "node:url"; +import { mockProcessEnv, mockProcessPlatform } from "../processMocks"; // Mock the shared CommonJS exports used by the compiled test modules. const shellEnv: typeof import("../../client/getShellEnv") = require( @@ -14,11 +15,11 @@ const shellEnv: typeof import("../../client/getShellEnv") = require( suite("runExecutable", () => { const originalPlatform = process.platform; - const originalEnv = process.env; + const setPlatform = mockProcessPlatform(); + mockProcessEnv(); let tempDir: string; setup(() => { - process.env = { ...originalEnv }; tempDir = mkdtempSync(path.join(tmpdir(), "vp-runtime-")); mock.method(shellEnv, "getShellEnv", async () => ({ PATH: process.env.PATH })); }); @@ -26,8 +27,6 @@ suite("runExecutable", () => { teardown(() => { mock.restoreAll(); rmSync(tempDir, { recursive: true, force: true }); - Object.defineProperty(process, "platform", { value: originalPlatform }); - process.env = originalEnv; }); for (const command of ["lint", "fmt"] as const) { @@ -129,7 +128,7 @@ process.exit(child.status ?? 1); }); test("quotes Windows vp.cmd paths and passes the subcommand through the shell", async () => { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); const result = await runExecutable({ path: "C:\\My Project\\node_modules\\.bin\\vp.cmd", loader: "native", @@ -190,7 +189,7 @@ process.exit(child.status ?? 1); }); test("should use shell on Windows for binary executables", async () => { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); const result = await runExecutable({ path: "C:\\Path With Spaces\\oxc-language-server", @@ -201,7 +200,7 @@ process.exit(child.status ?? 1); }); test("should prepend nodePath to PATH", async () => { - Object.defineProperty(process, "platform", { value: "linux" }); + setPlatform("linux"); process.env.PATH = "/usr/bin:/bin"; const result = await runExecutable( @@ -218,7 +217,7 @@ process.exit(child.status ?? 1); }); test("should set path in quotes on Windows for binary executables", async () => { - Object.defineProperty(process, "platform", { value: "win32" }); + setPlatform("win32"); const result = await runExecutable({ path: "C:\\Path With Spaces\\oxc-language-server", diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts index ac0184f6..59221175 100644 --- a/tests/unit/vitePlus.spec.ts +++ b/tests/unit/vitePlus.spec.ts @@ -6,6 +6,7 @@ import { commands, ConfigurationTarget, Uri, window, workspace } from "vscode"; import { ConfigService } from "../../client/ConfigService"; import { runExecutable } from "../../client/tools/lsp_helper"; import { WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER } from "../test-helpers"; +import { mockProcessEnv, mockProcessPlatform } from "../processMocks"; // Mock the shared CommonJS exports used by the compiled test modules. const shellEnv: typeof import("../../client/getShellEnv") = require( @@ -20,7 +21,8 @@ suite("Vite+ server selection", () => { const secondRoot = WORKSPACE_SECOND_FOLDER && path.join(WORKSPACE_SECOND_FOLDER.uri.fsPath, "vite-plus-tests"); const conf = workspace.getConfiguration("oxc", WORKSPACE_FOLDER.uri); - const originalPath = process.env.PATH; + const setPlatform = mockProcessPlatform(); + mockProcessEnv(); let service: ConfigService; function file(relative: string, content = "", dir = root): string { @@ -78,8 +80,6 @@ suite("Vite+ server selection", () => { service.dispose(); findBinary.clearGlobalNodeModulesPathsCache(); mock.restoreAll(); - if (originalPath === undefined) delete process.env.PATH; - else process.env.PATH = originalPath; await commands.executeCommand("workbench.action.closeAllEditors"); for (const folder of [WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER]) { if (!folder) continue; @@ -311,42 +311,32 @@ suite("Vite+ server selection", () => { declare(); const shellBin = path.join(root, "shell-bin"); const vpPath = file(`vp.${extension}`, "", shellBin); - const originalPlatform = process.platform; - const originalEnv = process.env; - try { - Object.defineProperty(process, "platform", { value: "win32" }); - process.env = { ...originalEnv }; - for (const key of Object.keys(process.env)) { - if (key.toUpperCase() === "PATH") delete process.env[key]; - } - process.env.Path = shellBin; - // Use a fresh instance of the real provider to exercise its environment copy. - const { getShellEnv } = await import( - `../../client/getShellEnv.ts?windowsPath=${extension}` - ); - mock.method(shellEnv, "getShellEnv", getShellEnv); - - const binaries = await Promise.all([ - service.getOxlintServerBinPath(), - service.getOxfmtServerBinPath(), - ]); - await Promise.all( - binaries.map(async (binary) => { - strictEqual(binary?.path, vpPath); - const executable = await runExecutable(binary!, true); - strictEqual( - executable.options?.env?.PATH, - `${path.dirname(process.execPath)}${path.delimiter}${shellBin}`, - ); - strictEqual(executable.options?.env?.Path, undefined); - }), - ); - strictEqual(process.env.Path, shellBin); - strictEqual(process.env.PATH, undefined); - } finally { - Object.defineProperty(process, "platform", { value: originalPlatform }); - process.env = originalEnv; + setPlatform("win32"); + for (const key of Object.keys(process.env)) { + if (key.toUpperCase() === "PATH") delete process.env[key]; } + process.env.Path = shellBin; + // Use a fresh instance of the real provider to exercise its environment copy. + const { getShellEnv } = await import(`../../client/getShellEnv.ts?windowsPath=${extension}`); + mock.method(shellEnv, "getShellEnv", getShellEnv); + + const binaries = await Promise.all([ + service.getOxlintServerBinPath(), + service.getOxfmtServerBinPath(), + ]); + await Promise.all( + binaries.map(async (binary) => { + strictEqual(binary?.path, vpPath); + const executable = await runExecutable(binary!, true); + strictEqual( + executable.options?.env?.PATH, + `${path.dirname(process.execPath)}${path.delimiter}${shellBin}`, + ); + strictEqual(executable.options?.env?.Path, undefined); + }), + ); + strictEqual(process.env.Path, shellBin); + strictEqual(process.env.PATH, undefined); }); } From 8562609202a5d9a5c7d2bba8eb0671aee2918848 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 22:00:36 +0800 Subject: [PATCH 09/15] docs: require vite-plus 0.3.2 for runtime reuse --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 00ab6f21..58ceb1b5 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ You can also set `oxc.path.vp` to an absolute path or a path relative to the wor On Windows, use `./node_modules/.bin/vp.cmd` for npm or pnpm, or `./node_modules/.bin/vp.exe` for Bun. -With `oxc.useExecPath`, the extension runs npm and pnpm project entries with VS Code's bundled Node. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. Running without a system Node installation requires a Vite+ release that includes [the runtime reuse fix](https://github.com/voidzero-dev/vite-plus/pull/2673). Earlier releases still require `node` on `PATH` for their subprocesses. +With `oxc.useExecPath`, the extension runs npm and pnpm project entries with VS Code's bundled Node. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. Running without a system Node installation requires `vite-plus` `0.3.2` or later. Earlier releases still require `node` on `PATH` for their subprocesses. The source settings and `oxc.path.vp` can differ between workspace folders. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority over source selection for their respective tools. Changing a source setting restarts only its tool. From 78e42996a949b07efe69931d367ce02cb6f206b9 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 22:10:56 +0800 Subject: [PATCH 10/15] refactor: simplify Vite+ discovery and server launch --- client/ConfigService.ts | 70 ++++++++++++---------- client/detectVitePlus.ts | 28 ++++++--- client/extension.ts | 20 +++---- client/findBinary.ts | 52 ++++++++--------- client/tools/formatter.ts | 43 ++++++++------ client/tools/linter.ts | 41 +++++++------ client/tools/lsp_helper.ts | 78 ++++++++++++------------- tests/unit/vitePlusLifecycle.spec.ts | 86 ++++++++++++++++------------ 8 files changed, 221 insertions(+), 197 deletions(-) diff --git a/client/ConfigService.ts b/client/ConfigService.ts index 684fc7e9..49cd395d 100644 --- a/client/ConfigService.ts +++ b/client/ConfigService.ts @@ -1,8 +1,7 @@ import * as path from "node:path"; import { ConfigurationChangeEvent, Uri, window, workspace, WorkspaceFolder } from "vscode"; -import { detectVitePlusProject, VitePlusError } from "./detectVitePlus"; -import { getShellEnv } from "./getShellEnv"; import { DiagnosticPullMode } from "vscode-languageclient"; +import { detectVitePlusProject, VitePlusError } from "./detectVitePlus"; import { BinarySearchResult, clearGlobalNodeModulesPathsCache, @@ -12,6 +11,7 @@ import { searchSettingsBin, searchYarnPnpBin, } from "./findBinary"; +import { getShellEnv } from "./getShellEnv"; import { IDisposable } from "./types"; import { VSCodeConfig } from "./VSCodeConfig"; import { @@ -62,14 +62,10 @@ export class ConfigService implements IDisposable { workspaceUri: string; options: OxlintWorkspaceConfigInterface; }[] { - return [...this.workspaceConfigs.entries()].map(([path, config]) => { - const options = config.toOxlintConfig(isVitePlus); - - return { - workspaceUri: Uri.file(path).toString(), - options, - }; - }); + return [...this.workspaceConfigs.entries()].map(([path, config]) => ({ + workspaceUri: Uri.file(path).toString(), + options: config.toOxlintConfig(isVitePlus), + })); } public getFormatterServerConfig(isVitePlus = false): { @@ -156,33 +152,38 @@ export class ConfigService implements IDisposable { } private async searchVitePlus(command: "lint" | "fmt"): Promise { - if (!workspace.isTrusted) return null; + if (!workspace.isTrusted) { + return null; + } const documentUri = window.activeTextEditor?.document.uri; const activeFolder = documentUri?.scheme === "file" ? workspace.getWorkspaceFolder(documentUri) : undefined; - const folders = (activeFolder ? [activeFolder] : (workspace.workspaceFolders ?? [])).map( - (folder): VitePlusSearchFolder => { - const config = workspace.getConfiguration(ConfigService.namespace, folder.uri); - return { - root: folder.uri.fsPath, - start: activeFolder && documentUri ? path.dirname(documentUri.fsPath) : folder.uri.fsPath, - source: config.get(`${command}.binarySource`) ?? "auto", - configuredPath: config.get("path.vp"), - }; - }, - ); + const workspaceFolders = activeFolder ? [activeFolder] : (workspace.workspaceFolders ?? []); + const folders = workspaceFolders.map((folder): VitePlusSearchFolder => { + const config = workspace.getConfiguration(ConfigService.namespace, folder.uri); + return { + root: folder.uri.fsPath, + start: activeFolder && documentUri ? path.dirname(documentUri.fsPath) : folder.uri.fsPath, + source: config.get(`${command}.binarySource`) ?? "auto", + configuredPath: config.get("path.vp"), + }; + }); // Share only searches with the same document context and settings. A slow // search for another project must not select its binary after navigation. const key = JSON.stringify(folders); const pending = this.vitePlusSearches.get(key); - if (pending) return pending; + if (pending) { + return pending; + } const search = this.resolveVitePlus(folders); this.vitePlusSearches.set(key, search); try { return await search; } finally { - if (this.vitePlusSearches.get(key) === search) this.vitePlusSearches.delete(key); + if (this.vitePlusSearches.get(key) === search) { + this.vitePlusSearches.delete(key); + } } } @@ -190,24 +191,31 @@ export class ConfigService implements IDisposable { folders: VitePlusSearchFolder[], ): Promise { for (const { root, start, source, configuredPath } of folders) { - if (source === "oxc") continue; + if (source === "oxc") { + continue; + } if (configuredPath) { // An explicit vp path opts in without requiring a dependency declaration. // oxlint-disable-next-line no-await-in-loop -- workspace folder order is significant const binary = await searchSettingsBin("vp", configuredPath, root); - if (!binary) + if (!binary) { throw new VitePlusError(`Invalid Vite+ binary: ${configuredPath}. Check oxc.path.vp.`); + } return { ...binary, cwd: root }; } const project = detectVitePlusProject(start, source === "vite-plus", root); - if (!project) continue; + if (!project) { + continue; + } + if (project.vpPath) { + return { path: project.vpPath, loader: "native", cwd: project.root }; + } // Global vp is eligible only after detection or explicit opt-in. // oxlint-disable no-await-in-loop -- global lookup requires a Vite+ project - const binary: BinarySearchResult | undefined = project.vpPath - ? { path: project.vpPath, loader: "native" } - : ((await searchEnvPath("vp", await getShellEnv())) ?? - (await searchGlobalNodeModulesBin("vp", "vite-plus"))); + const binary = + (await searchEnvPath("vp", await getShellEnv())) ?? + (await searchGlobalNodeModulesBin("vp", "vite-plus")); // oxlint-enable no-await-in-loop if (!binary) { throw new VitePlusError( diff --git a/client/detectVitePlus.ts b/client/detectVitePlus.ts index be578acf..fd586d35 100644 --- a/client/detectVitePlus.ts +++ b/client/detectVitePlus.ts @@ -38,18 +38,20 @@ function isRootWorkspace(dir: string, pkg: PackageJson | null): boolean { */ export function detectVitePlusProject( start: string, - enabled = false, + forceVitePlus = false, workspaceFolder?: string, ): VitePlusProject | null { let dir = path.resolve(start); try { - if (statSync(dir).isFile()) dir = path.dirname(dir); + if (statSync(dir).isFile()) { + dir = path.dirname(dir); + } } catch { // The caller can also pass a directory that does not exist yet. } let pkg = readPackageJson(dir); - if (enabled) { + if (forceVitePlus) { // Explicit opt-in skips dependency detection, but still needs a stable cwd // when the active document moves between source directories. const fallbackRoot = workspaceFolder ? path.resolve(workspaceFolder) : dir; @@ -65,21 +67,29 @@ export function detectVitePlusProject( } } else { while (!pkg?.dependencies?.["vite-plus"] && !pkg?.devDependencies?.["vite-plus"]) { - if (isRootWorkspace(dir, pkg) || dir === path.dirname(dir)) return null; - dir = path.dirname(dir); + const parent = path.dirname(dir); + if (isRootWorkspace(dir, pkg) || dir === parent) { + return null; + } + dir = parent; pkg = readPackageJson(dir); } } const root = dir; + const binNames = process.platform === "win32" ? ["vp.cmd", "vp.exe"] : ["vp"]; while (true) { - const binNames = process.platform === "win32" ? ["vp.cmd", "vp.exe"] : ["vp"]; for (const name of binNames) { const vpPath = path.join(dir, "node_modules", ".bin", name); - if (existsSync(vpPath)) return { root, vpPath }; + if (existsSync(vpPath)) { + return { root, vpPath }; + } + } + const parent = path.dirname(dir); + if (isRootWorkspace(dir, pkg) || dir === parent) { + return { root }; } - if (isRootWorkspace(dir, pkg) || dir === path.dirname(dir)) return { root }; - dir = path.dirname(dir); + dir = parent; pkg = readPackageJson(dir); } } diff --git a/client/extension.ts b/client/extension.ts index c9cfa580..e4418031 100644 --- a/client/extension.ts +++ b/client/extension.ts @@ -10,7 +10,7 @@ import ToolInterface from "./tools/ToolInterface"; const outputChannelName = "Oxc"; const tools: ToolInterface[] = []; -export async function activate(context: ExtensionContext) { +export async function activate(context: ExtensionContext): Promise { const configService = new ConfigService(); const outputChannelLint = window.createOutputChannel(outputChannelName + " (Lint)", { @@ -74,7 +74,7 @@ export async function activate(context: ExtensionContext) { context.subscriptions.push(formatter); } - const restartTool = async (tool: ToolInterface, outputChannel: LogOutputChannel) => { + async function restartTool(tool: ToolInterface, outputChannel: LogOutputChannel): Promise { try { await tool.restart(); } catch (e) { @@ -82,7 +82,7 @@ export async function activate(context: ExtensionContext) { Try to restart the editor manually. `); } - }; + } configService.onConfigChange = async function onConfigChange(event) { await Promise.all(tools.map((tool) => tool.onConfigChange(event))); @@ -112,16 +112,11 @@ export async function activate(context: ExtensionContext) { const initialDocument = window.activeTextEditor?.document.uri.toString(); const binaryPaths = await Promise.all(tools.map((tool) => tool.getBinary())); - await Promise.all( - tools.map((tool): Promise => { - const binaryPath = binaryPaths[tools.indexOf(tool)]; - return tool.activate(binaryPath); - }), - ); + await Promise.all(tools.map((tool, index) => tool.activate(binaryPaths[index]))); // A window has one client per tool. Re-resolve on navigation, and restart // only when the executable, Vite+ command, or project directory changes. - const switchProject = async () => { + async function switchProject(): Promise { await Promise.all( tools.map(async (tool) => { try { @@ -134,14 +129,15 @@ export async function activate(context: ExtensionContext) { } }), ); - }; + } context.subscriptions.push( window.onDidChangeActiveTextEditor((editor) => { if ( editor?.document.uri.scheme !== "file" || !workspace.getWorkspaceFolder(editor.document.uri) - ) + ) { return; + } void switchProject(); }), ); diff --git a/client/findBinary.ts b/client/findBinary.ts index 151d23b7..9f381bc3 100644 --- a/client/findBinary.ts +++ b/client/findBinary.ts @@ -40,18 +40,25 @@ export function replaceTargetFromMainToBin(resolvedPath: string, binaryName: str throw new Error(`Could not find package.json for "${binaryName}"`); } +function binaryCandidates(folder: string, binaryName: string): string[] { + const basePath = path.join(folder, binaryName); + if (process.platform !== "win32") { + return [basePath]; + } + // Prefer Windows shims over the extensionless POSIX vp shim. + if (binaryName === "vp") { + return [`${basePath}.cmd`, `${basePath}.exe`, basePath]; + } + return [basePath, `${basePath}.exe`]; +} + async function searchNodeModulesDefaultBinPath( binaryName: string, folders: string[], ): Promise { - const candidates = folders.flatMap((folder) => { - const basePath = path.join(folder, ".bin", binaryName); - return process.platform === "win32" - ? binaryName === "vp" - ? [`${basePath}.cmd`, `${basePath}.exe`, basePath] - : [basePath, `${basePath}.exe`] - : [basePath]; - }); + const candidates = folders.flatMap((folder) => + binaryCandidates(path.join(folder, ".bin"), binaryName), + ); const exists = await Promise.all( candidates.map(async (candidate) => { @@ -261,19 +268,12 @@ export async function searchEnvPath( return undefined; } - // generate candidate paths by joining each PATH entry with the binary name - // on Windows, also consider the .exe extension + // Ignore empty PATH entries and preserve directory and executable priority. const candidates = envPath.split(path.delimiter).flatMap((folder) => { - // filter out empty entries which can occur if PATH starts or ends with a delimiter if (!folder) { return []; } - const basePath = path.join(folder, defaultBinaryName); - return process.platform === "win32" - ? defaultBinaryName === "vp" - ? [`${basePath}.cmd`, `${basePath}.exe`, basePath] - : [basePath, `${basePath}.exe`] - : [basePath]; + return binaryCandidates(folder, defaultBinaryName); }); const binary = await Promise.all( @@ -391,12 +391,10 @@ async function resolveGlobalNodeModulesPaths(): Promise { // only use this function with internal code, because it executes shell commands // which could be a security risk if the command or args are user-controlled -const safeSpawnSync = async ( +async function safeSpawnSync( command: string, args: readonly string[] = [], -): Promise => { - let output: string | undefined; - +): Promise { try { const result = spawnSync(command, args, { shell: true, @@ -405,14 +403,10 @@ const safeSpawnSync = async ( }); if (result.error || result.status !== 0) { - output = undefined; - } else { - const trimmed = result.stdout.trim(); - output = trimmed ? trimmed : undefined; + return undefined; } + return result.stdout.trim() || undefined; } catch { - output = undefined; + return undefined; } - - return output; -}; +} diff --git a/client/tools/formatter.ts b/client/tools/formatter.ts index 473b465d..98cc2aa2 100644 --- a/client/tools/formatter.ts +++ b/client/tools/formatter.ts @@ -1,6 +1,5 @@ import { promises as fsPromises } from "node:fs"; import { isDeepStrictEqual } from "node:util"; -import { VitePlusError } from "../detectVitePlus"; import { CodeAction, @@ -24,10 +23,11 @@ import { import { OxcCommands } from "../commands"; import { ConfigService } from "../ConfigService"; +import { VitePlusError } from "../detectVitePlus"; +import type { BinarySearchResult } from "../findBinary"; import StatusBarItemHandler from "../StatusBarItemHandler"; import { onClientNotification, runExecutable } from "./lsp_helper"; import ToolInterface from "./ToolInterface"; -import type { BinarySearchResult } from "../findBinary"; const languageClientName = "oxc"; @@ -331,21 +331,24 @@ export default class FormatterTool implements ToolInterface { try { bin = await this.configService.getOxfmtServerBinPath(); } catch (error) { - if (!(error instanceof VitePlusError)) throw error; + if (!(error instanceof VitePlusError)) { + throw error; + } this.binaryError = error.message; return undefined; } - if (bin) { - try { - await fsPromises.access(bin.path); - return bin; - } catch (e) { - this.outputChannel.error(`Invalid bin path: ${bin.path}`, e); - } + if (!bin) { + return undefined; + } + try { + await fsPromises.access(bin.path); + return bin; + } catch (error) { + this.outputChannel.error(`Invalid bin path: ${bin.path}`, error); } } - async activate(binary?: BinarySearchResult) { + async activate(binary?: BinarySearchResult): Promise { this.binary = binary; // No valid binary found for the formatter. if (!binary) { @@ -355,7 +358,7 @@ export default class FormatterTool implements ToolInterface { return Promise.resolve(); } - this.outputChannel.info(`Using server binary at: ${binary?.path}`); + this.outputChannel.info(`Using server binary at: ${binary.path}`); const run: Executable = await runExecutable( binary, @@ -381,10 +384,7 @@ export default class FormatterTool implements ToolInterface { workspace: { configuration: (params: ConfigurationParams) => { return params.items.map((item) => { - if (item.section !== "oxc_language_server") { - return null; - } - if (item.scopeUri === undefined) { + if (item.section !== "oxc_language_server" || item.scopeUri === undefined) { return null; } @@ -442,15 +442,18 @@ export default class FormatterTool implements ToolInterface { restart(onlyIfBinaryChanged = false): Promise { const restart = this.restartQueue.then(async () => { - if (!onlyIfBinaryChanged) this.configService.clearBinarySearchCaches(); + if (!onlyIfBinaryChanged) { + this.configService.clearBinarySearchCaches(); + } const previousError = this.binaryError; const newBinary = await this.getBinary(); if ( onlyIfBinaryChanged && isDeepStrictEqual(this.binary, newBinary) && previousError === this.binaryError - ) + ) { return; + } await this.stopClient(); await this.activate(newBinary); }); @@ -463,7 +466,9 @@ export default class FormatterTool implements ToolInterface { await this.client?.start(); this.binaryError = undefined; } catch (error) { - if (!this.binary?.vitePlus) throw error; + if (!this.binary?.vitePlus) { + throw error; + } this.binaryError = `Failed to start Vite+ ${this.binary.vitePlus} --lsp. Install or upgrade vite-plus in ${this.binary.cwd}, then restart the Oxc servers. ${error instanceof Error ? error.message : String(error)}`; this.outputChannel.error(this.binaryError); } diff --git a/client/tools/linter.ts b/client/tools/linter.ts index 5a5d584c..397cb356 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -1,6 +1,5 @@ import { promises as fsPromises } from "node:fs"; import { isDeepStrictEqual } from "node:util"; -import { VitePlusError } from "../detectVitePlus"; import { CodeActionKind, @@ -29,11 +28,12 @@ import { import { OxcCommands } from "../commands"; import { ConfigService } from "../ConfigService"; +import { VitePlusError } from "../detectVitePlus"; +import type { BinarySearchResult } from "../findBinary"; import StatusBarItemHandler from "../StatusBarItemHandler"; import { VSCodeConfig } from "../VSCodeConfig"; import { onClientNotification, runExecutable } from "./lsp_helper"; import ToolInterface from "./ToolInterface"; -import type { BinarySearchResult } from "../findBinary"; const languageClientName = "oxc"; @@ -219,17 +219,20 @@ export default class LinterTool implements ToolInterface { try { bin = await this.configService.getOxlintServerBinPath(); } catch (error) { - if (!(error instanceof VitePlusError)) throw error; + if (!(error instanceof VitePlusError)) { + throw error; + } this.binaryError = error.message; return undefined; } - if (bin) { - try { - await fsPromises.access(bin.path); - return bin; - } catch (e) { - this.outputChannel.error(`Invalid bin path: ${bin.path}`, e); - } + if (!bin) { + return undefined; + } + try { + await fsPromises.access(bin.path); + return bin; + } catch (error) { + this.outputChannel.error(`Invalid bin path: ${bin.path}`, error); } } @@ -260,7 +263,7 @@ export default class LinterTool implements ToolInterface { debug: run, }; - this.outputChannel.info(`Using server binary at: ${binary?.path}`); + this.outputChannel.info(`Using server binary at: ${binary.path}`); // see https://github.com/oxc-project/oxc/blob/9b475ad05b750f99762d63094174be6f6fc3c0eb/crates/oxc_linter/src/loader/partial_loader/mod.rs#L17-L20 const supportedExtensions = [ @@ -337,10 +340,7 @@ export default class LinterTool implements ToolInterface { workspace: { configuration: (params: ConfigurationParams) => { return params.items.map((item) => { - if (item.section !== "oxc_language_server") { - return null; - } - if (item.scopeUri === undefined) { + if (item.section !== "oxc_language_server" || item.scopeUri === undefined) { return null; } @@ -433,15 +433,18 @@ export default class LinterTool implements ToolInterface { restart(onlyIfBinaryChanged = false): Promise { const restart = this.restartQueue.then(async () => { - if (!onlyIfBinaryChanged) this.configService.clearBinarySearchCaches(); + if (!onlyIfBinaryChanged) { + this.configService.clearBinarySearchCaches(); + } const previousError = this.binaryError; const newBinary = await this.getBinary(); if ( onlyIfBinaryChanged && isDeepStrictEqual(this.binary, newBinary) && previousError === this.binaryError - ) + ) { return; + } await this.stopClient(); await this.activate(newBinary); }); @@ -454,7 +457,9 @@ export default class LinterTool implements ToolInterface { await this.client?.start(); this.binaryError = undefined; } catch (error) { - if (!this.binary?.vitePlus) throw error; + if (!this.binary?.vitePlus) { + throw error; + } this.binaryError = `Failed to start Vite+ ${this.binary.vitePlus} --lsp. Install or upgrade vite-plus in ${this.binary.cwd}, then restart the Oxc servers. ${error instanceof Error ? error.message : String(error)}`; this.outputChannel.error(this.binaryError); } diff --git a/client/tools/lsp_helper.ts b/client/tools/lsp_helper.ts index 08ae8cb9..269ff560 100644 --- a/client/tools/lsp_helper.ts +++ b/client/tools/lsp_helper.ts @@ -15,7 +15,9 @@ export async function runExecutable( ): Promise { if (binary.vitePlus && useExecPath && binary.loader === "native") { const nodeEntry = resolveVitePlusNodeEntry(binary.path); - if (nodeEntry) binary = { ...binary, path: nodeEntry, loader: "node" }; + if (nodeEntry) { + binary = { ...binary, path: nodeEntry, loader: "node" }; + } } const shellEnv = await getShellEnv(); @@ -31,10 +33,6 @@ export async function runExecutable( if (suppressProgramErrors) { serverEnv.OXLINT_TSGOLINT_DANGEROUSLY_SUPPRESS_PROGRAM_DIAGNOSTICS = "true"; } - // when the binary path ends with `oxlint/bin/oxlint` or a common js extension, we should run it with `node` - // the path is defined in `ConfigService.searchNodeModulesBin` - // Probably it would be better to read the shebang for unknown extensions, and run with `node` if the shebang contains `node`, - // but for now we can just check for common node extensions and the known path for `oxlint` const isNode = binary.loader === "node"; let nodeCommand: string; @@ -51,49 +49,45 @@ export async function runExecutable( serverEnv.PATH = `${nodeDir}${path.delimiter}${serverEnv.PATH ?? ""}`; } - const isWindows = process.platform === "win32"; const args = binary.vitePlus ? [binary.vitePlus, "--lsp"] : ["--lsp"]; - // In Yarn PnP environments, inject the PnP loaders so that both CJS require() - // and ESM import calls can resolve dependencies through PnP. - // --require .pnp.cjs: patches CJS resolution (e.g., oxlint's NAPI-RS bindings via createRequire) - // --loader .pnp.loader.mjs: patches ESM resolution (e.g., oxfmt's tinypool import) - const pnpArgs: string[] = []; - if (isNode && binary.yarnPnpLoaderPath) { - pnpArgs.push("--require", binary.yarnPnpLoaderPath); - const esmLoaderPath = path.join(path.dirname(binary.yarnPnpLoaderPath), ".pnp.loader.mjs"); - pnpArgs.push("--loader", pathToFileURL(esmLoaderPath).href); + if (isNode || (useExecPath && !binary.vitePlus)) { + // Yarn PnP needs loaders for both CJS require() and ESM imports. + const pnpArgs: string[] = []; + if (isNode && binary.yarnPnpLoaderPath) { + pnpArgs.push("--require", binary.yarnPnpLoaderPath); + const esmLoaderPath = path.join(path.dirname(binary.yarnPnpLoaderPath), ".pnp.loader.mjs"); + pnpArgs.push("--loader", pathToFileURL(esmLoaderPath).href); + } + + return { + command: nodeCommand, + args: [...pnpArgs, binary.path, ...args], + options: { + cwd: binary.cwd, + env: serverEnv, + }, + }; } - // vp can be a package-manager shell shim or a native executable. Neither - // can be interpreted as JavaScript, even when useExecPath is enabled. - return isNode || (useExecPath && !binary.vitePlus) - ? { - command: nodeCommand, - args: [...pnpArgs, binary.path, ...args], - options: { - cwd: binary.cwd, - env: serverEnv, - }, - } - : { - // On Windows with shell, quote the command path to handle spaces in usernames/paths - command: isWindows ? `"${binary.path}"` : binary.path, - args, - options: { - cwd: binary.cwd, - // On Windows we need to run the binary in a shell to be able to execute the shell npm bin script. - // Searching for the right `.exe` file inside `node_modules/` is not reliable as it depends on - // the package manager used (npm, yarn, pnpm, etc) and the package version. - // The npm bin script is a shell script that points to the actual binary. - // Security: We validated the user defined binary path in `configService.searchBinaryPath()`. - shell: isWindows, - env: serverEnv, - }, - }; + // Keep native vp binaries and unresolved shell shims out of the Node launch path. + const isWindows = process.platform === "win32"; + return { + // Windows package-manager shims need a shell; quote paths that can contain spaces. + command: isWindows ? `"${binary.path}"` : binary.path, + args, + options: { + cwd: binary.cwd, + shell: isWindows, + env: serverEnv, + }, + }; } -export function onClientNotification(params: ShowMessageParams, outputChannel: LogOutputChannel) { +export function onClientNotification( + params: ShowMessageParams, + outputChannel: LogOutputChannel, +): void { switch (params.type) { case MessageType.Debug: outputChannel.debug(params.message); diff --git a/tests/unit/vitePlusLifecycle.spec.ts b/tests/unit/vitePlusLifecycle.spec.ts index 4ed186f2..3c3f493f 100644 --- a/tests/unit/vitePlusLifecycle.spec.ts +++ b/tests/unit/vitePlusLifecycle.spec.ts @@ -2,7 +2,13 @@ import { deepStrictEqual, ok, strictEqual } from "assert"; import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import * as path from "node:path"; import { mock } from "node:test"; -import { CancellationTokenSource, ConfigurationTarget, window, workspace } from "vscode"; +import { + CancellationTokenSource, + ConfigurationTarget, + LogOutputChannel, + window, + workspace, +} from "vscode"; import type { LanguageClient } from "vscode-languageclient/node"; import { ConfigService } from "../../client/ConfigService"; import { @@ -15,18 +21,35 @@ import Formatter from "../../client/tools/formatter"; import Linter from "../../client/tools/linter"; import { WORKSPACE_FOLDER } from "../test-helpers"; +function getWorkspaceOptions(settings: { workspaceUri: string; options: unknown }[]): unknown { + return settings.find(({ workspaceUri }) => workspaceUri === WORKSPACE_FOLDER.uri.toString())! + .options; +} + for (const [Tool, command, getter] of [ [Linter, "lint", "getOxlintServerBinPath"], [Formatter, "fmt", "getOxfmtServerBinPath"], ] as const) { suite(`Vite+ ${command} lifecycle`, () => { const root = path.join(WORKSPACE_FOLDER.uri.fsPath, `vp-${command}-lifecycle`); + const nestedConfigKey = command === "lint" ? "disableNestedConfig" : "fmt.disableNestedConfig"; let service: ConfigService; let tool: Linter | Formatter; - let output: ReturnType; + let output: LogOutputChannel; let status: StatusBarItemHandler; let selected: BinarySearchResult; + function assertNestedConfig(options: unknown, expected: boolean): void { + const values = options as Record; + strictEqual(values[nestedConfigKey], expected); + if (command === "lint") { + strictEqual( + (values.flags as Record).disable_nested_config, + String(expected), + ); + } + } + setup(async () => { // No server needs to run to exercise client replacement on navigation. await workspace.getConfiguration("oxc").update("enable", false); @@ -36,10 +59,9 @@ for (const [Tool, command, getter] of [ selected = { path: vpPath, loader: "node", vitePlus: command, cwd: root }; service = new ConfigService(); mock.method(service, getter, async () => selected); - const channel = window.createOutputChannel(`Vite+ ${command} lifecycle`, { log: true }); - output = channel; + output = window.createOutputChannel(`Vite+ ${command} lifecycle`, { log: true }); status = new StatusBarItemHandler("test"); - tool = new Tool(channel, service, status); + tool = new Tool(output, service, status); await tool.activate(selected); }); @@ -54,39 +76,26 @@ for (const [Tool, command, getter] of [ await workspace.getConfiguration("oxc").update("enable", undefined); await workspace .getConfiguration("oxc", WORKSPACE_FOLDER.uri) - .update( - command === "lint" ? "disableNestedConfig" : "fmt.disableNestedConfig", - undefined, - ConfigurationTarget.WorkspaceFolder, - ); + .update(nestedConfigKey, undefined, ConfigurationTarget.WorkspaceFolder); rmSync(root, { recursive: true, force: true }); }); for (const userSetting of [false, true]) { test(`disables nested configs only for Vite+ with user setting ${userSetting}`, async () => { - const key = command === "lint" ? "disableNestedConfig" : "fmt.disableNestedConfig"; const config = workspace.getConfiguration("oxc", WORKSPACE_FOLDER.uri); - await config.update(key, userSetting, ConfigurationTarget.WorkspaceFolder); + await config.update(nestedConfigKey, userSetting, ConfigurationTarget.WorkspaceFolder); service.getWorkspaceConfig(WORKSPACE_FOLDER.uri)!.refresh(); - const event = { affectsConfiguration: (section: string) => section === `oxc.${key}` }; + const event = { + affectsConfiguration: (section: string) => section === `oxc.${nestedConfigKey}`, + }; - const check = async (expected: boolean) => { + async function checkClientConfig(expected: boolean): Promise { const client = (tool as unknown as { client: LanguageClient }).client; - const optionsForWorkspace = (settings: { workspaceUri: string; options: unknown }[]) => - settings.find(({ workspaceUri }) => workspaceUri === WORKSPACE_FOLDER.uri.toString())! - .options; - const assertOptions = (options: unknown) => { - const values = options as Record; - strictEqual(values[key], expected); - if (command === "lint") { - strictEqual( - (values.flags as Record).disable_nested_config, - String(expected), - ); - } - }; - - assertOptions(optionsForWorkspace(client.clientOptions.initializationOptions)); + + assertNestedConfig( + getWorkspaceOptions(client.clientOptions.initializationOptions), + expected, + ); const cancellation = new CancellationTokenSource(); try { const pulled = await client.clientOptions.middleware!.workspace!.configuration!( @@ -101,7 +110,7 @@ for (const [Tool, command, getter] of [ async () => [], ); ok(Array.isArray(pulled)); - assertOptions(pulled[0]); + assertNestedConfig(pulled[0], expected); deepStrictEqual(pulled.slice(1), [null, null]); } finally { cancellation.dispose(); @@ -114,27 +123,30 @@ for (const [Tool, command, getter] of [ strictEqual(notification.mock.callCount(), 1); const [method, params] = notification.mock.calls[0].arguments; strictEqual(method, "workspace/didChangeConfiguration"); - assertOptions(optionsForWorkspace(params.settings)); - assertOptions(optionsForWorkspace(client.clientOptions.initializationOptions)); + assertNestedConfig(getWorkspaceOptions(params.settings), expected); + assertNestedConfig( + getWorkspaceOptions(client.clientOptions.initializationOptions), + expected, + ); } finally { running.mock.restore(); notification.mock.restore(); } strictEqual( - workspace.getConfiguration("oxc", WORKSPACE_FOLDER.uri).get(key), + workspace.getConfiguration("oxc", WORKSPACE_FOLDER.uri).get(nestedConfigKey), userSetting, "the saved setting must not change", ); - }; + } - await check(true); + await checkClientConfig(true); const vitePlus = selected; selected = { path: selected.path, loader: "node" }; await tool.restart(true); - await check(userSetting); + await checkClientConfig(userSetting); selected = vitePlus; await tool.restart(true); - await check(true); + await checkClientConfig(true); }); } From 0cf3b15f9677f9181c7196ba7758bd850cbde3fd Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 12 Sep 2026 22:22:54 +0800 Subject: [PATCH 11/15] fix: use bundled Node for global pnpm Vite+ shims --- README.md | 2 +- client/resolveVitePlusNodeEntry.ts | 76 ++++++++++++++++++------------ tests/unit/lsp_helper.spec.ts | 58 +++++++++++++++++++---- 3 files changed, 96 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 58ceb1b5..fd8cc907 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ You can also set `oxc.path.vp` to an absolute path or a path relative to the wor On Windows, use `./node_modules/.bin/vp.cmd` for npm or pnpm, or `./node_modules/.bin/vp.exe` for Bun. -With `oxc.useExecPath`, the extension runs npm and pnpm project entries with VS Code's bundled Node. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. Running without a system Node installation requires `vite-plus` `0.3.2` or later. Earlier releases still require `node` on `PATH` for their subprocesses. +With `oxc.useExecPath`, the extension runs npm and pnpm entries with VS Code's bundled Node. This includes global pnpm installations. You can also set `oxc.path.vp` to `./node_modules/vite-plus/bin/vp` to select the JavaScript entry directly. Running without a system Node installation requires `vite-plus` `0.3.2` or later. Earlier releases still require `node` on `PATH` for their subprocesses. The source settings and `oxc.path.vp` can differ between workspace folders. Explicit `oxc.path.oxlint` and `oxc.path.oxfmt` settings take priority over source selection for their respective tools. Changing a source setting restarts only its tool. diff --git a/client/resolveVitePlusNodeEntry.ts b/client/resolveVitePlusNodeEntry.ts index c6a72e19..bef0f191 100644 --- a/client/resolveVitePlusNodeEntry.ts +++ b/client/resolveVitePlusNodeEntry.ts @@ -1,41 +1,55 @@ -import { closeSync, openSync, readSync } from "node:fs"; +import { closeSync, openSync, readFileSync, readSync } from "node:fs"; import * as path from "node:path"; +const nodeShebangPattern = /^#!\s*(?:\S*\/node|\S*\/env\s+(?:-S\s+)?node)(?:\s|$)/; + +function readShebang(file: string): string { + const fd = openSync(file, "r"); + try { + // A standalone native vp can be large; only read its header. + const buffer = Buffer.alloc(256); + const length = readSync(fd, buffer, 0, buffer.length, 0); + return buffer.toString("utf8", 0, length).split(/\r?\n/, 1)[0]; + } finally { + closeSync(fd); + } +} + /** Resolve Node entry points without interpreting shell shims or native vp binaries as JS. */ export function resolveVitePlusNodeEntry(vpPath: string): string | undefined { - const binDir = path.dirname(vpPath); - const candidates = [vpPath]; - if (path.basename(binDir) === ".bin") { - // pnpm and Windows npm shims sit next to the installed package. - candidates.push(path.resolve(binDir, "..", "vite-plus", "bin", "vp")); - } else if (path.extname(vpPath) === ".cmd") { - // Global npm shims on Windows sit next to node_modules. - candidates.push(path.join(binDir, "node_modules", "vite-plus", "bin", "vp")); - } + try { + const shebang = readShebang(vpPath); + if (nodeShebangPattern.test(shebang)) { + return vpPath; + } - for (const candidate of candidates) { - let fd: number | undefined; - try { - fd = openSync(candidate, "r"); - // Read only the shebang: a standalone native vp can be a large file. - const buffer = Buffer.alloc(256); - const length = readSync(fd, buffer, 0, buffer.length, 0); - const shebang = buffer.toString("utf8", 0, length).split(/\r?\n/, 1)[0]; - if (/^#!\s*(?:\S*\/node|\S*\/env\s+(?:-S\s+)?node)(?:\s|$)/.test(shebang)) return candidate; - if ( - candidate === vpPath && - path.extname(vpPath) !== ".cmd" && - !/^#!.*(?:\/|\s)(?:sh|bash|dash|zsh|ksh)(?:\s|$)/.test(shebang) - ) { - // A native vp in .bin must not be replaced by an adjacent JS package. - return undefined; - } - } catch { - // The candidate is absent or unreadable; keep the original executable. + const isCmd = path.extname(vpPath) === ".cmd"; + if (!isCmd && !/^#!.*(?:\/|\s)(?:sh|bash|dash|zsh|ksh)(?:\s|$)/.test(shebang)) { return undefined; - } finally { - if (fd !== undefined) closeSync(fd); } + + // Read the literal target used by npm/pnpm shims, including custom global directories. + // Do not execute the shim or expand shell variables to locate the entry. + const shim = readFileSync(vpPath, "utf8"); + const target = isCmd + ? /"%(?:~dp0|dp0%)[\\/]([^"\r\n]+)"[ \t]+%\*/.exec(shim) + : /"\$basedir\/([^"\r\n]+)"[ \t]+"\$@"/.exec(shim); + const binDir = path.dirname(vpPath); + let nodeEntry: string | undefined; + if (target) { + const relativeTarget = isCmd ? target[1].replaceAll("\\", path.sep) : target[1]; + nodeEntry = path.resolve(binDir, relativeTarget); + } else if (path.basename(binDir) === ".bin") { + nodeEntry = path.resolve(binDir, "..", "vite-plus", "bin", "vp"); + } else if (isCmd) { + nodeEntry = path.join(binDir, "node_modules", "vite-plus", "bin", "vp"); + } + + if (nodeEntry && nodeShebangPattern.test(readShebang(nodeEntry))) { + return nodeEntry; + } + } catch { + // An absent or unreadable entry must leave the original executable intact. } return undefined; } diff --git a/tests/unit/lsp_helper.spec.ts b/tests/unit/lsp_helper.spec.ts index b20fe117..ea519830 100644 --- a/tests/unit/lsp_helper.spec.ts +++ b/tests/unit/lsp_helper.spec.ts @@ -58,15 +58,30 @@ suite("runExecutable", () => { }); } - for (const shimType of ["npm", "pnpm", "cmd", "global-cmd"] as const) { + for (const shimType of [ + "npm", + "pnpm", + "cmd", + "global-cmd", + "global-pnpm", + "global-pnpm-cmd", + ] as const) { for (const command of ["lint", "fmt"] as const) { test(`runs ${shimType} vp ${command} with bundled Node and no system Node`, async function () { if (shimType === "npm" && originalPlatform === "win32") this.skip(); - const nodeEntry = path.join(tempDir, "node_modules", "vite-plus", "bin", "vp"); - const shim = - shimType === "global-cmd" - ? path.join(tempDir, "vp.cmd") - : path.join(tempDir, "node_modules", ".bin", shimType === "cmd" ? "vp.cmd" : "vp"); + const isGlobalPnpm = shimType === "global-pnpm" || shimType === "global-pnpm-cmd"; + const isCmd = shimType.endsWith("cmd"); + const modulesDir = isGlobalPnpm + ? path.join(tempDir, "custom global store", "5", "node_modules") + : path.join(tempDir, "node_modules"); + const nodeEntry = path.join(modulesDir, "vite-plus", "bin", "vp"); + let binDir = path.join(tempDir, "node_modules", ".bin"); + if (isGlobalPnpm) { + binDir = path.join(tempDir, "global bin"); + } else if (shimType === "global-cmd") { + binDir = tempDir; + } + const shim = path.join(binDir, isCmd ? "vp.cmd" : "vp"); mkdirSync(path.dirname(nodeEntry), { recursive: true }); mkdirSync(path.dirname(shim), { recursive: true }); writeFileSync( @@ -88,12 +103,26 @@ process.stderr.write(child.stderr); process.exit(child.status ?? 1); `, ); - if (shimType === "npm") symlinkSync(nodeEntry, shim); - else + if (isGlobalPnpm) { + const relativeEntry = path.relative(binDir, nodeEntry); + // pnpm shims can contain long NODE_PATH setup before the launch command. + const padding = " ".repeat(512); + const script = isCmd + ? `@SETLOCAL\r\n@REM ${padding}\r\nnode "%~dp0\\${relativeEntry.replaceAll(path.sep, "\\")}" %*\r\n` + : `#!/bin/sh\n# ${padding}\nexec node "$basedir/${relativeEntry.replaceAll(path.sep, "/")}" "$@"\n`; + writeFileSync(shim, script); + // The recorded target must win over an unrelated adjacent installation. + const adjacentEntry = path.join(binDir, "node_modules", "vite-plus", "bin", "vp"); + mkdirSync(path.dirname(adjacentEntry), { recursive: true }); + writeFileSync(adjacentEntry, "#!/usr/bin/env node\nthrow new Error('wrong entry');\n"); + } else if (shimType === "npm") { + symlinkSync(nodeEntry, shim); + } else { writeFileSync( shim, shimType === "pnpm" ? '#!/bin/sh\nexec node "$@"\n' : "@echo off\r\nnode %*\r\n", ); + } process.env.PATH = path.join(tempDir, "no-system-node"); const result = await runExecutable( @@ -115,6 +144,19 @@ process.exit(child.status ?? 1); } } + test("keeps a global shim when its recorded target is not a Node entry", async () => { + const shim = path.join(tempDir, "vp.cmd"); + writeFileSync(shim, '@echo off\r\n"%~dp0\\native-vp" %*\r\n'); + writeFileSync(path.join(tempDir, "native-vp"), Buffer.from([0x7f, 0x45, 0x4c, 0x46])); + const adjacentEntry = path.join(tempDir, "node_modules", "vite-plus", "bin", "vp"); + mkdirSync(path.dirname(adjacentEntry), { recursive: true }); + writeFileSync(adjacentEntry, "#!/usr/bin/env node\n"); + + const result = await runExecutable({ path: shim, loader: "native", vitePlus: "lint" }, true); + strictEqual(result.command, process.platform === "win32" ? `"${shim}"` : shim); + deepStrictEqual(result.args, ["lint", "--lsp"]); + }); + test("keeps a standalone native vp executable with useExecPath", async () => { const vpPath = path.join(tempDir, "node_modules", ".bin", "vp"); const nodeEntry = path.join(tempDir, "node_modules", "vite-plus", "bin", "vp"); From 8653c9b89ce520b2e5cb63c21f4d72e1a477d93f Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 13 Sep 2026 00:03:28 +0800 Subject: [PATCH 12/15] fix: resolve Vite+ shims and recover after startup failures --- client/resolveVitePlusNodeEntry.ts | 14 ++++-- client/tools/formatter.ts | 24 +++++---- client/tools/linter.ts | 34 ++++++------- tests/unit/extension.spec.ts | 6 +++ tests/unit/lsp_helper.spec.ts | 22 +++++++-- tests/unit/resolveVitePlusNodeEntry.spec.ts | 55 +++++++++++++++++++++ tests/unit/vitePlusLifecycle.spec.ts | 15 +++++- 7 files changed, 128 insertions(+), 42 deletions(-) create mode 100644 tests/unit/resolveVitePlusNodeEntry.spec.ts diff --git a/client/resolveVitePlusNodeEntry.ts b/client/resolveVitePlusNodeEntry.ts index bef0f191..9b01b456 100644 --- a/client/resolveVitePlusNodeEntry.ts +++ b/client/resolveVitePlusNodeEntry.ts @@ -1,4 +1,4 @@ -import { closeSync, openSync, readFileSync, readSync } from "node:fs"; +import { closeSync, lstatSync, openSync, readFileSync, readSync, realpathSync } from "node:fs"; import * as path from "node:path"; const nodeShebangPattern = /^#!\s*(?:\S*\/node|\S*\/env\s+(?:-S\s+)?node)(?:\s|$)/; @@ -32,13 +32,17 @@ export function resolveVitePlusNodeEntry(vpPath: string): string | undefined { // Do not execute the shim or expand shell variables to locate the entry. const shim = readFileSync(vpPath, "utf8"); const target = isCmd - ? /"%(?:~dp0|dp0%)[\\/]([^"\r\n]+)"[ \t]+%\*/.exec(shim) + ? /"((?:%(?:~dp0|dp0%)[\\/]|[a-zA-Z]:[\\/]|\\\\)[^"\r\n]+)"[ \t]+%\*/.exec(shim) : /"\$basedir\/([^"\r\n]+)"[ \t]+"\$@"/.exec(shim); - const binDir = path.dirname(vpPath); + // pnpm shell shims can follow symlinks before computing their base directory. + const shimPath = !isCmd && lstatSync(vpPath).isSymbolicLink() ? realpathSync(vpPath) : vpPath; + const binDir = path.dirname(shimPath); let nodeEntry: string | undefined; if (target) { - const relativeTarget = isCmd ? target[1].replaceAll("\\", path.sep) : target[1]; - nodeEntry = path.resolve(binDir, relativeTarget); + const entryPath = isCmd + ? target[1].replace(/^%(?:~dp0|dp0%)[\\/]/, "").replaceAll("\\", path.sep) + : target[1]; + nodeEntry = path.resolve(binDir, entryPath); } else if (path.basename(binDir) === ".bin") { nodeEntry = path.resolve(binDir, "..", "vite-plus", "bin", "vp"); } else if (isCmd) { diff --git a/client/tools/formatter.ts b/client/tools/formatter.ts index 98cc2aa2..e9ecc7a2 100644 --- a/client/tools/formatter.ts +++ b/client/tools/formatter.ts @@ -267,7 +267,7 @@ export default class FormatterTool implements ToolInterface { })), ]; - private disposeResources: (() => Promise) | undefined; + private disposeResources: (() => void) | undefined; // Command and provider disposables (registered once at construction) private readonly restartCommand: { dispose: () => void }; @@ -409,12 +409,7 @@ export default class FormatterTool implements ToolInterface { }, ); - this.disposeResources = async () => { - try { - await this.client?.dispose(); - } catch { - // do nothing, the client may already be stopped - } + this.disposeResources = () => { onNotificationDispose.dispose(); }; @@ -431,13 +426,16 @@ export default class FormatterTool implements ToolInterface { private async stopClient(): Promise { try { - await this.client?.stop(); - } catch { - // do nothing, the client may already be stopped + await this.client?.dispose(); + } catch (error) { + // A client whose startup failed can reject disposal. Still release our resources + // so a corrected executable can start without reloading the window. + this.outputChannel.warn("Failed to dispose the oxfmt client.", error); + } finally { + this.disposeResources?.(); + this.disposeResources = undefined; + this.client = undefined; } - await this.disposeResources?.(); - this.disposeResources = undefined; - this.client = undefined; } restart(onlyIfBinaryChanged = false): Promise { diff --git a/client/tools/linter.ts b/client/tools/linter.ts index 397cb356..01cf832a 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -150,7 +150,7 @@ export default class LinterTool implements ToolInterface { private binaryError: string | undefined; private restartQueue: Promise = Promise.resolve(); - private disposeResources: (() => Promise) | undefined; + private disposeResources: (() => void) | undefined; // Command disposables (registered once at construction) private readonly restartCommand: { dispose: () => void }; @@ -371,6 +371,12 @@ export default class LinterTool implements ToolInterface { }); let activatorDispatcher: { dispose: () => void } | undefined; + this.disposeResources = () => { + onNotificationDispose.dispose(); + onDeleteFilesDispose.dispose(); + activatorDispatcher?.dispose(); + }; + if (this.allowedToStartServer) { if (this.configService.vsCodeConfig.enableOxlint) { await this.startClient(); @@ -379,17 +385,6 @@ export default class LinterTool implements ToolInterface { activatorDispatcher = this.generateActivatorByConfig(this.configService.vsCodeConfig); } - this.disposeResources = async () => { - try { - await this.client?.dispose(); - } catch { - // do nothing, the client may already be stopped - } - onNotificationDispose.dispose(); - onDeleteFilesDispose.dispose(); - activatorDispatcher?.dispose(); - }; - this.updateStatusBar(this.configService.vsCodeConfig.enableOxlint); } @@ -400,13 +395,16 @@ export default class LinterTool implements ToolInterface { private async stopClient(): Promise { try { - await this.client?.stop(); - } catch { - // do nothing, the client may already be stopped + await this.client?.dispose(); + } catch (error) { + // A client whose startup failed can reject disposal. Still release our resources + // so a corrected executable can start without reloading the window. + this.outputChannel.warn("Failed to dispose the oxlint client.", error); + } finally { + this.disposeResources?.(); + this.disposeResources = undefined; + this.client = undefined; } - await this.disposeResources?.(); - this.disposeResources = undefined; - this.client = undefined; } dispose(): void { diff --git a/tests/unit/extension.spec.ts b/tests/unit/extension.spec.ts index 954d536f..e0739f88 100644 --- a/tests/unit/extension.spec.ts +++ b/tests/unit/extension.spec.ts @@ -22,6 +22,12 @@ suite("navigation during extension activation", () => { for (const name of ["a", "b"]) writeFileSync(path.join(root, `${name}.txt`), ""); // The test host has already registered the extension's commands. mock.method(commands, "registerCommand", () => ({ dispose() {} })); + // Log channels with the same name share a logger in VS Code. Keep disposal + // of these test instances from closing the activated extension's channels. + const createOutputChannel = window.createOutputChannel; + mock.method(window, "createOutputChannel", (name: string, options: { log: true }) => + createOutputChannel(`Activation test ${name}`, options), + ); context = { extension: { packageJSON: { version: "test" } }, subscriptions: [], diff --git a/tests/unit/lsp_helper.spec.ts b/tests/unit/lsp_helper.spec.ts index ea519830..64be1936 100644 --- a/tests/unit/lsp_helper.spec.ts +++ b/tests/unit/lsp_helper.spec.ts @@ -1,6 +1,6 @@ import { deepStrictEqual, strictEqual } from "assert"; import { spawnSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { mock } from "node:test"; import { runExecutable } from "../../client/tools/lsp_helper"; @@ -64,12 +64,15 @@ suite("runExecutable", () => { "cmd", "global-cmd", "global-pnpm", + "global-pnpm-symlink", "global-pnpm-cmd", ] as const) { for (const command of ["lint", "fmt"] as const) { test(`runs ${shimType} vp ${command} with bundled Node and no system Node`, async function () { - if (shimType === "npm" && originalPlatform === "win32") this.skip(); - const isGlobalPnpm = shimType === "global-pnpm" || shimType === "global-pnpm-cmd"; + if ((shimType === "npm" || shimType.endsWith("symlink")) && originalPlatform === "win32") { + this.skip(); + } + const isGlobalPnpm = shimType.startsWith("global-pnpm"); const isCmd = shimType.endsWith("cmd"); const modulesDir = isGlobalPnpm ? path.join(tempDir, "custom global store", "5", "node_modules") @@ -81,7 +84,7 @@ suite("runExecutable", () => { } else if (shimType === "global-cmd") { binDir = tempDir; } - const shim = path.join(binDir, isCmd ? "vp.cmd" : "vp"); + let shim = path.join(binDir, isCmd ? "vp.cmd" : "vp"); mkdirSync(path.dirname(nodeEntry), { recursive: true }); mkdirSync(path.dirname(shim), { recursive: true }); writeFileSync( @@ -115,6 +118,12 @@ process.exit(child.status ?? 1); const adjacentEntry = path.join(binDir, "node_modules", "vite-plus", "bin", "vp"); mkdirSync(path.dirname(adjacentEntry), { recursive: true }); writeFileSync(adjacentEntry, "#!/usr/bin/env node\nthrow new Error('wrong entry');\n"); + if (shimType === "global-pnpm-symlink") { + const alias = path.join(tempDir, "linked bin", "nested", "vp"); + mkdirSync(path.dirname(alias), { recursive: true }); + symlinkSync(shim, alias); + shim = alias; + } } else if (shimType === "npm") { symlinkSync(nodeEntry, shim); } else { @@ -130,7 +139,10 @@ process.exit(child.status ?? 1); true, ); strictEqual(result.command, process.execPath); - deepStrictEqual(result.args, [shimType === "npm" ? shim : nodeEntry, command, "--lsp"]); + let expectedEntry = nodeEntry; + if (shimType === "npm") expectedEntry = shim; + if (shimType === "global-pnpm-symlink") expectedEntry = realpathSync(nodeEntry); + deepStrictEqual(result.args, [expectedEntry, command, "--lsp"]); strictEqual(result.options?.env?.ELECTRON_RUN_AS_NODE, "1"); // vp must reuse process.execPath for its subprocess, without PATH shims. const child = spawnSync(result.command, result.args, { diff --git a/tests/unit/resolveVitePlusNodeEntry.spec.ts b/tests/unit/resolveVitePlusNodeEntry.spec.ts new file mode 100644 index 00000000..ab9e71f6 --- /dev/null +++ b/tests/unit/resolveVitePlusNodeEntry.spec.ts @@ -0,0 +1,55 @@ +import { strictEqual } from "node:assert"; +import { readFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import * as path from "node:path"; +import { runInNewContext } from "node:vm"; + +// Exercise the compiled resolver with Windows path semantics on every CI platform. +// These tests do not claim to exercise Windows process creation. +suite("Windows Vite+ shim targets", () => { + const filename = path.join(__dirname, "../client/resolveVitePlusNodeEntry.js"); + const source = readFileSync(filename, "utf8"); + const realRequire = createRequire(filename); + const shimPath = "C:\\global bin\\vp.cmd"; + + for (const target of [ + "C:\\global store\\5\\node_modules\\vite-plus\\bin\\vp", + "D:\\global store\\5\\node_modules\\vite-plus\\bin\\vp", + "\\\\server\\share\\vite-plus\\bin\\vp", + ]) { + test(`resolves an absolute target: ${target}`, () => { + const files = new Map([ + [shimPath, `@echo off\r\nnode "${target}" %*\r\n`], + [target, "#!/usr/bin/env node\n"], + ]); + function read(file: string): string { + const contents = files.get(file); + if (contents === undefined) throw new Error(`ENOENT: ${file}`); + return contents; + } + const exports: typeof import("../../client/resolveVitePlusNodeEntry") = runInNewContext( + `${source}\nexports;`, + { + Buffer, + exports: {}, + require(id: string) { + if (id === "node:path") return path.win32; + if (id === "node:fs") { + return { + openSync: (file: string) => file, + closeSync() {}, + readFileSync: read, + realpathSync: (file: string) => file, + readSync(file: string, buffer: Buffer, offset: number, length: number) { + return buffer.write(read(file).slice(0, length), offset, length, "utf8"); + }, + }; + } + return realRequire(id); + }, + }, + ); + strictEqual(exports.resolveVitePlusNodeEntry(shimPath), target); + }); + } +}); diff --git a/tests/unit/vitePlusLifecycle.spec.ts b/tests/unit/vitePlusLifecycle.spec.ts index 3c3f493f..cf618af5 100644 --- a/tests/unit/vitePlusLifecycle.spec.ts +++ b/tests/unit/vitePlusLifecycle.spec.ts @@ -1,4 +1,4 @@ -import { deepStrictEqual, ok, strictEqual } from "assert"; +import { deepStrictEqual, notStrictEqual, ok, rejects, strictEqual } from "assert"; import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import * as path from "node:path"; import { mock } from "node:test"; @@ -171,6 +171,19 @@ for (const [Tool, command, getter] of [ ); }); + test("replaces a client after its server fails to launch", async () => { + mock.getter(service.vsCodeConfig, "useExecPath", () => false); + mock.getter(service.vsCodeConfig, "nodePath", () => path.join(root, "missing-node")); + await tool.restart(); + const failedClient = (tool as unknown as { client: LanguageClient }).client; + await rejects(failedClient.start()); + + await tool.restart(); + const replacement = (tool as unknown as { client: LanguageClient }).client; + notStrictEqual(replacement, failedClient); + ok(replacement, "a failed startup must not prevent the next activation"); + }); + test("navigation reuses global locations and an explicit restart refreshes them", async () => { const name = command === "lint" ? "oxlint" : "oxfmt"; const firstModules = path.join(root, "first", "node_modules"); From 2e22cbf90060cd84f0ecd7e250b2fa2e89913696 Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 13 Sep 2026 00:15:01 +0800 Subject: [PATCH 13/15] refactor: simplify binary selection and tool lifecycle --- client/findBinary.ts | 13 ++++--------- client/tools/formatter.ts | 13 +++++-------- client/tools/linter.ts | 14 ++++++-------- client/tools/lsp_helper.ts | 12 ++++++++---- tests/unit/detectVitePlus.spec.ts | 5 ++++- tests/unit/extension.spec.ts | 4 ++-- tests/unit/getShellEnv.spec.ts | 4 +--- tests/unit/vitePlus.spec.ts | 11 ++++++----- 8 files changed, 36 insertions(+), 40 deletions(-) diff --git a/client/findBinary.ts b/client/findBinary.ts index 9f381bc3..8d46a11c 100644 --- a/client/findBinary.ts +++ b/client/findBinary.ts @@ -60,23 +60,18 @@ async function searchNodeModulesDefaultBinPath( binaryCandidates(path.join(folder, ".bin"), binaryName), ); - const exists = await Promise.all( + const binaries = await Promise.all( candidates.map(async (candidate) => { try { await workspace.fs.stat(Uri.file(candidate)); - return true; + return { path: candidate, loader: "native" } as const; } catch { - return false; + return undefined; } }), ); - const firstExistingCandidateIndex = exists.findIndex(Boolean); - if (firstExistingCandidateIndex === -1) { - return undefined; - } - - return { path: candidates[firstExistingCandidateIndex], loader: "native" }; + return binaries.find(Boolean); } /** * Returns node_modules paths derived from all package.json files found in the workspace. diff --git a/client/tools/formatter.ts b/client/tools/formatter.ts index e9ecc7a2..4501d9bb 100644 --- a/client/tools/formatter.ts +++ b/client/tools/formatter.ts @@ -350,12 +350,11 @@ export default class FormatterTool implements ToolInterface { async activate(binary?: BinarySearchResult): Promise { this.binary = binary; - // No valid binary found for the formatter. if (!binary) { const message = this.binaryError ?? "No valid oxfmt binary found."; this.statusBarItemHandler.updateTool("formatter", false, message); this.outputChannel.warn(message); - return Promise.resolve(); + return; } this.outputChannel.info(`Using server binary at: ${binary.path}`); @@ -481,10 +480,8 @@ export default class FormatterTool implements ToolInterface { if (!this.configService.vsCodeConfig.enableOxfmt) { await this.client.stop(); } - } else { - if (this.configService.vsCodeConfig.enableOxfmt) { - await this.startClient(); - } + } else if (this.configService.vsCodeConfig.enableOxfmt) { + await this.startClient(); } } @@ -499,7 +496,7 @@ export default class FormatterTool implements ToolInterface { event.affectsConfiguration(`${ConfigService.namespace}.enable`) || event.affectsConfiguration(`${ConfigService.namespace}.enable.oxfmt`) ) { - await this.toggleClient(); // update the client state + await this.toggleClient(); } this.updateStatusBar(); @@ -524,7 +521,7 @@ export default class FormatterTool implements ToolInterface { this.formatActionProvider.dispose(); } - private updateStatusBar() { + private updateStatusBar(): void { const enable = this.configService.vsCodeConfig.enableOxfmt; let text = diff --git a/client/tools/linter.ts b/client/tools/linter.ts index 01cf832a..4a7e7abb 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -242,7 +242,7 @@ export default class LinterTool implements ToolInterface { const message = this.binaryError ?? "No valid oxlint binary found."; this.statusBarItemHandler.updateTool("linter", false, message); this.outputChannel.warn(message); - return Promise.resolve(); + return; } this.allowedToStartServer = @@ -413,19 +413,17 @@ export default class LinterTool implements ToolInterface { this.applyAllFixesCommand.dispose(); } - async toggleClient(configService: ConfigService): Promise { + async toggleClient(): Promise { if (this.client === undefined || !this.allowedToStartServer) { return; } if (this.client.isRunning()) { - if (!configService.vsCodeConfig.enableOxlint) { + if (!this.configService.vsCodeConfig.enableOxlint) { await this.client.stop(); } - } else { - if (configService.vsCodeConfig.enableOxlint) { - await this.startClient(); - } + } else if (this.configService.vsCodeConfig.enableOxlint) { + await this.startClient(); } } @@ -474,7 +472,7 @@ export default class LinterTool implements ToolInterface { event.affectsConfiguration(`${ConfigService.namespace}.enable`) || event.affectsConfiguration(`${ConfigService.namespace}.enable.oxlint`) ) { - await this.toggleClient(this.configService); // update the client state + await this.toggleClient(); } this.updateStatusBar(this.configService.vsCodeConfig.enableOxlint); diff --git a/client/tools/lsp_helper.ts b/client/tools/lsp_helper.ts index 269ff560..8b1a4468 100644 --- a/client/tools/lsp_helper.ts +++ b/client/tools/lsp_helper.ts @@ -52,17 +52,21 @@ export async function runExecutable( const args = binary.vitePlus ? [binary.vitePlus, "--lsp"] : ["--lsp"]; if (isNode || (useExecPath && !binary.vitePlus)) { + const nodeArgs = [binary.path, ...args]; // Yarn PnP needs loaders for both CJS require() and ESM imports. - const pnpArgs: string[] = []; if (isNode && binary.yarnPnpLoaderPath) { - pnpArgs.push("--require", binary.yarnPnpLoaderPath); const esmLoaderPath = path.join(path.dirname(binary.yarnPnpLoaderPath), ".pnp.loader.mjs"); - pnpArgs.push("--loader", pathToFileURL(esmLoaderPath).href); + nodeArgs.unshift( + "--require", + binary.yarnPnpLoaderPath, + "--loader", + pathToFileURL(esmLoaderPath).href, + ); } return { command: nodeCommand, - args: [...pnpArgs, binary.path, ...args], + args: nodeArgs, options: { cwd: binary.cwd, env: serverEnv, diff --git a/tests/unit/detectVitePlus.spec.ts b/tests/unit/detectVitePlus.spec.ts index bfd8f38f..b18373a8 100644 --- a/tests/unit/detectVitePlus.spec.ts +++ b/tests/unit/detectVitePlus.spec.ts @@ -16,7 +16,10 @@ suite("detectVitePlusProject", () => { return target; } - function pkg(relative = "", value: object = { devDependencies: { "vite-plus": "latest" } }) { + function pkg( + relative = "", + value: object = { devDependencies: { "vite-plus": "latest" } }, + ): string { return file(path.join(relative, "package.json"), JSON.stringify(value)); } diff --git a/tests/unit/extension.spec.ts b/tests/unit/extension.spec.ts index e0739f88..89393328 100644 --- a/tests/unit/extension.spec.ts +++ b/tests/unit/extension.spec.ts @@ -56,10 +56,10 @@ suite("navigation during extension activation", () => { const ready = new Promise((resolve) => { entered = resolve; }); - const wait = async () => { + async function wait(): Promise { if (++waiting === 2) entered(); await gate; - }; + } const selected = new Map(); for (const [Tool, name] of [ [Linter, "lint"], diff --git a/tests/unit/getShellEnv.spec.ts b/tests/unit/getShellEnv.spec.ts index d3650d0f..70bcff9e 100644 --- a/tests/unit/getShellEnv.spec.ts +++ b/tests/unit/getShellEnv.spec.ts @@ -12,9 +12,7 @@ let moduleId = 0; async function loadFreshGetShellEnvModule(): Promise { // append a query parameter to force a fresh import of the module to reset the cachedEnv variable - const module = await import(`../../client/getShellEnv.ts?testModule=${moduleId++}`); - - return module; + return import(`../../client/getShellEnv.ts?testModule=${moduleId++}`); } function createMockShellScript(dir: string, name: string, scriptBody: string): string { diff --git a/tests/unit/vitePlus.spec.ts b/tests/unit/vitePlus.spec.ts index 59221175..2070c0b7 100644 --- a/tests/unit/vitePlus.spec.ts +++ b/tests/unit/vitePlus.spec.ts @@ -4,6 +4,7 @@ import * as path from "node:path"; import { mock } from "node:test"; import { commands, ConfigurationTarget, Uri, window, workspace } from "vscode"; import { ConfigService } from "../../client/ConfigService"; +import type { BinarySearchResult } from "../../client/findBinary"; import { runExecutable } from "../../client/tools/lsp_helper"; import { WORKSPACE_FOLDER, WORKSPACE_SECOND_FOLDER } from "../test-helpers"; import { mockProcessEnv, mockProcessPlatform } from "../processMocks"; @@ -32,11 +33,11 @@ suite("Vite+ server selection", () => { return target; } - function declare(dir = root) { + function declare(dir = root): void { file("package.json", JSON.stringify({ devDependencies: { "vite-plus": "latest" } }), dir); } - function shim(dir = root) { + function shim(dir = root): string { return file( path.join("node_modules/.bin", process.platform === "win32" ? "vp.cmd" : "vp"), "", @@ -44,7 +45,7 @@ suite("Vite+ server selection", () => { ); } - function standaloneTools() { + function standaloneTools(): Record<"oxlint" | "oxfmt", BinarySearchResult> { const binaries = { oxlint: { path: file("tools/oxlint.js"), loader: "node" as const }, oxfmt: { path: file("tools/oxfmt.js"), loader: "node" as const }, @@ -57,14 +58,14 @@ suite("Vite+ server selection", () => { return binaries; } - async function sources(lint: string, fmt: string) { + async function sources(lint: string, fmt: string): Promise { await Promise.all([ conf.update("lint.binarySource", lint, ConfigurationTarget.WorkspaceFolder), conf.update("fmt.binarySource", fmt, ConfigurationTarget.WorkspaceFolder), ]); } - async function open(dir = root) { + async function open(dir = root): Promise { await window.showTextDocument(Uri.file(file("index.txt", "", dir))); } From 2f33a2f2e5a99cb0c6063d34a5163b065fb332d1 Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 14 Sep 2026 00:23:25 +0800 Subject: [PATCH 14/15] fix: preserve unrecognized Vite+ wrappers --- client/resolveVitePlusNodeEntry.ts | 22 +++++++--------- tests/unit/lsp_helper.spec.ts | 42 +++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/client/resolveVitePlusNodeEntry.ts b/client/resolveVitePlusNodeEntry.ts index 9b01b456..3899b8ab 100644 --- a/client/resolveVitePlusNodeEntry.ts +++ b/client/resolveVitePlusNodeEntry.ts @@ -34,22 +34,18 @@ export function resolveVitePlusNodeEntry(vpPath: string): string | undefined { const target = isCmd ? /"((?:%(?:~dp0|dp0%)[\\/]|[a-zA-Z]:[\\/]|\\\\)[^"\r\n]+)"[ \t]+%\*/.exec(shim) : /"\$basedir\/([^"\r\n]+)"[ \t]+"\$@"/.exec(shim); + if (!target) { + return undefined; + } + // pnpm shell shims can follow symlinks before computing their base directory. const shimPath = !isCmd && lstatSync(vpPath).isSymbolicLink() ? realpathSync(vpPath) : vpPath; - const binDir = path.dirname(shimPath); - let nodeEntry: string | undefined; - if (target) { - const entryPath = isCmd - ? target[1].replace(/^%(?:~dp0|dp0%)[\\/]/, "").replaceAll("\\", path.sep) - : target[1]; - nodeEntry = path.resolve(binDir, entryPath); - } else if (path.basename(binDir) === ".bin") { - nodeEntry = path.resolve(binDir, "..", "vite-plus", "bin", "vp"); - } else if (isCmd) { - nodeEntry = path.join(binDir, "node_modules", "vite-plus", "bin", "vp"); - } + const entryPath = isCmd + ? target[1].replace(/^%(?:~dp0|dp0%)[\\/]/, "").replaceAll("\\", path.sep) + : target[1]; + const nodeEntry = path.resolve(path.dirname(shimPath), entryPath); - if (nodeEntry && nodeShebangPattern.test(readShebang(nodeEntry))) { + if (nodeShebangPattern.test(readShebang(nodeEntry))) { return nodeEntry; } } catch { diff --git a/tests/unit/lsp_helper.spec.ts b/tests/unit/lsp_helper.spec.ts index 64be1936..5354147e 100644 --- a/tests/unit/lsp_helper.spec.ts +++ b/tests/unit/lsp_helper.spec.ts @@ -106,14 +106,18 @@ process.stderr.write(child.stderr); process.exit(child.status ?? 1); `, ); - if (isGlobalPnpm) { + if (shimType === "npm") { + symlinkSync(nodeEntry, shim); + } else { const relativeEntry = path.relative(binDir, nodeEntry); // pnpm shims can contain long NODE_PATH setup before the launch command. - const padding = " ".repeat(512); + const padding = isGlobalPnpm ? " ".repeat(512) : ""; const script = isCmd ? `@SETLOCAL\r\n@REM ${padding}\r\nnode "%~dp0\\${relativeEntry.replaceAll(path.sep, "\\")}" %*\r\n` - : `#!/bin/sh\n# ${padding}\nexec node "$basedir/${relativeEntry.replaceAll(path.sep, "/")}" "$@"\n`; + : `#!/bin/sh\nbasedir=$(dirname "$0")\n# ${padding}\nexec node "$basedir/${relativeEntry.replaceAll(path.sep, "/")}" "$@"\n`; writeFileSync(shim, script); + } + if (isGlobalPnpm) { // The recorded target must win over an unrelated adjacent installation. const adjacentEntry = path.join(binDir, "node_modules", "vite-plus", "bin", "vp"); mkdirSync(path.dirname(adjacentEntry), { recursive: true }); @@ -124,13 +128,6 @@ process.exit(child.status ?? 1); symlinkSync(shim, alias); shim = alias; } - } else if (shimType === "npm") { - symlinkSync(nodeEntry, shim); - } else { - writeFileSync( - shim, - shimType === "pnpm" ? '#!/bin/sh\nexec node "$@"\n' : "@echo off\r\nnode %*\r\n", - ); } process.env.PATH = path.join(tempDir, "no-system-node"); @@ -156,6 +153,31 @@ process.exit(child.status ?? 1); } } + test("keeps an unrecognized shell wrapper despite an adjacent Vite+ installation", async function () { + if (originalPlatform === "win32") this.skip(); + const shim = path.join(tempDir, "node_modules", ".bin", "vp"); + const adjacentEntry = path.join(tempDir, "node_modules", "vite-plus", "bin", "vp"); + mkdirSync(path.dirname(shim), { recursive: true }); + mkdirSync(path.dirname(adjacentEntry), { recursive: true }); + writeFileSync(shim, '#!/bin/sh\nprintf "%s\\n" "custom wrapper" "$@"\n', { mode: 0o755 }); + writeFileSync(adjacentEntry, "#!/usr/bin/env node\nthrow new Error('wrong entry');\n"); + process.env.PATH = path.join(tempDir, "no-system-node"); + + const result = await runExecutable( + { path: shim, loader: "native", vitePlus: "lint", cwd: tempDir }, + true, + ); + strictEqual(result.command, shim); + deepStrictEqual(result.args, ["lint", "--lsp"]); + const child = spawnSync(result.command, result.args, { + ...result.options, + encoding: "utf8", + timeout: 5000, + }); + strictEqual(child.status, 0, child.stderr || child.error?.message); + strictEqual(child.stdout, "custom wrapper\nlint\n--lsp\n"); + }); + test("keeps a global shim when its recorded target is not a Node entry", async () => { const shim = path.join(tempDir, "vp.cmd"); writeFileSync(shim, '@echo off\r\n"%~dp0\\native-vp" %*\r\n'); From c3b69224df749a428b7db09c456a21a82d546bff Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 14 Sep 2026 00:41:37 +0800 Subject: [PATCH 15/15] refactor: simplify linter startup condition --- client/tools/linter.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/tools/linter.ts b/client/tools/linter.ts index 4a7e7abb..01751c9a 100644 --- a/client/tools/linter.ts +++ b/client/tools/linter.ts @@ -245,11 +245,11 @@ export default class LinterTool implements ToolInterface { return; } + const requiresConfig = !binary.vitePlus && this.configService.vsCodeConfig.requireConfig; this.allowedToStartServer = - !binary.vitePlus && this.configService.vsCodeConfig.requireConfig - ? (await workspace.findFiles(oxlintConfigDefaultFilePattern, "**/node_modules/**", 1)) - .length > 0 - : true; + !requiresConfig || + (await workspace.findFiles(oxlintConfigDefaultFilePattern, "**/node_modules/**", 1)).length > + 0; const run: Executable = await runExecutable( binary,