Skip to content

Commit

Permalink
chore: tweak completions (#2630)
Browse files Browse the repository at this point in the history
- 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)
  • Loading branch information
dummdidumm authored Dec 12, 2024
1 parent 29497af commit cdd3f2e
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 38 deletions.
26 changes: 26 additions & 0 deletions packages/language-server/src/lib/documents/Document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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) => {
Expand Down Expand Up @@ -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
*/
Expand Down
11 changes: 6 additions & 5 deletions packages/language-server/src/plugins/html/HTMLPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export class HTMLPlugin
: null;

const svelteStrictMode = prettierConfig?.svelteStrictMode;

items.forEach((item) => {
const startQuote = svelteStrictMode ? '"{' : '{';
const endQuote = svelteStrictMode ? '}"' : '}';
Expand All @@ -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:')) {
Expand All @@ -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
);
Expand Down
24 changes: 4 additions & 20 deletions packages/language-server/src/plugins/svelte/SvelteDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -37,7 +36,6 @@ type PositionMapper = Pick<DocumentMapper, 'getGeneratedPosition' | 'getOriginal
export class SvelteDocument {
private transpiledDoc: Promise<ITranspiledSvelteDocument> | undefined;
private compileResult: Promise<SvelteCompileResult> | undefined;
private svelteVersion: [number, number] | undefined;

public script: TagInformation | null;
public moduleScript: TagInformation | null;
Expand All @@ -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;
Expand All @@ -73,7 +68,7 @@ export class SvelteDocument {

async getTranspiled(): Promise<ITranspiledSvelteDocument> {
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(
Expand All @@ -99,16 +94,7 @@ export class SvelteDocument {
}

async getCompiledWith(options: CompileOptions = {}): Promise<SvelteCompileResult> {
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);
}
}

Expand All @@ -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),
{
Expand Down Expand Up @@ -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?.() || '';
Expand Down
6 changes: 3 additions & 3 deletions packages/language-server/src/plugins/svelte/SveltePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -52,9 +52,9 @@ export class SveltePlugin

async getCodeLens(document: Document): Promise<CodeLens[] | null> {
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();
Expand Down Expand Up @@ -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';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,16 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
if (
// Cursor is somewhere in regular HTML text
(svelteNode?.type === 'Text' &&
['Element', 'InlineComponent', 'Fragment', 'SlotTemplate'].includes(
svelteNode.parent?.type as any
)) ||
[
'Element',
'InlineComponent',
'Fragment',
'SlotTemplate',
'SnippetBlock',
'IfBlock',
'EachBlock',
'AwaitBlock'
].includes(svelteNode.parent?.type as any)) ||
// Cursor is at <div>|</div> in which case there's no TextNode inbetween
document.getText().substring(originalOffset - 1, originalOffset + 2) === '></'
) {
Expand Down Expand Up @@ -282,6 +289,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
const isCompletionInTag = svelteIsInTag(svelteNode, originalOffset);
const isHandlerCompletion =
svelteNode?.type === 'EventHandler' && svelteNode.parent?.type === 'Element';
const preferComponents = wordInfo.word[0] === '<' || isInScript(position, tsDoc);

const completionItems: CompletionItem[] = customCompletions;
const isValidCompletion = createIsValidCompletion(document, position, !!tsDoc.parserError);
Expand All @@ -295,7 +303,8 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
isCompletionInTag,
commitCharactersOptions,
asStore,
existingImports
existingImports,
preferComponents
);
if (completion) {
completionItems.push(
Expand Down Expand Up @@ -651,7 +660,8 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
isCompletionInTag: boolean,
commitCharactersOptions: CommitCharactersOptions,
asStore: boolean,
existingImports: Set<string>
existingImports: Set<string>,
preferComponents: boolean
): AppCompletionItem<CompletionResolveInfo> | null {
const completionLabelAndInsert = this.getCompletionLabelAndInsert(snapshot, comp);
if (!completionLabelAndInsert) {
Expand Down Expand Up @@ -697,8 +707,10 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
kind: scriptElementKindToCompletionItemKind(comp.kind),
commitCharacters: this.getCommitCharacters(comp, commitCharactersOptions, isSvelteComp),
// Make sure svelte component and runes take precedence
sortText: isRunesCompletion || isSvelteComp ? '-1' : comp.sortText,
preselect: isRunesCompletion || isSvelteComp ? true : comp.isRecommended,
sortText:
preferComponents && (isRunesCompletion || isSvelteComp) ? '-1' : comp.sortText,
preselect:
preferComponents && (isRunesCompletion || isSvelteComp) ? true : comp.isRecommended,
insertTextFormat: comp.isSnippet ? InsertTextFormat.Snippet : undefined,
labelDetails,
textEdit,
Expand Down
2 changes: 1 addition & 1 deletion packages/language-server/src/plugins/typescript/service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dirname, join, resolve, basename } from 'path';
import { dirname, basename } from 'path';
import ts from 'typescript';
import {
DiagnosticSeverity,
Expand Down
13 changes: 11 additions & 2 deletions packages/language-server/test/plugins/html/HTMLPlugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import {
import { HTMLPlugin } from '../../../src/plugins';
import { DocumentManager, Document } from '../../../src/lib/documents';
import { LSConfigManager } from '../../../src/ls-config';
import { VERSION } from 'svelte/compiler';

const isSvelte5Plus = Number(VERSION.split('.')[0]) >= 5;

describe('HTML Plugin', () => {
function setup(content: string) {
Expand Down Expand Up @@ -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, <CompletionItem>{
const expected: CompletionItem = {
label: 'on:click',
kind: CompletionItemKind.Value,
documentation: {
Expand All @@ -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 () => {
Expand Down

0 comments on commit cdd3f2e

Please sign in to comment.