Skip to content

Commit

Permalink
feat: Svelte 5 component class/function interop (#2380)
Browse files Browse the repository at this point in the history
Svelte 5 uses functions to define components under the hood. This should be represented in the types. We can't just switch to using functions though because d.ts files created from Svelte 4 libraries should still work, and those contain classes. So we need interop between functions and classes. The idea is therefore:

Svelte 5 creates a default export which is both a function and a class constructor
Various places are adjusted to support the new default exports

Also see sveltejs/svelte#11775
  • Loading branch information
dummdidumm committed May 29, 2024
1 parent 15a4aab commit 2478212
Show file tree
Hide file tree
Showing 130 changed files with 1,565 additions and 697 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,12 @@ export class JsOrTsComponentInfoProvider implements ComponentInfoProvider {
return null;
}

const defClass = findContainingNode(sourceFile, def.textSpan, ts.isClassDeclaration);
const defClass = findContainingNode(
sourceFile,
def.textSpan,
(node): node is ts.ClassDeclaration | ts.VariableDeclaration =>
ts.isClassDeclaration(node) || ts.isTypeAliasDeclaration(node)
);

if (!defClass) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ export class SvelteDocumentSnapshot implements DocumentSnapshot {
private url = pathToUrl(this.filePath);

version = this.parent.version;
isSvelte5Plus = Number(this.svelteVersion?.split('.')[0]) >= 5;

constructor(
public readonly parent: Document,
Expand All @@ -281,10 +282,6 @@ export class SvelteDocumentSnapshot implements DocumentSnapshot {
private readonly htmlAst?: TemplateNode
) {}

get isSvelte5Plus() {
return Number(this.svelteVersion?.split('.')[0]) >= 5;
}

get filePath() {
return this.parent.getFilePath() || '';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe

if (detail) {
const { detail: itemDetail, documentation: itemDocumentation } =
this.getCompletionDocument(detail, is$typeImport);
this.getCompletionDocument(tsDoc, detail, is$typeImport);

// VSCode + tsserver won't have this pop-in effect
// because tsserver has internal APIs for caching
Expand Down Expand Up @@ -897,7 +897,11 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
return completionItem;
}

private getCompletionDocument(compDetail: ts.CompletionEntryDetails, is$typeImport: boolean) {
private getCompletionDocument(
tsDoc: SvelteDocumentSnapshot,
compDetail: ts.CompletionEntryDetails,
is$typeImport: boolean
) {
const { sourceDisplay, documentation: tsDocumentation, displayParts, tags } = compDetail;
let parts = compDetail.codeActions?.map((codeAction) => codeAction.description) ?? [];

Expand All @@ -910,7 +914,18 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
);
}

parts.push(changeSvelteComponentName(ts.displayPartsToString(displayParts)));
let text = changeSvelteComponentName(ts.displayPartsToString(displayParts));
if (tsDoc.isSvelte5Plus && text.includes('(alias)')) {
// The info contains both the const and type export along with a bunch of gibberish we want to hide
if (text.includes('__SvelteComponent_')) {
// import - remove completely
text = '';
} else if (text.includes('__sveltets_2_IsomorphicComponent')) {
// already imported - only keep the last part
text = text.substring(text.lastIndexOf('import'));
}
}
parts.push(text);

const markdownDoc = getMarkdownDocumentation(tsDocumentation, tags);
const documentation: MarkupContent | undefined = markdownDoc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
continue;
}

diagnostic = adjustIfNecessary(diagnostic);
diagnostic = adjustIfNecessary(diagnostic, tsDoc.isSvelte5Plus);
diagnostic = swapDiagRangeStartEndIfNecessary(diagnostic);
converted.push(diagnostic);
}
Expand Down Expand Up @@ -350,7 +350,7 @@ function isNoUsedBeforeAssigned(
/**
* Some diagnostics have JSX-specific or confusing nomenclature. Enhance/adjust them for more clarity.
*/
function adjustIfNecessary(diagnostic: Diagnostic): Diagnostic {
function adjustIfNecessary(diagnostic: Diagnostic, isSvelte5Plus: boolean): Diagnostic {
if (
diagnostic.code === DiagnosticCode.ARG_TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
diagnostic.message.includes('ConstructorOfATypedSvelteComponent')
Expand All @@ -362,9 +362,11 @@ function adjustIfNecessary(diagnostic: Diagnostic): Diagnostic {
'\n\nPossible causes:\n' +
'- You use the instance type of a component where you should use the constructor type\n' +
'- Type definitions are missing for this Svelte Component. ' +
'If you are using Svelte 3.31+, use SvelteComponentTyped to add a definition:\n' +
' import type { SvelteComponentTyped } from "svelte";\n' +
' class ComponentName extends SvelteComponentTyped<{propertyName: string;}> {}'
(isSvelte5Plus
? ''
: 'If you are using Svelte 3.31+, use SvelteComponentTyped to add a definition:\n' +
' import type { SvelteComponentTyped } from "svelte";\n' +
' class ComponentName extends SvelteComponentTyped<{propertyName: string;}> {}')
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,26 @@ export class FindReferencesProviderImpl implements FindReferencesProvider {
)
);

return flatten(references.filter(isNotNullOrUndefined));
const flattened: Location[] = [];
for (const ref of references) {
if (ref) {
const tmp: Location[] = []; // perf optimization: we know each iteration has unique references
for (const r of ref) {
const exists = flattened.some(
(f) =>
f.uri === r.uri &&
f.range.start.line === r.range.start.line &&
f.range.start.character === r.range.start.character
);
if (!exists) {
tmp.push(r);
}
}
flattened.push(...tmp);
}
}

return flattened;
}

private async mapReference(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,16 @@ export class HoverProviderImpl implements HoverProvider {
return null;
}

const declaration = ts.displayPartsToString(info.displayParts);
let declaration = ts.displayPartsToString(info.displayParts);
if (
tsDoc.isSvelte5Plus &&
declaration.includes('(alias)') &&
declaration.includes('__sveltets_2_IsomorphicComponent')
) {
// info ends with "import ComponentName"
declaration = declaration.substring(declaration.lastIndexOf('import'));
}

const documentation = getMarkdownDocumentation(info.documentation, info.tags);

// https://microsoft.github.io/language-server-protocol/specification#textDocument_hover
Expand Down
37 changes: 29 additions & 8 deletions packages/language-server/src/plugins/typescript/features/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,42 @@ export function getComponentAtPosition(
doc.positionAt(node.start + symbolPosWithinNode + 1)
);

let def = lang.getDefinitionAtPosition(tsDoc.filePath, tsDoc.offsetAt(generatedPosition))?.[0];

while (def != null && def.kind !== ts.ScriptElementKind.classElement) {
const newDef = lang.getDefinitionAtPosition(tsDoc.filePath, def.textSpan.start)?.[0];
if (newDef?.fileName === def.fileName && newDef?.textSpan.start === def.textSpan.start) {
let defs = lang.getDefinitionAtPosition(tsDoc.filePath, tsDoc.offsetAt(generatedPosition));
// Svelte 5 uses a const and a type alias instead of a class, and we want the latter.
// We still gotta check for a class in Svelte 5 because of d.ts files generated for Svelte 4 containing classes.
let def1 = defs?.[0];
let def2 = tsDoc.isSvelte5Plus ? defs?.[1] : undefined;

while (
def1 != null &&
def1.kind !== ts.ScriptElementKind.classElement &&
(def2 == null ||
def2.kind !== ts.ScriptElementKind.constElement ||
!def2.name.endsWith('__SvelteComponent_'))
) {
const newDefs = lang.getDefinitionAtPosition(tsDoc.filePath, def1.textSpan.start);
const newDef = newDefs?.[0];
if (newDef?.fileName === def1.fileName && newDef?.textSpan.start === def1.textSpan.start) {
break;
}
def = newDef;
defs = newDefs;
def1 = newDef;
def2 = tsDoc.isSvelte5Plus ? newDefs?.[1] : undefined;
}

if (!def) {
if (!def1 && !def2) {
return null;
}

return JsOrTsComponentInfoProvider.create(lang, def);
if (
def2 != null &&
def2.kind === ts.ScriptElementKind.constElement &&
def2.name.endsWith('__SvelteComponent_')
) {
def1 = undefined;
}

return JsOrTsComponentInfoProvider.create(lang, def1! || def2!);
}

export function isComponentAtPosition(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDo
import { __resetCache } from '../../../../src/plugins/typescript/service';
import { pathToUrl } from '../../../../src/utils';
import { serviceWarmup } from '../test-utils';
import { VERSION } from 'svelte/compiler';

const testDir = path.join(__dirname, '..');
const isSvelte5Plus = +VERSION.split('.')[0] >= 5;

describe('CallHierarchyProvider', function () {
const callHierarchyTestDirRelative = path.join('testfiles', 'call-hierarchy');
Expand Down Expand Up @@ -386,6 +388,11 @@ describe('CallHierarchyProvider', function () {
});

it('can provide outgoing calls for component file', async () => {
if (isSvelte5Plus) {
// Doesn't work due to https://github.com/microsoft/TypeScript/issues/43740 and https://github.com/microsoft/TypeScript/issues/42375
return;
}

const { provider, document } = setup(outgoingComponentName);

const items = await provider.prepareCallHierarchy(document, { line: 10, character: 1 });
Expand All @@ -411,6 +418,11 @@ describe('CallHierarchyProvider', function () {
});

it('can provide outgoing calls for component tags', async () => {
if (isSvelte5Plus) {
// Doesn't work due to https://github.com/microsoft/TypeScript/issues/43740 and https://github.com/microsoft/TypeScript/issues/42375
return;
}

const { provider, document } = setup(outgoingComponentName);

const items = await provider.prepareCallHierarchy(document, { line: 0, character: 2 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import { __resetCache } from '../../../../src/plugins/typescript/service';
import { pathToUrl } from '../../../../src/utils';
import { recursiveServiceWarmup } from '../test-utils';
import { DiagnosticCode } from '../../../../src/plugins/typescript/features/DiagnosticsProvider';
import { VERSION } from 'svelte/compiler';

const testDir = path.join(__dirname, '..');
const indent = ' '.repeat(4);
const isSvelte5Plus = +VERSION.split('.')[0] >= 5;

describe('CodeActionsProvider', function () {
recursiveServiceWarmup(
Expand Down Expand Up @@ -374,6 +376,17 @@ describe('CodeActionsProvider', function () {
uri: getUri('codeaction-checkJs.svelte'),
version: null
};

if (isSvelte5Plus) {
// Maybe because of the hidden interface declarations? It's harmless anyway
if (
codeActions.length === 4 &&
codeActions[3].title === "Add '@ts-ignore' to all error messages"
) {
codeActions.splice(3, 1);
}
}

assert.deepStrictEqual(codeActions, <CodeAction[]>[
{
edit: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ import { sortBy } from 'lodash';
import { LSConfigManager } from '../../../../src/ls-config';
import { __resetCache } from '../../../../src/plugins/typescript/service';
import { getRandomVirtualDirPath, serviceWarmup, setupVirtualEnvironment } from '../test-utils';
import { VERSION } from 'svelte/compiler';

const testDir = join(__dirname, '..');
const testFilesDir = join(testDir, 'testfiles', 'completions');
const newLine = ts.sys.newLine;
const indent = ' '.repeat(4);
const isSvelte5Plus = +VERSION.split('.')[0] >= 5;

const fileNameToAbsoluteUri = (file: string) => {
return pathToUrl(join(testFilesDir, file));
Expand Down Expand Up @@ -855,7 +857,7 @@ describe('CompletionProviderImpl', function () {

assert.strictEqual(
detail,
'Add import from "../imported-file.svelte"\n\nclass ImportedFile'
`Add import from "../imported-file.svelte"${isSvelte5Plus ? '' : '\n\nclass ImportedFile'}`
);

assert.strictEqual(
Expand Down Expand Up @@ -893,7 +895,7 @@ describe('CompletionProviderImpl', function () {

assert.strictEqual(
detail,
'Add import from "../imported-file.svelte"\n\nclass ImportedFile'
`Add import from "../imported-file.svelte"${isSvelte5Plus ? '' : '\n\nclass ImportedFile'}`
);

assert.strictEqual(
Expand Down Expand Up @@ -1502,7 +1504,10 @@ describe('CompletionProviderImpl', function () {
const item2 = completions2?.items.find((item) => item.label === 'Bar');
const { detail } = await completionProvider.resolveCompletion(document, item2!);

assert.strictEqual(detail, 'Add import from "./Bar.svelte"\n\nclass Bar');
assert.strictEqual(
detail,
`Add import from "./Bar.svelte"${isSvelte5Plus ? '' : '\n\nclass Bar'}`
);
});

it("doesn't use empty cache", async () => {
Expand Down Expand Up @@ -1551,7 +1556,10 @@ describe('CompletionProviderImpl', function () {
const item2 = completions?.items.find((item) => item.label === 'Bar');
const { detail } = await completionProvider.resolveCompletion(document, item2!);

assert.strictEqual(detail, 'Add import from "./Bar.svelte"\n\nclass Bar');
assert.strictEqual(
detail,
`Add import from "./Bar.svelte"${isSvelte5Plus ? '' : '\n\nclass Bar'}`
);
});

it('can auto import new export', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ import { FindComponentReferencesProviderImpl } from '../../../../src/plugins/typ
import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDocResolver';
import { pathToUrl } from '../../../../src/utils';
import { serviceWarmup } from '../test-utils';
import { Location } from 'vscode-html-languageservice';
import { VERSION } from 'svelte/compiler';

const testDir = path.join(__dirname, '..', 'testfiles');
const isSvelte5Plus = +VERSION.split('.')[0] >= 5;

describe('FindComponentReferencesProvider', function () {
serviceWarmup(this, testDir);
Expand Down Expand Up @@ -54,20 +57,7 @@ describe('FindComponentReferencesProvider', function () {

const results = await provider.findComponentReferences(document.uri.toString());

assert.deepStrictEqual(results, [
{
range: {
start: {
line: 8,
character: 15
},
end: {
line: 8,
character: 22
}
},
uri: getUri('find-component-references-parent.svelte')
},
const expected: Location[] = [
{
range: {
start: {
Expand Down Expand Up @@ -120,6 +110,22 @@ describe('FindComponentReferencesProvider', function () {
},
uri: getUri('find-component-references-parent2.svelte')
}
]);
];
if (!isSvelte5Plus) {
expected.unshift({
range: {
start: {
line: 8,
character: 15
},
end: {
line: 8,
character: 22
}
},
uri: getUri('find-component-references-parent.svelte')
});
}
assert.deepStrictEqual(results, expected);
});
});

0 comments on commit 2478212

Please sign in to comment.