Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle issues with svelte file watching #2421

Merged
merged 7 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 6 additions & 2 deletions packages/language-server/src/plugins/typescript/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
4 changes: 4 additions & 0 deletions packages/language-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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;
Expand All @@ -627,7 +627,8 @@ describe('TypescriptPlugin', function () {
snapshotManager,
plugin,
targetSvelteFile,
lsAndTsDocResolver
lsAndTsDocResolver,
docManager
};
};

Expand Down Expand Up @@ -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();
Expand Down
59 changes: 59 additions & 0 deletions packages/language-server/test/plugins/typescript/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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[]>[]);
});
});