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

feat: Svelte 5 component class/function interop #2380

Merged
merged 36 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0f68761
wip
dummdidumm May 21, 2024
fea467d
exports
dummdidumm May 21, 2024
f5dfbbe
bindings
dummdidumm May 21, 2024
9c6766d
generics
dummdidumm May 21, 2024
0b46528
Merge remote-tracking branch 'upstream/master' into component-class-f…
dummdidumm May 21, 2024
0ae7ace
resolve component type for Svelte 5 type alias correctly
dummdidumm May 21, 2024
1fec79e
dts generation simple
dummdidumm May 21, 2024
4dc0867
dts generics
dummdidumm May 22, 2024
c9bf951
take into account Record<string, never> case, remove obsolete function
dummdidumm May 22, 2024
c670fcd
omit partial/any prop wrappers in runes mode, obsolete there
dummdidumm May 22, 2024
3abf0df
jsdoc on component
dummdidumm May 22, 2024
1cf5d37
tests
dummdidumm May 22, 2024
b35a5c4
accessors
dummdidumm May 22, 2024
4ff61f9
fix tests
dummdidumm May 22, 2024
f999d76
run these in runes mode onl
dummdidumm May 22, 2024
5180e9c
fix tests
dummdidumm May 22, 2024
ea88604
generics + children
dummdidumm May 22, 2024
b88e1e9
tweak test suite for Svelte 5
dummdidumm May 22, 2024
e156aa3
more
dummdidumm May 22, 2024
eaeaddc
skip tests, unfortunate limitation, but maybe not that relevant?
dummdidumm May 23, 2024
0fd2fc3
adjust test
dummdidumm May 23, 2024
2700d72
fix
dummdidumm May 23, 2024
571ddde
fix test
dummdidumm May 23, 2024
941d7b1
fix references
dummdidumm May 23, 2024
9d05749
cleanup
dummdidumm May 23, 2024
136e552
tweak output to not generate props / slots when it doesn't have to
dummdidumm May 23, 2024
bb1131c
tweak repl debugger
dummdidumm May 23, 2024
a60cf7c
dont run runes tests in v4 / do stuff only in v5
dummdidumm May 23, 2024
36df9e1
update v4 test
dummdidumm May 23, 2024
9e35892
check for component type to exist
dummdidumm May 23, 2024
da4a891
fix check of "has component export" (it failed because it doesn't wor…
dummdidumm May 24, 2024
ab0d687
forward-compat for future: don't rely on old SvelteComponent class fo…
dummdidumm May 24, 2024
921f0e8
move bindings to a property so it doesn't pollute the instance return…
dummdidumm May 24, 2024
788405d
Merge branch 'master' into component-class-fn-interop
dummdidumm May 28, 2024
6f7c999
remove gibberish from more places
dummdidumm May 29, 2024
84344d5
Merge branch 'component-class-fn-interop' of https://github.com/svelt…
dummdidumm May 29, 2024
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 @@ -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
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
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);
});
});
Loading
Loading