diff --git a/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts b/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts index 7349a450e..d9b3913fe 100644 --- a/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts @@ -15,6 +15,7 @@ export class UpdateImportsProviderImpl implements UpdateImportsProvider { constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {} async updateImports(fileRename: FileRename): Promise { + // TODO does this handle folder moves/renames correctly? old/new path isn't a file then const oldPath = urlToPath(fileRename.oldUri); const newPath = urlToPath(fileRename.newUri); if (!oldPath || !newPath) { diff --git a/packages/svelte-vscode/src/extension.ts b/packages/svelte-vscode/src/extension.ts index ac29dab2c..9c7e9700b 100644 --- a/packages/svelte-vscode/src/extension.ts +++ b/packages/svelte-vscode/src/extension.ts @@ -352,14 +352,47 @@ function addRenameFileListener(getLS: () => LanguageClient) { newUri: evt.files[0].newUri.toString(true) } ); - if (!editsForFileRename) { + const edits = editsForFileRename?.documentChanges?.filter(TextDocumentEdit.is); + if (!edits) { return; } const workspaceEdit = new WorkspaceEdit(); - // Renaming a file should only result in edits of existing files - editsForFileRename.documentChanges?.filter(TextDocumentEdit.is).forEach((change) => + // We need to take into account multiple cases: + // - A Svelte file is moved/renamed + // -> all updates will be related to that Svelte file, do that here. The TS LS won't even notice the update + // - A TS/JS file is moved/renamed + // -> all updates will be related to that TS/JS file + // -> let the TS LS take care of these updates in TS/JS files, do Svelte file updates here + // - A folder with TS/JS AND Svelte files is moved/renamed + // -> all Svelte file updates are handled here + // -> all TS/JS file updates that consist of only TS/JS import updates are handled by the TS LS + // -> all TS/JS file updates that consist of only Svelte import updates are handled here + // -> all TS/JS file updates that are mixed are handled here, but also possibly by the TS LS + // if the TS plugin doesn't prevent it. This trades risk of broken updates with certainty of missed updates + edits.forEach((change) => { + const isTsOrJsFile = + change.textDocument.uri.endsWith('.ts') || + change.textDocument.uri.endsWith('.js'); + const containsSvelteImportUpdate = change.edits.some((edit) => + edit.newText.endsWith('.svelte') + ); + if (isTsOrJsFile && !containsSvelteImportUpdate) { + return; + } + change.edits.forEach((edit) => { + if ( + isTsOrJsFile && + !TsPlugin.isEnabled() && + !edit.newText.endsWith('.svelte') + ) { + // TS plugin enabled -> all mixed imports are handled here + // TS plugin disabled -> let TS/JS path updates be handled by the TS LS, Svelte here + return; + } + + // Renaming a file should only result in edits of existing files workspaceEdit.replace( Uri.parse(change.textDocument.uri), new Range( @@ -368,8 +401,8 @@ function addRenameFileListener(getLS: () => LanguageClient) { ), edit.newText ); - }) - ); + }); + }); workspace.applyEdit(workspaceEdit); } ); diff --git a/packages/svelte-vscode/src/tsplugin.ts b/packages/svelte-vscode/src/tsplugin.ts index 2a474c586..066b2afe6 100644 --- a/packages/svelte-vscode/src/tsplugin.ts +++ b/packages/svelte-vscode/src/tsplugin.ts @@ -4,12 +4,12 @@ export class TsPlugin { private enabled: boolean; constructor(context: ExtensionContext) { - this.enabled = this.getEnabledState(); + this.enabled = TsPlugin.isEnabled(); this.toggleTsPlugin(this.enabled); context.subscriptions.push( workspace.onDidChangeConfiguration(() => { - const enabled = this.getEnabledState(); + const enabled = TsPlugin.isEnabled(); if (enabled !== this.enabled) { this.enabled = enabled; this.toggleTsPlugin(this.enabled); @@ -18,7 +18,7 @@ export class TsPlugin { ); } - private getEnabledState(): boolean { + static isEnabled(): boolean { return workspace.getConfiguration('svelte').get('enable-ts-plugin') ?? false; } diff --git a/packages/typescript-plugin/src/language-service/index.ts b/packages/typescript-plugin/src/language-service/index.ts index b43f69526..39b662b9e 100644 --- a/packages/typescript-plugin/src/language-service/index.ts +++ b/packages/typescript-plugin/src/language-service/index.ts @@ -9,6 +9,7 @@ import { decorateDiagnostics } from './diagnostics'; import { decorateFindReferences } from './find-references'; import { decorateGetImplementation } from './implementation'; import { decorateRename } from './rename'; +import { decorateUpdateImports } from './update-imports'; const sveltePluginPatchSymbol = Symbol('sveltePluginPatchSymbol'); @@ -41,6 +42,7 @@ function decorateLanguageServiceInner( decorateCompletions(ls, logger); decorateGetDefinition(ls, snapshotManager, logger); decorateGetImplementation(ls, snapshotManager, logger); + decorateUpdateImports(ls, snapshotManager, logger); return ls; } diff --git a/packages/typescript-plugin/src/language-service/update-imports.ts b/packages/typescript-plugin/src/language-service/update-imports.ts new file mode 100644 index 000000000..c0de3d3d5 --- /dev/null +++ b/packages/typescript-plugin/src/language-service/update-imports.ts @@ -0,0 +1,32 @@ +import type ts from 'typescript/lib/tsserverlibrary'; +import { Logger } from '../logger'; +import { SvelteSnapshotManager } from '../svelte-snapshots'; +import { isSvelteFilePath } from '../utils'; + +export function decorateUpdateImports( + ls: ts.LanguageService, + snapshotManager: SvelteSnapshotManager, + logger: Logger +): void { + const getEditsForFileRename = ls.getEditsForFileRename; + ls.getEditsForFileRename = (oldFilePath, newFilePath, formatOptions, preferences) => { + const renameLocations = getEditsForFileRename( + oldFilePath, + newFilePath, + formatOptions, + preferences + ); + // If a file move/rename of a TS/JS file results a Svelte file change, + // the Svelte extension will notice that, too, and adjusts the same imports. + // This results in duplicate adjustments or race conditions with conflicting text spans + // which can break imports in some cases. + // Therefore don't do any updates of Svelte files and and also no updates of mixed TS files + // and let the Svelte extension handle that. + return renameLocations?.filter((renameLocation) => { + return ( + !isSvelteFilePath(renameLocation.fileName) && + !renameLocation.textChanges.some((change) => change.newText.endsWith('.svelte')) + ); + }); + }; +}