fix: handle issues with svelte file watching (#2421)

Fix the neovim file-watching issue mentioned in Discord by checking if a directory actually exists before watching it.
Also fixes #2419 through several measures:
- don't close a document when it was changed
- fix the retrieval of the document (pathToUrl) was missing

Author: jasonlyu123

packages/language-server/src/lib/documents/DocumentManager.ts

@@ -47,7 +47,8 @@ export class DocumentManager {
         let document: Document;
         if (this.documents.has(textDocument.uri)) {
             document = this.documents.get(textDocument.uri)!;
-            document.openedByClient = openedByClient;
+            // open state should only be updated when the document is closed
+            document.openedByClient ||= openedByClient;
             document.setText(textDocument.text);
         } else {
             document = this.createDocument(textDocument);

packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts

@@ -204,11 +204,21 @@ export class LSAndTSDocResolver {
      * Updates snapshot path in all existing ts services and retrieves snapshot
      */
     async updateSnapshotPath(oldPath: string, newPath: string): Promise<void> {
+        const document = this.docManager.get(pathToUrl(oldPath));
+        const isOpenedInClient = document?.openedByClient;
         for (const snapshot of this.globalSnapshotsManager.getByPrefix(oldPath)) {
             await this.deleteSnapshot(snapshot.filePath);
         }
-        // This may not be a file but a directory, still try
-        await this.getSnapshot(newPath);
+
+        if (isOpenedInClient) {
+            this.docManager.openClientDocument({
+                uri: pathToUrl(newPath),
+                text: document!.getText()
+            });
+        } else {
+            // This may not be a file but a directory, still try
+            await this.getSnapshot(newPath);
+        }
     }
 
     /**

packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts

@@ -35,7 +35,7 @@ import {
     mapSymbolInformationToOriginal
 } from '../../lib/documents';
 import { LSConfigManager, LSTypescriptConfig } from '../../ls-config';
-import { isNotNullOrUndefined, isZeroLengthRange } from '../../utils';
+import { isNotNullOrUndefined, isZeroLengthRange, pathToUrl } from '../../utils';
 import {
     AppCompletionItem,
     AppCompletionList,
@@ -523,7 +523,7 @@ export class TypeScriptPlugin
 
             const isSvelteFile = isSvelteFilePath(fileName);
             const isClientSvelteFile =
-                isSvelteFile && this.documentManager.get(fileName)?.openedByClient;
+                isSvelteFile && this.documentManager.get(pathToUrl(fileName))?.openedByClient;
 
             if (changeType === FileChangeType.Deleted) {
                 if (!isClientSvelteFile) {

packages/language-server/src/plugins/typescript/service.ts

@@ -353,11 +353,15 @@ async function createLanguageService(
             return;
         }
 
+        const canonicalWorkspacePath = getCanonicalFileName(workspacePath);
         const patterns: RelativePattern[] = [];
 
         Object.entries(wildcardDirectories).forEach(([dir, flags]) => {
-            // already watched
-            if (getCanonicalFileName(dir).startsWith(workspacePath)) {
+            if (
+                // already watched
+                getCanonicalFileName(dir).startsWith(canonicalWorkspacePath) ||
+                !tsSystem.directoryExists(dir)
+            ) {
                 return;
             }
             patterns.push({

packages/language-server/src/server.ts

@@ -104,6 +104,8 @@ export function startServer(options?: LSOptions) {
         pendingWatchPatterns = patterns;
     };
 
+    // Include Svelte files to better deal with scenarios such as switching git branches
+    // where files that are not opened in the client could change
     const nonRecursiveWatchPattern = '*.{ts,js,mts,mjs,cjs,cts,json,svelte}';
     const recursiveWatchPattern = '**/' + nonRecursiveWatchPattern;
 
@@ -329,6 +331,8 @@ export function startServer(options?: LSOptions) {
         connection?.client.register(DidChangeWatchedFilesNotification.type, {
             watchers: [
                 {
+                    // Editors have exlude configs, such as VSCode with `files.watcherExclude`,
+                    // which means it's safe to watch recursively here
                     globPattern: recursiveWatchPattern
                 }
             ]

packages/language-server/test/plugins/typescript/TypescriptPlugin.test.ts

@@ -57,7 +57,7 @@ describe('TypescriptPlugin', function () {
             docManager
         );
         docManager.openClientDocument(<any>'some doc');
-        return { plugin, document, lsAndTsDocResolver };
+        return { plugin, document, lsAndTsDocResolver, docManager };
     }
 
     it('provides document symbols', async () => {
@@ -618,7 +618,7 @@ describe('TypescriptPlugin', function () {
     });
 
     const setupForOnWatchedFileChanges = async () => {
-        const { plugin, document, lsAndTsDocResolver } = setup('empty.svelte');
+        const { plugin, document, lsAndTsDocResolver, docManager } = setup('empty.svelte');
         const targetSvelteFile = document.getFilePath()!;
         const snapshotManager = (await lsAndTsDocResolver.getTSService(targetSvelteFile))
             .snapshotManager;
@@ -627,7 +627,8 @@ describe('TypescriptPlugin', function () {
             snapshotManager,
             plugin,
             targetSvelteFile,
-            lsAndTsDocResolver
+            lsAndTsDocResolver,
+            docManager
         };
     };
 
@@ -769,6 +770,49 @@ describe('TypescriptPlugin', function () {
         );
     });
 
+    it("shouldn't close svelte document when renamed", async () => {
+        const { plugin, docManager, targetSvelteFile } = await setupForOnWatchedFileChanges();
+        docManager.openClientDocument({
+            text: '',
+            uri: pathToUrl(targetSvelteFile)
+        });
+
+        const basename = path.basename(targetSvelteFile);
+        const newFileName = basename.replace('.svelte', '').toUpperCase() + '.svelte';
+        const newFilePath = path.join(path.dirname(targetSvelteFile), newFileName);
+        await plugin.onWatchFileChanges([
+            {
+                fileName: targetSvelteFile,
+                changeType: FileChangeType.Deleted
+            },
+            {
+                fileName: newFilePath,
+                changeType: FileChangeType.Created
+            }
+        ]);
+
+        const document = docManager.get(pathToUrl(targetSvelteFile));
+        assert.ok(document);
+    });
+
+    it("shouldn't mark client svelte document as close", async () => {
+        const { plugin, docManager, targetSvelteFile } = await setupForOnWatchedFileChanges();
+        docManager.openClientDocument({
+            text: '',
+            uri: pathToUrl(targetSvelteFile)
+        });
+
+        await plugin.onWatchFileChanges([
+            {
+                fileName: targetSvelteFile,
+                changeType: FileChangeType.Changed
+            }
+        ]);
+
+        const document = docManager.get(pathToUrl(targetSvelteFile));
+        assert.equal(document?.openedByClient, true);
+    });
+
     // Hacky, but it works. Needed due to testing both new and old transformation
     after(() => {
         __resetCache();

packages/language-server/test/plugins/typescript/service.test.ts

@@ -9,6 +9,8 @@ import {
 } from '../../../src/plugins/typescript/service';
 import { pathToUrl } from '../../../src/utils';
 import { createVirtualTsSystem, getRandomVirtualDirPath } from './test-utils';
+import sinon from 'sinon';
+import { RelativePattern } from 'vscode-languageserver-protocol';
 
 describe('service', () => {
     const testDir = path.join(__dirname, 'testfiles');
@@ -263,4 +265,61 @@ describe('service', () => {
             ls.getService().getSemanticDiagnostics(document.getFilePath()!);
         });
     });
+
+    it('skip directory watching if directory is root', async () => {
+        const dirPath = getRandomVirtualDirPath(path.join(testDir, 'Test'));
+        const { virtualSystem, lsDocumentContext } = setup();
+
+        const rootUris = [pathToUrl(dirPath)];
+
+        const watchDirectory = sinon.spy();
+        lsDocumentContext.watchDirectory = watchDirectory;
+        lsDocumentContext.nonRecursiveWatchPattern = '*.ts';
+
+        virtualSystem.readDirectory = () => [];
+        virtualSystem.directoryExists = () => true;
+
+        virtualSystem.writeFile(
+            path.join(dirPath, 'tsconfig.json'),
+            JSON.stringify({
+                compilerOptions: {},
+                include: ['src/**/*.ts', 'test/**/*.ts', '../foo/**/*.ts']
+            })
+        );
+
+        await getService(path.join(dirPath, 'random.svelte'), rootUris, lsDocumentContext);
+
+        sinon.assert.calledWith(watchDirectory.firstCall, <RelativePattern[]>[
+            {
+                baseUri: pathToUrl(path.join(dirPath, '../foo')),
+                pattern: '**/*.ts'
+            }
+        ]);
+    });
+
+    it('skip directory watching if directory do not exist', async () => {
+        const dirPath = getRandomVirtualDirPath(path.join(testDir, 'Test'));
+        const { virtualSystem, lsDocumentContext } = setup();
+
+        const rootUris = [pathToUrl(dirPath)];
+
+        const watchDirectory = sinon.spy();
+        lsDocumentContext.watchDirectory = watchDirectory;
+        lsDocumentContext.nonRecursiveWatchPattern = '*.ts';
+
+        virtualSystem.readDirectory = () => [];
+        virtualSystem.directoryExists = () => false;
+
+        virtualSystem.writeFile(
+            path.join(dirPath, 'tsconfig.json'),
+            JSON.stringify({
+                compilerOptions: {},
+                include: ['../test/**/*.ts']
+            })
+        );
+
+        await getService(path.join(dirPath, 'random.svelte'), rootUris, lsDocumentContext);
+
+        sinon.assert.calledWith(watchDirectory.firstCall, <RelativePattern[]>[]);
+    });
 });