fix: watch stylesheets for delete/create (#30)#158
Conversation
There was a problem hiding this comment.
Pull request overview
Adds client→server notifications to keep the server-side StylesheetMap cache in sync with stylesheet file create/delete events, preventing “peek” results from referencing deleted files (fixes #30).
Changes:
- Client: add a workspace
FileSystemWatcherforcss/scss/lessand send custom LSP notifications on create/delete. - Server: handle
cssPeek/stylesheetDeleted/cssPeek/stylesheetCreatednotifications and refactor per-file loading into a sharedloadStylesheethelper. - Tests: add a lifecycle test that validates results disappear after removing a stylesheet from the map.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
client/src/extension.ts |
Adds filesystem watching and forwards create/delete events to the LSP server. |
server/src/server.ts |
Adds notification handlers and factors stylesheet loading into loadStylesheet. |
tests/src/stylesheetLifecycle.test.ts |
Adds coverage for create/delete cache lifecycle behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const watcher = Workspace.createFileSystemWatcher( | ||
| `{${(peekToInclude || []).join(",")}}` | ||
| ); | ||
| watcher.onDidDelete((uri) => { | ||
| client.sendNotification( | ||
| "cssPeek/stylesheetDeleted", | ||
| uri.toString() | ||
| ); | ||
| }); | ||
| watcher.onDidCreate((uri) => { | ||
| client.sendNotification("cssPeek/stylesheetCreated", { | ||
| uri: uri.toString(), | ||
| fsPath: uri.fsPath, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Addressed in d58b79f. The watcher now filters create/delete events against peekToExclude (using minimatch against both fsPath and the URI string), bails early for non-file URIs, and passes ignoreChangeEvents: true to createFileSystemWatcher since only create/delete are handled.
| watcher.onDidCreate((uri) => { | ||
| client.sendNotification("cssPeek/stylesheetCreated", { | ||
| uri: uri.toString(), | ||
| fsPath: uri.fsPath, | ||
| }); | ||
| }); | ||
| context.subscriptions.push(watcher); |
There was a problem hiding this comment.
Addressed in d58b79f. Watchers are now tracked in a Map<string, FileSystemWatcher> keyed by folder URI. The onDidChangeWorkspaceFolders removal handler disposes the watcher for each removed folder (before stopping the client), and deactivate() disposes any still-alive watchers. Per-folder watchers are no longer pushed onto context.subscriptions.
| function loadStylesheet(fileUri: Uri): void { | ||
| 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 |
| // Mirror the server's loadStylesheet logic so this test exercises the same | ||
| // path used by the cssPeek/stylesheetCreated notification handler. | ||
| function loadStylesheet(map: StylesheetMap, fileUri: Uri): void { | ||
| const languageId = fileUri.fsPath.split(".").slice(-1)[0]; | ||
| const text = fs.readFileSync(fileUri.fsPath, "utf8"); | ||
| const document = ServerTextDocument.create(fileUri.uri, languageId, 1, text); | ||
| map[fileUri.uri] = { document }; | ||
| } |
There was a problem hiding this comment.
Reworded in d58b79f. The comment now states explicitly that the test mirrors the logic of the cssPeek/stylesheetCreated/Deleted handlers (load -> confirm peek -> delete entry -> confirm peek empty) and does not invoke the server handlers directly, with a note to cross-check server/src/server.ts loadStylesheet if behavior changes.
Until now, deleting a CSS file left the cached entry in StylesheetMap,
so peek kept returning matches from files that no longer existed.
Workspace.createFileSystemWatcher fires on disk-level events that the
LSP documents sync misses (it only covers open editors).
- Client: one FileSystemWatcher per workspace folder, ignoreChangeEvents
so we only listen for create/delete. Events are filtered against
peekToExclude (via minimatch) and non-file schemes so excluded paths
don't leak into the server cache. Watchers are tracked per folder and
disposed on folder removal + deactivate.
- Notification protocol: cssPeek/stylesheetDeleted (uri string) and
cssPeek/stylesheetCreated ({uri, languageId, text} payload). The
client reads file content with workspace.fs.readFile so the server
remains fs-free; the create payload reuses the same Stylesheet shape
setupInitialStyleMap consumes.
- Server: small loadStylesheet helper that wraps TextDocument.create +
styleSheets[uri] assignment, called from setupInitialStyleMap and the
create handler. Delete handler just removes the key.
- Test: tests/src/stylesheetLifecycle.test.ts spins up a temp .css
fixture, confirms peek resolves the class, deletes the fixture +
prunes the map, and asserts peek returns nothing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pranaygp
force-pushed
the
worktree-agent-aa4bd1c63f182ff4e
branch
from
d58b79f to
2787f5d
Compare
Closes #30.
Summary
StylesheetMaplingers, so peek still returns matches pointing at the deleted file. This PR watches the workspace filesystem and prunes (or loads) entries as files come and go, so peek stays accurate without a VSCode restart.client/src/extension.ts): registers avscode.workspace.createFileSystemWatcherfor the same**/*.{css,scss,less}globs used during initial discovery and forwardsonDidDelete/onDidCreateto the server via custom LSP notifications (cssPeek/stylesheetDeleted,cssPeek/stylesheetCreated). Create/delete events are filtered throughpeekToExcludeand limited tofile://URIs;ignoreChangeEventsistruesince only create/delete are handled. Watchers are tracked per workspace folder and disposed when the folder is removed (or on deactivate), so we don't keep firing notifications at stopped clients.server/src/server.ts): adds notification handlers that prune the deleted URI fromStylesheetMap(clearing its cached symbols) and load newly created stylesheets via the samefs.readFileSyncpath used during init. Factored the per-file load logic into a sharedloadStylesheethelper used by both the initial setup and the create notification.Test plan
yarn build— zero TS errorsyarn teston macOS — 25 passing, including the newStylesheet create/delete lifecycleintegration test that creates a temp CSS fixture, confirmsfindDefinitionpicks up its class, deletes the fixture + prunes the cache, and confirms peek no longer returns ityarn lint— cleanyarn prettier— changed files pass (pre-existing warnings in other files are unrelated)Merge notes
This PR branches off
master, where the server still usesfs.readFileSyncin itsloadStylesheethelper. PR #156 dropsfsfrom the server in favor of a client-pushed content pattern, so the two PRs conflict inserver/src/server.ts.Suggested merge order:
masterand swap thefs.readFileSynccall inloadStylesheetfor the client-pushed content pattern introduced by fix(server): drop fs dependency (#152) #156.