From cdd3f2e9ed18f981d2bd58e6950eb4a40e99f277 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Thu, 12 Dec 2024 14:18:57 +0100 Subject: [PATCH] chore: tweak completions (#2630) - In Svelte 5, prefer `onclick` over `on:click` suggestions - Ignore completions in more Text positions - Only prefer runes/components in the script tag and in opening tags; should give more accurate completions in the template (as part of this I refactered the compiler usage a bit; it's reusable/shared between documents now) --- .../src/lib/documents/Document.ts | 26 +++++++++++++++++++ .../src/plugins/html/HTMLPlugin.ts | 11 ++++---- .../src/plugins/svelte/SvelteDocument.ts | 24 +++-------------- .../src/plugins/svelte/SveltePlugin.ts | 6 ++--- .../typescript/features/CompletionProvider.ts | 26 ++++++++++++++----- .../src/plugins/typescript/service.ts | 2 +- .../test/plugins/html/HTMLPlugin.test.ts | 13 ++++++++-- 7 files changed, 70 insertions(+), 38 deletions(-) diff --git a/packages/language-server/src/lib/documents/Document.ts b/packages/language-server/src/lib/documents/Document.ts index 369808e2c..dde5a0e2f 100644 --- a/packages/language-server/src/lib/documents/Document.ts +++ b/packages/language-server/src/lib/documents/Document.ts @@ -5,6 +5,7 @@ import { parseHtml } from './parseHtml'; import { SvelteConfig, configLoader } from './configLoader'; import { HTMLDocument } from 'vscode-html-languageservice'; import { Range } from 'vscode-languageserver'; +import { importSvelte } from '../../importPackage'; /** * Represents a text document contains a svelte component. @@ -25,6 +26,16 @@ export class Document extends WritableDocument { */ private path = urlToPath(this.url); + private _compiler: typeof import('svelte/compiler') | undefined; + get compiler() { + return this.getCompiler(); + } + + private svelteVersion: [number, number] | undefined; + public get isSvelte5() { + return this.getSvelteVersion()[0] > 4; + } + constructor( public url: string, public content: string @@ -34,6 +45,13 @@ export class Document extends WritableDocument { this.updateDocInfo(); } + private getCompiler() { + if (!this._compiler) { + this._compiler = importSvelte(this.getFilePath() || ''); + } + return this._compiler; + } + private updateDocInfo() { this.html = parseHtml(this.content); const update = (config: SvelteConfig | undefined) => { @@ -66,6 +84,14 @@ export class Document extends WritableDocument { } } + getSvelteVersion() { + if (!this.svelteVersion) { + const [major, minor] = this.compiler.VERSION.split('.'); + this.svelteVersion = [Number(major), Number(minor)]; + } + return this.svelteVersion; + } + /** * Get text content */ diff --git a/packages/language-server/src/plugins/html/HTMLPlugin.ts b/packages/language-server/src/plugins/html/HTMLPlugin.ts index ffba79d62..ca830b3e2 100644 --- a/packages/language-server/src/plugins/html/HTMLPlugin.ts +++ b/packages/language-server/src/plugins/html/HTMLPlugin.ts @@ -159,6 +159,7 @@ export class HTMLPlugin : null; const svelteStrictMode = prettierConfig?.svelteStrictMode; + items.forEach((item) => { const startQuote = svelteStrictMode ? '"{' : '{'; const endQuote = svelteStrictMode ? '}"' : '}'; @@ -171,6 +172,10 @@ export class HTMLPlugin ...item.textEdit, newText: item.textEdit.newText.replace('="$1"', `$2=${startQuote}$1${endQuote}`) }; + // In Svelte 5, people should use `onclick` instead of `on:click` + if (document.isSvelte5) { + item.sortText = 'z' + (item.sortText ?? item.label); + } } if (item.label.startsWith('bind:')) { @@ -182,11 +187,7 @@ export class HTMLPlugin }); return CompletionList.create( - [ - ...this.toCompletionItems(items), - ...this.getLangCompletions(items), - ...emmetResults.items - ], + [...items, ...this.getLangCompletions(items), ...emmetResults.items], // Emmet completions change on every keystroke, so they are never complete emmetResults.items.length > 0 ); diff --git a/packages/language-server/src/plugins/svelte/SvelteDocument.ts b/packages/language-server/src/plugins/svelte/SvelteDocument.ts index 65aa1146e..0619d1068 100644 --- a/packages/language-server/src/plugins/svelte/SvelteDocument.ts +++ b/packages/language-server/src/plugins/svelte/SvelteDocument.ts @@ -5,7 +5,6 @@ import { CompileOptions } from 'svelte/types/compiler/interfaces'; // @ts-ignore import { PreprocessorGroup, Processed } from 'svelte/types/compiler/preprocess'; import { Position } from 'vscode-languageserver'; -import { getPackageInfo, importSvelte } from '../../importPackage'; import { Document, DocumentMapper, @@ -37,7 +36,6 @@ type PositionMapper = Pick | undefined; private compileResult: Promise | undefined; - private svelteVersion: [number, number] | undefined; public script: TagInformation | null; public moduleScript: TagInformation | null; @@ -48,9 +46,6 @@ export class SvelteDocument { public get config() { return this.parent.configPromise; } - public get isSvelte5() { - return this.getSvelteVersion()[0] > 4; - } constructor(private parent: Document) { this.script = this.parent.scriptInfo; @@ -73,7 +68,7 @@ export class SvelteDocument { async getTranspiled(): Promise { if (!this.transpiledDoc) { - const [major, minor] = this.getSvelteVersion(); + const [major, minor] = this.parent.getSvelteVersion(); if (major > 3 || (major === 3 && minor >= 32)) { this.transpiledDoc = TranspiledSvelteDocument.create( @@ -99,16 +94,7 @@ export class SvelteDocument { } async getCompiledWith(options: CompileOptions = {}): Promise { - const svelte = importSvelte(this.getFilePath()); - return svelte.compile((await this.getTranspiled()).getText(), options); - } - - private getSvelteVersion() { - if (!this.svelteVersion) { - const { major, minor } = getPackageInfo('svelte', this.getFilePath()).version; - this.svelteVersion = [major, minor]; - } - return this.svelteVersion; + return this.parent.compiler.compile((await this.getTranspiled()).getText(), options); } } @@ -123,8 +109,7 @@ export class TranspiledSvelteDocument implements ITranspiledSvelteDocument { } const filename = document.getFilePath() || ''; - const svelte = importSvelte(filename); - const preprocessed = await svelte.preprocess( + const preprocessed = await document.compiler.preprocess( document.getText(), wrapPreprocessors(config?.preprocess), { @@ -453,8 +438,7 @@ async function transpile( return wrappedPreprocessor; }); - const svelte = importSvelte(document.getFilePath() || ''); - const result = await svelte.preprocess(document.getText(), wrappedPreprocessors, { + const result = await document.compiler.preprocess(document.getText(), wrappedPreprocessors, { filename: document.getFilePath() || '' }); const transpiled = result.code || result.toString?.() || ''; diff --git a/packages/language-server/src/plugins/svelte/SveltePlugin.ts b/packages/language-server/src/plugins/svelte/SveltePlugin.ts index 3fe7c49a5..70e00e9a0 100644 --- a/packages/language-server/src/plugins/svelte/SveltePlugin.ts +++ b/packages/language-server/src/plugins/svelte/SveltePlugin.ts @@ -16,7 +16,7 @@ import { WorkspaceEdit } from 'vscode-languageserver'; import { Plugin } from 'prettier'; -import { getPackageInfo, importPrettier, importSvelte } from '../../importPackage'; +import { getPackageInfo, importPrettier } from '../../importPackage'; import { Document } from '../../lib/documents'; import { Logger } from '../../logger'; import { LSConfigManager, LSSvelteConfig } from '../../ls-config'; @@ -52,9 +52,9 @@ export class SveltePlugin async getCodeLens(document: Document): Promise { if (!this.featureEnabled('runesLegacyModeCodeLens')) return null; + if (!document.isSvelte5) return null; const doc = await this.getSvelteDoc(document); - if (!doc.isSvelte5) return null; try { const result = await doc.getCompiled(); @@ -355,7 +355,7 @@ export class SveltePlugin private migrate(document: Document): WorkspaceEdit | string { try { - const compiler = importSvelte(document.getFilePath() ?? '') as any; + const compiler = document.compiler as any; if (!compiler.migrate) { return 'Your installed Svelte version does not support migration'; } diff --git a/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts b/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts index dae13c72c..c37b9137e 100644 --- a/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts @@ -168,9 +168,16 @@ export class CompletionsProviderImpl implements CompletionsProvider| in which case there's no TextNode inbetween document.getText().substring(originalOffset - 1, originalOffset + 2) === '> + existingImports: Set, + preferComponents: boolean ): AppCompletionItem | null { const completionLabelAndInsert = this.getCompletionLabelAndInsert(snapshot, comp); if (!completionLabelAndInsert) { @@ -697,8 +707,10 @@ export class CompletionsProviderImpl implements CompletionsProvider= 5; describe('HTML Plugin', () => { function setup(content: string) { @@ -67,7 +70,7 @@ describe('HTML Plugin', () => { const completions = await plugin.getCompletions(document, Position.create(0, 7)); const onClick = completions?.items.find((item) => item.label === 'on:click'); - assert.deepStrictEqual(onClick, { + const expected: CompletionItem = { label: 'on:click', kind: CompletionItemKind.Value, documentation: { @@ -80,7 +83,13 @@ describe('HTML Plugin', () => { ), insertTextFormat: InsertTextFormat.Snippet, command: undefined - }); + }; + + if (isSvelte5Plus) { + expected.sortText = 'zon:click'; + } + + assert.deepStrictEqual(onClick, expected); }); it('provide event handler completions in svelte strict mode', async () => {