From 8fa4abec96bcca9911e7470412c7b29dc342f97f Mon Sep 17 00:00:00 2001 From: Pranay Prakash Date: Sat, 16 May 2026 17:47:34 -0700 Subject: [PATCH] fix(server): drop fs dependency (#152) Squashed: includes the original fs-removal commit and the follow-up review fixes (TextDecoder for browser compat, bounded read concurrency, race-guard during async file reads, virtualWorkspaces.supported reverted to false until documentSelector is broadened, engines.vscode bumped to the workspace.fs minimum). Co-Authored-By: Claude Opus 4.7 (1M context) --- client/src/extension.ts | 223 ++++++++++++++++++++----------- client/tsconfig.json | 2 +- package.json | 4 +- server/src/server.ts | 24 ++-- server/src/types.ts | 20 +-- tests/src/findDefinition.test.ts | 117 ++++++++++++++++ tests/tsconfig.json | 2 +- 7 files changed, 288 insertions(+), 104 deletions(-) diff --git a/client/src/extension.ts b/client/src/extension.ts index 177391e..44247fd 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -29,6 +29,33 @@ const SUPPORTED_EXTENSION_REGEX = /\.(css|scss|less)$/; let defaultClient: LanguageClient; const clients: Map = new Map(); +// Tracks folders whose LanguageClient is mid-creation. The stylesheet read +// step is asynchronous, so we have to claim the folder before awaiting any +// I/O — otherwise a second matching document opened in the same folder +// during that window would spawn a duplicate server (see #154). +const pendingClientFolders: Set = new Set(); + +const READ_CONCURRENCY = 16; +const utf8Decoder = new TextDecoder("utf-8"); + +async function mapWithConcurrency( + items: T[], + limit: number, + fn: (item: T) => Promise +): Promise { + const results: R[] = new Array(items.length); + let next = 0; + async function worker() { + while (next < items.length) { + const i = next++; + results[i] = await fn(items[i]); + } + } + await Promise.all( + Array.from({ length: Math.min(limit, items.length) }, () => worker()) + ); + return results; +} let _sortedWorkspaceFolders: string[] | undefined; function sortedWorkspaceFolders(): string[] { @@ -106,6 +133,105 @@ export function activate(context: ExtensionContext): void { ) as boolean; const respectGitignore: boolean = config.get("respectGitignore") as boolean; + const documentSelector = [ + ...SUPPORTED_EXTENSIONS.map((language) => ({ scheme: "file", language })), + ...SUPPORTED_EXTENSIONS.map((language) => ({ + scheme: "untitled", + language, + })), + ...peekFromLanguages.map((language) => ({ scheme: "file", language })), + ...peekFromLanguages.map((language) => ({ scheme: "untitled", language })), + ]; + + type Stylesheet = { uri: string; languageId: string; text: string }; + + async function readStylesheet(u: Uri): Promise { + try { + const bytes = await Workspace.fs.readFile(u); + return { + uri: u.toString(), + languageId: u.path.split(".").pop() || "", + text: utf8Decoder.decode(bytes), + }; + } catch (err) { + sendTelemetryErrorEvent("readStylesheet", { + context: "client", + method: "readStylesheet", + uri: u.toString(), + error: err instanceof Error ? err.message : String(err), + }); + return null; + } + } + + async function startClientForFolder( + folder: WorkspaceFolder, + folderKey: string + ): Promise { + try { + // Discover stylesheets and read their contents via vscode.workspace.fs + // (works in virtual/web workspaces). Reads are capped to READ_CONCURRENCY + // so workspaces with thousands of files don't blow up memory at startup. + const gitignoreGlobs = respectGitignore + ? await readGitignoreGlobs(folder.uri) + : []; + const mergedExcludes = Array.from( + new Set([...(peekToExclude || []), ...gitignoreGlobs]) + ); + const file_searches = await Workspace.findFiles( + `{${(peekToInclude || []).join(",")}}`, + `{${mergedExcludes.join(",")}}` + ); + const stylesheets: Stylesheet[] = ( + await mapWithConcurrency( + file_searches, + READ_CONCURRENCY, + readStylesheet + ) + ).filter((s): s is Stylesheet => s !== null); + + // The folder may have been removed while we were reading files. + if (!pendingClientFolders.has(folderKey)) return; + + const debugOptions = { + execArgv: ["--nolazy", `--inspect=${6011 + clients.size}`], + }; + const serverOptions = { + run: { module, transport: TransportKind.ipc }, + debug: { + module, + transport: TransportKind.ipc, + options: debugOptions, + }, + }; + const clientOptions: LanguageClientOptions = { + documentSelector, + diagnosticCollectionName: "css-peek", + synchronize: { + configurationSection: "cssPeek", + }, + initializationOptions: { + stylesheets, + peekFromLanguages, + peekToLinkedOnly, + }, + workspaceFolder: folder, + outputChannel, + }; + const client = new LanguageClient( + "css-peek", + "CSS Peek", + serverOptions, + clientOptions + ); + client.registerProposedFeatures(); + client.start(); + clients.set(folderKey, client); + } finally { + pendingClientFolders.delete(folderKey); + } + } + function didOpenTextDocument(document: TextDocument): void { try { if ( @@ -116,25 +242,6 @@ export function activate(context: ExtensionContext): void { return; } - const documentSelector = [ - ...SUPPORTED_EXTENSIONS.map((language) => ({ - scheme: "file", - language, - })), - ...SUPPORTED_EXTENSIONS.map((language) => ({ - scheme: "untitled", - language, - })), - ...peekFromLanguages.map((language) => ({ - scheme: "file", - language, - })), - ...peekFromLanguages.map((language) => ({ - scheme: "untitled", - language, - })), - ]; - const uri = document.uri; const telemetryData = { context: "client", @@ -192,66 +299,15 @@ export function activate(context: ExtensionContext): void { folder = getOuterMostWorkspaceFolder(folder); telemetryData.workspaceFolder = folder; - if (!clients.has(folder.uri.toString())) { - const gitignorePromise = respectGitignore - ? readGitignoreGlobs(folder.uri) - : Promise.resolve([] as string[]); - - gitignorePromise - .then((gitignoreGlobs) => { - const mergedExcludes = Array.from( - new Set([...(peekToExclude || []), ...gitignoreGlobs]) - ); - return Workspace.findFiles( - `{${(peekToInclude || []).join(",")}}`, - `{${mergedExcludes.join(",")}}` - ); - }) - .then((file_searches) => { - const potentialFiles: Uri[] = file_searches.filter( - (uri: Uri) => uri.scheme === "file" - ); - - const debugOptions = { - execArgv: ["--nolazy", `--inspect=${6011 + clients.size}`], - }; - const serverOptions = { - run: { module, transport: TransportKind.ipc }, - debug: { - module, - transport: TransportKind.ipc, - options: debugOptions, - }, - }; - const clientOptions: LanguageClientOptions = { - documentSelector, - diagnosticCollectionName: "css-peek", - synchronize: { - configurationSection: "cssPeek", - }, - initializationOptions: { - stylesheets: potentialFiles.map((u) => ({ - uri: u.toString(), - // TODO: don't rely on fsPath in a virtual workspace - // https://github.com/microsoft/vscode/wiki/Virtual-Workspaces - fsPath: u.fsPath, - })), - peekFromLanguages, - peekToLinkedOnly, - }, - workspaceFolder: folder, - outputChannel, - }; - const client = new LanguageClient( - "css-peek", - "CSS Peek", - serverOptions, - clientOptions - ); - client.registerProposedFeatures(); - client.start(); - clients.set(folder.uri.toString(), client); - }); + const folderKey = folder.uri.toString(); + if (!clients.has(folderKey) && !pendingClientFolders.has(folderKey)) { + // Claim the folder synchronously before any awaits so concurrent + // didOpenTextDocument calls don't race to start a second server. + pendingClientFolders.add(folderKey); + startClientForFolder(folder, folderKey).catch(() => { + // Errors are already reported via telemetry inside startClientForFolder. + // The catch is just to satisfy the unhandled-rejection contract. + }); } sendTelemetryEvent("Document Opened", telemetryData); } catch (e) { @@ -266,7 +322,12 @@ export function activate(context: ExtensionContext): void { Workspace.textDocuments.forEach(didOpenTextDocument); Workspace.onDidChangeWorkspaceFolders((event) => { for (const folder of event.removed) { - const client = clients.get(folder.uri.toString()); + const folderKey = folder.uri.toString(); + // If the folder was still mid-spawn, drop the pending claim so the + // continuation in startClientForFolder bails out before constructing a + // client. + pendingClientFolders.delete(folderKey); + const client = clients.get(folderKey); if (client) { sendTelemetryEvent("Workspace Folder Closed", { context: "client", @@ -278,7 +339,7 @@ export function activate(context: ExtensionContext): void { uriScheme: folder.uri.scheme, }); - clients.delete(folder.uri.toString()); + clients.delete(folderKey); client.stop(); } } diff --git a/client/tsconfig.json b/client/tsconfig.json index fb94611..a8e599d 100644 --- a/client/tsconfig.json +++ b/client/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "module": "commonjs", "target": "es2019", - "lib": ["ES2019"], + "lib": ["ES2019", "DOM"], "outDir": "out", "rootDir": "src", "sourceMap": true, diff --git a/package.json b/package.json index 309b347..0ad12a3 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "workspace symbol" ], "engines": { - "vscode": "^1.33.0" + "vscode": "^1.37.0" }, "main": "./client/out/extension", "browser": "./client/out/extension", @@ -157,7 +157,7 @@ }, "virtualWorkspaces": { "supported": false, - "description": "The extension currently relies on the `fs` module but it should be easy to change this. Please make a PR to help." + "description": "The server no longer touches the host filesystem, but the client's documentSelector still gates on the `file` and `untitled` URI schemes. Expanding the selector to cover virtual schemes (e.g. `vscode-vfs`) is a separate change." } }, "scripts": { diff --git a/server/src/server.ts b/server/src/server.ts index 3129032..3d00c10 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -1,6 +1,5 @@ "use strict"; -import fs = require("fs"); import { minimatch } from "minimatch"; import * as path from "path"; import { @@ -15,7 +14,7 @@ import { DidChangeConfigurationNotification, } from "vscode-languageserver/node"; import { TextDocument } from "vscode-languageserver-textdocument"; -import { Uri, StylesheetMap, Selector } from "./types"; +import { Stylesheet, StylesheetMap, Selector } from "./types"; import findSelector from "./core/findSelector"; import { @@ -197,7 +196,7 @@ documents.onDidClose((e) => { }); function setupInitialStyleMap(params: InitializeParams) { - const styleFiles = params.initializationOptions.stylesheets; + const styleFiles: Stylesheet[] = params.initializationOptions.stylesheets; connection.console.log( `[Server(${process.pid}) ${path.basename( @@ -205,14 +204,17 @@ function setupInitialStyleMap(params: InitializeParams) { )}/] Number of style sheets - ${styleFiles.length}` ); - styleFiles.forEach((fileUri: Uri) => { - const languageId = fileUri.fsPath.split(".").slice(-1)[0]; - // TODO: this is bad. stop using the file system directly. Instead, use the VSCode - // fs API to support the virutal filesystem - // https://github.com/microsoft/vscode/wiki/Virtual-Workspaces - const text = fs.readFileSync(fileUri.fsPath, "utf8"); - const document = TextDocument.create(fileUri.uri, languageId, 1, text); - styleSheets[fileUri.uri] = { + // Stylesheet contents are read on the client (via `vscode.workspace.fs`) + // and shipped over LSP so the server never touches the host file system. + // This keeps the server compatible with virtual workspaces and the web build. + styleFiles.forEach((file) => { + const document = TextDocument.create( + file.uri, + file.languageId, + 1, + file.text + ); + styleSheets[file.uri] = { document, }; }); diff --git a/server/src/types.ts b/server/src/types.ts index 42db22c..e177369 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -8,21 +8,25 @@ export type StylesheetMap = { }; }; -// Based off the `vscode` `Uri` namespace -export type Uri = { +// A stylesheet payload passed from the client during LSP initialization. +// The client reads file contents via `vscode.workspace.fs.readFile` +// (which works in virtual/web workspaces) and ships them to the server, so +// the server never needs to touch the host file system itself. +export type Stylesheet = { /** * The actual Uri string representation */ readonly uri: string; /** - * The string representing the corresponding file system path of this Uri. - * - * Will handle UNC paths and normalize windows drive letters to lower-case. Also - * uses the platform specific path separator. Will *not* validate the path for - * invalid characters and semantics. Will *not* look at the scheme of this Uri. + * The language id of the stylesheet (e.g. "css", "scss", "less"). */ - readonly fsPath: string; + readonly languageId: string; + + /** + * The full text content of the stylesheet. + */ + readonly text: string; }; export type Selector = { attribute: string; value: string }; diff --git a/tests/src/findDefinition.test.ts b/tests/src/findDefinition.test.ts index 71b80dc..367d2fb 100644 --- a/tests/src/findDefinition.test.ts +++ b/tests/src/findDefinition.test.ts @@ -32,6 +32,73 @@ async function loadStylesheets(files: string[]): Promise { return map; } +/** + * Mirrors the production code path: the client reads file contents via + * `vscode.workspace.fs.readFile` (which works in virtual/web workspaces), + * ships `{uri, languageId, text}` tuples to the server, and the server + * materializes them into a `StylesheetMap` without touching `fs`. + * + * Uses `TextDecoder` (browser-safe) rather than `Buffer` to match the + * production code, which targets both Node and web extension hosts. + */ +const utf8Decoder = new TextDecoder("utf-8"); +async function loadStylesheetsViaWorkspaceFs( + files: string[] +): Promise { + const map: StylesheetMap = {}; + for (const file of files) { + const uri = vscode.Uri.joinPath( + vscode.workspace.workspaceFolders![0].uri, + file + ); + const bytes = await vscode.workspace.fs.readFile(uri); + const text = utf8Decoder.decode(bytes); + const languageId = file.split(".").pop() || ""; + const serverDoc = ServerTextDocument.create( + uri.toString(), + languageId, + 1, + text + ); + map[serverDoc.uri] = { document: serverDoc }; + } + return map; +} + +/** + * Constructs a StylesheetMap whose entries have non-`file` URIs. This is the + * shape `setupInitialStyleMap` will encounter when running against a virtual + * workspace (e.g. GitHub Remote Repositories' `vscode-vfs://github/...`), + * once the client's documentSelector is broadened to those schemes. The + * server itself is scheme-agnostic — it only ever indexes by the URI string + * and reads text from the in-memory TextDocument. + */ +function buildVirtualStylesheetMap(): StylesheetMap { + const map: StylesheetMap = {}; + const entries: Array<{ uri: string; languageId: string; text: string }> = [ + { + uri: "vscode-vfs://github/example/repo/styles.css", + languageId: "css", + text: ".virtual-only { color: red; }\n#virtual-id { color: blue; }\n", + }, + { + uri: "vscode-test-web:///workspace/extra.scss", + languageId: "scss", + text: ".virtual-only { font-weight: bold; }\n", + }, + ]; + for (const entry of entries) { + const doc = ServerTextDocument.create( + entry.uri, + entry.languageId, + 1, + entry.text + ); + map[doc.uri] = { document: doc }; + } + return map; +} + suite("findDefinition", () => { create(console as any); let map: StylesheetMap; @@ -83,6 +150,56 @@ suite("findDefinition", () => { assert.deepStrictEqual(lines, [0, 16, 16, 19]); }); + test("stylesheets loaded via vscode.workspace.fs are discoverable", async () => { + // Exercises the same code path the extension client now uses to ship + // stylesheet contents to the server — no `fs` module involved. + const fsMap = await loadStylesheetsViaWorkspaceFs([ + "stylesheet.css", + "example.less", + "example.scss", + "my_style.scss", + "extendFailureCase.less", + ]); + + const classDefs = findDefinition( + { attribute: "class", value: "test" }, + fsMap + ); + assert.strictEqual(classDefs.length, 3); + + const idDefs = findDefinition({ attribute: "id", value: "test-2" }, fsMap); + const idFiles = idDefs + .map((d) => vscode.Uri.parse(d.uri).path.split("/").pop()) + .sort(); + assert.deepStrictEqual( + idFiles, + ["example.less", "example.scss", "stylesheet.css"].sort() + ); + }); + + test("resolves selectors across non-file URI schemes", () => { + // Verifies the server's scheme-agnosticism: once the client ships + // stylesheet payloads keyed by `vscode-vfs://` / `vscode-test-web://` + // URIs (virtual workspaces), findDefinition still resolves them and + // returns Locations on those schemes — no `file:` assumption anywhere. + const virtualMap = buildVirtualStylesheetMap(); + + const classDefs = findDefinition( + { attribute: "class", value: "virtual-only" }, + virtualMap + ); + assert.strictEqual(classDefs.length, 2); + const schemes = classDefs.map((d) => vscode.Uri.parse(d.uri).scheme).sort(); + assert.deepStrictEqual(schemes, ["vscode-test-web", "vscode-vfs"]); + + const idDefs = findDefinition( + { attribute: "id", value: "virtual-id" }, + virtualMap + ); + assert.strictEqual(idDefs.length, 1); + assert.strictEqual(vscode.Uri.parse(idDefs[0].uri).scheme, "vscode-vfs"); + }); + test("peekVariables toggle filters Variable-kind symbols", () => { // Pre-populate the symbol cache with a Variable symbol whose name would // otherwise match the selector. Isolates the filter from the language diff --git a/tests/tsconfig.json b/tests/tsconfig.json index fed07c6..ba95222 100644 --- a/tests/tsconfig.json +++ b/tests/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "module": "commonjs", "target": "es2019", - "lib": ["ES2019"], + "lib": ["ES2019", "DOM"], "outDir": "out", "rootDir": "src", "sourceMap": true,