chore: Bindable and Svelte 5 interop (#2336)

- $bindable() support, adjust language server to accomodate for weird error message types and binding shorthand rename - run Svelte 5 in CI - remove programmatic implicit children handling, it's handled within Svelte's component types by now - cannot bind to exports in runes mode
sveltejs · Apr 25, 2024 · 957b8d6 · 957b8d6
1 parent 4c8c0db
commit 957b8d6
Show file tree

Hide file tree

Showing 94 changed files with 1,589 additions and 311 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -23,6 +23,31 @@ jobs:
         env:
           CI: true
 
+  test-svelte5:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3
+      - uses: pnpm/[email protected]
+      - uses: actions/setup-node@v3
+        with:
+          node-version: "18.x"
+          cache: pnpm
+
+      # Lets us use one-liner JSON manipulations the package.json files
+      - run: "npm install -g json"
+
+      # Get projects set up
+      - run: json -I -f package.json -e 'this.pnpm={"overrides":{"svelte":"^5.0.0-next.100"}}'
+      - run: pnpm install --no-frozen-lockfile
+      - run: pnpm bootstrap
+      - run: pnpm build
+
+      # Run any tests
+      - run: pnpm test
+        env:
+          CI: true
+
   lint:
     runs-on: ubuntu-latest
 

diff --git a/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts b/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts
@@ -63,7 +63,7 @@ async function tryGetDiagnostics(
         if (cancellationToken?.isCancellationRequested) {
             return [];
         }
-        return (((res.stats as any)?.warnings || res.warnings || []) as Warning[])
+        return (res.warnings || [])
             .filter((warning) => settings[warning.code] !== 'ignore')
             .map((warning) => {
                 const start = warning.start || { line: 1, column: 0 };

diff --git a/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts b/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts
@@ -87,6 +87,7 @@ export namespace DocumentSnapshot {
             document,
             parserError,
             scriptKind,
+            options.version,
             text,
             nrPrependedLines,
             exportedNames,
@@ -272,13 +273,18 @@ export class SvelteDocumentSnapshot implements DocumentSnapshot {
         public readonly parent: Document,
         public readonly parserError: ParserError | null,
         public readonly scriptKind: ts.ScriptKind,
+        public readonly svelteVersion: string | undefined,
         private readonly text: string,
         private readonly nrPrependedLines: number,
         private readonly exportedNames: IExportedNames,
         private readonly tsxMap?: EncodedSourceMap,
         private readonly htmlAst?: TemplateNode
     ) {}
 
+    get isSvelte5Plus() {
+        return Number(this.svelteVersion?.split('.')[0]) >= 5;
+    }
+
     get filePath() {
         return this.parent.getFilePath() || '';
     }

diff --git a/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts b/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts
@@ -21,7 +21,14 @@ import {
     isStoreVariableIn$storeDeclaration,
     get$storeOffsetOf$storeDeclaration
 } from './utils';
-import { not, flatten, passMap, swapRangeStartEndIfNecessary, memoize } from '../../../utils';
+import {
+    not,
+    flatten,
+    passMap,
+    swapRangeStartEndIfNecessary,
+    memoize,
+    traverseTypeString
+} from '../../../utils';
 import { LSConfigManager } from '../../../ls-config';
 import { isAttributeName, isEventHandler } from '../svelte-ast-utils';
 
@@ -37,7 +44,10 @@ export enum DiagnosticCode {
     DUPLICATED_JSX_ATTRIBUTES = 17001, // "JSX elements cannot have multiple attributes with the same name."
     DUPLICATE_IDENTIFIER = 2300, // "Duplicate identifier 'xxx'"
     MULTIPLE_PROPS_SAME_NAME = 1117, // "An object literal cannot have multiple properties with the same name in strict mode."
-    TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y = 2345, // "Argument of type '..' is not assignable to parameter of type '..'."
+    ARG_TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y = 2345, // "Argument of type '..' is not assignable to parameter of type '..'."
+    TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y = 2322, // "Type '..' is not assignable to type '..'."
+    TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y_DID_YOU_MEAN = 2820, // "Type '..' is not assignable to type '..'. Did you mean '...'?"
+    UNKNOWN_PROP = 2353, // "Object literal may only specify known properties, and '...' does not exist in type '...'"
     MISSING_PROPS = 2739, // "Type '...' is missing the following properties from type '..': ..."
     MISSING_PROP = 2741, // "Property '..' is missing in type '..' but required in type '..'."
     NO_OVERLOAD_MATCHES_CALL = 2769, // "No overload matches this call"
@@ -101,7 +111,7 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
         for (const diagnostic of diagnostics) {
             if (
                 (diagnostic.code === DiagnosticCode.NO_OVERLOAD_MATCHES_CALL ||
-                    diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y) &&
+                    diagnostic.code === DiagnosticCode.ARG_TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y) &&
                 !notGenerated(diagnostic)
             ) {
                 if (isStoreVariableIn$storeDeclaration(tsDoc.getFullText(), diagnostic.start!)) {
@@ -147,7 +157,7 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
             .map(mapRange(tsDoc, document, lang))
             .filter(hasNoNegativeLines)
             .filter(isNoFalsePositive(document, tsDoc))
-            .map(enhanceIfNecessary)
+            .map(adjustIfNecessary)
             .map(swapDiagRangeStartEndIfNecessary);
     }
 
@@ -180,9 +190,11 @@ function mapRange(
         }
 
         if (
-            [DiagnosticCode.MISSING_PROP, DiagnosticCode.MISSING_PROPS].includes(
+            ([DiagnosticCode.MISSING_PROP, DiagnosticCode.MISSING_PROPS].includes(
                 diagnostic.code as number
-            ) &&
+            ) ||
+                (DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
+                    diagnostic.message.includes("'PropsWithChildren<"))) &&
             !hasNonZeroRange({ range })
         ) {
             const node = getNodeIfIsInStartTag(document.html, document.offsetAt(range.start));
@@ -286,11 +298,11 @@ function isNoUsedBeforeAssigned(
 }
 
 /**
- * Some diagnostics have JSX-specific nomenclature. Enhance them for more clarity.
+ * Some diagnostics have JSX-specific or confusing nomenclature. Enhance/adjust them for more clarity.
  */
-function enhanceIfNecessary(diagnostic: Diagnostic): Diagnostic {
+function adjustIfNecessary(diagnostic: Diagnostic): Diagnostic {
     if (
-        diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
+        diagnostic.code === DiagnosticCode.ARG_TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
         diagnostic.message.includes('ConstructorOfATypedSvelteComponent')
     ) {
         return {
@@ -315,6 +327,37 @@ function enhanceIfNecessary(diagnostic: Diagnostic): Diagnostic {
         };
     }
 
+    if (
+        (diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y ||
+            diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y_DID_YOU_MEAN) &&
+        diagnostic.message.includes("'Bindable<")
+    ) {
+        const countBindable = (diagnostic.message.match(/'Bindable\</g) || []).length;
+        const countBinding = (diagnostic.message.match(/'Binding\</g) || []).length;
+        if (countBindable === 1 && countBinding === 0) {
+            // Remove distracting Bindable<...> from diagnostic message
+            const start = diagnostic.message.indexOf("'Bindable<");
+            const startType = start + "'Bindable".length;
+            const end = traverseTypeString(diagnostic.message, startType, '>');
+            diagnostic.message =
+                diagnostic.message.substring(0, start + 1) +
+                diagnostic.message.substring(startType + 1, end) +
+                diagnostic.message.substring(end + 1);
+        } else if (countBinding === 3 && countBindable === 1) {
+            // Only keep Type '...' is not assignable to type '...' in
+            // Type Bindings<...> is not assignable to type Bindable<...>, Type Binding<...> is not assignable to type Bindable<...>, Type '...' is not assignable to type '...'
+            const lines = diagnostic.message.split('\n');
+            if (lines.length === 3) {
+                diagnostic.message = lines[2].trimStart();
+            }
+        }
+
+        return {
+            ...diagnostic,
+            message: diagnostic.message
+        };
+    }
+
     return diagnostic;
 }
 

diff --git a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts
@@ -37,6 +37,9 @@ interface TsRenameLocation extends ts.RenameLocation {
     newName?: string;
 }
 
+const bind = 'bind:';
+const bindShortHandGeneratedLength = ':__sveltets_2_binding('.length;
+
 export class RenameProviderImpl implements RenameProvider {
     constructor(
         private readonly lsAndTsDocResolver: LSAndTSDocResolver,
@@ -73,7 +76,7 @@ export class RenameProviderImpl implements RenameProvider {
 
         const renameLocations = lang.findRenameLocations(
             tsDoc.filePath,
-            offset,
+            offset + (renameInfo.bindShorthand || 0),
             false,
             false,
             true
@@ -157,6 +160,7 @@ export class RenameProviderImpl implements RenameProvider {
     ):
         | (ts.RenameInfoSuccess & {
               isStore?: boolean;
+              bindShorthand?: number;
           })
         | null {
         // Don't allow renames in error-state, because then there is no generated svelte2tsx-code
@@ -165,6 +169,15 @@ export class RenameProviderImpl implements RenameProvider {
             return null;
         }
 
+        const svelteNode = tsDoc.svelteNodeAt(originalPosition);
+
+        let bindOffset = 0;
+        const bindingShorthand = this.getBindingShorthand(tsDoc, originalPosition, svelteNode);
+        if (bindingShorthand) {
+            bindOffset = bindingShorthand.end - bindingShorthand.start;
+            generatedOffset += bindShortHandGeneratedLength + bindOffset;
+        }
+
         const renameInfo = lang.getRenameInfo(tsDoc.filePath, generatedOffset, {
             allowRenameOfImportPath: false
         });
@@ -178,7 +191,6 @@ export class RenameProviderImpl implements RenameProvider {
             return null;
         }
 
-        const svelteNode = tsDoc.svelteNodeAt(originalPosition);
         if (
             isInHTMLTagRange(doc.html, doc.offsetAt(originalPosition)) ||
             isAttributeName(svelteNode, 'Element') ||
@@ -188,16 +200,22 @@ export class RenameProviderImpl implements RenameProvider {
         }
 
         // If $store is renamed, only allow rename for $|store|
-        if (tsDoc.getFullText().charAt(renameInfo.triggerSpan.start) === '$') {
+        const text = tsDoc.getFullText();
+        if (text.charAt(renameInfo.triggerSpan.start) === '$') {
             const definition = lang.getDefinitionAndBoundSpan(tsDoc.filePath, generatedOffset)
                 ?.definitions?.[0];
-            if (definition && isTextSpanInGeneratedCode(tsDoc.getFullText(), definition.textSpan)) {
+            if (definition && isTextSpanInGeneratedCode(text, definition.textSpan)) {
                 renameInfo.triggerSpan.start++;
                 renameInfo.triggerSpan.length--;
                 (renameInfo as any).isStore = true;
             }
         }
 
+        if (bindOffset) {
+            renameInfo.triggerSpan.start -= bindShortHandGeneratedLength + bindOffset;
+            (renameInfo as any).bindShorthand = bindShortHandGeneratedLength + bindOffset;
+        }
+
         return renameInfo;
     }
 
@@ -325,7 +343,6 @@ export class RenameProviderImpl implements RenameProvider {
             replacementsForProp,
             snapshots
         );
-        const bind = 'bind:';
 
         // Adjust shorthands
         return renameLocations.map((location) => {
@@ -351,31 +368,29 @@ export class RenameProviderImpl implements RenameProvider {
 
             const { parent } = snapshot;
 
-            let rangeStart = parent.offsetAt(location.range.start);
-            let suffixText = location.suffixText?.trimStart();
-
-            // suffix is of the form `: oldVarName` -> hints at a shorthand
-            if (!suffixText?.startsWith(':') || !getNodeIfIsInStartTag(parent.html, rangeStart)) {
-                return location;
-            }
-
-            const original = parent.getText({
-                start: Position.create(
-                    location.range.start.line,
-                    location.range.start.character - bind.length
-                ),
-                end: location.range.end
-            });
-
-            if (original.startsWith(bind)) {
+            const bindingShorthand = this.getBindingShorthand(snapshot, location.range.start);
+            if (bindingShorthand) {
                 // bind:|foo| -> bind:|newName|={foo}
+                const name = parent
+                    .getText()
+                    .substring(bindingShorthand.start, bindingShorthand.end);
                 return {
                     ...location,
                     prefixText: '',
-                    suffixText: `={${original.slice(bind.length)}}`
+                    suffixText: `={${name}}`
                 };
             }
 
+            let rangeStart = parent.offsetAt(location.range.start);
+
+            // suffix is of the form `: oldVarName` -> hints at a shorthand
+            if (
+                !location.suffixText?.trimStart()?.startsWith(':') ||
+                !getNodeIfIsInStartTag(parent.html, rangeStart)
+            ) {
+                return location;
+            }
+
             if (snapshot.getOriginalText().charAt(rangeStart - 1) === '{') {
                 // {|foo|} -> |{foo|}
                 rangeStart--;
@@ -582,8 +597,6 @@ export class RenameProviderImpl implements RenameProvider {
         renameLocations: TsRenameLocation[],
         snapshots: SnapshotMap
     ): TsRenameLocation[] {
-        const bind = 'bind:';
-
         return renameLocations.map((location) => {
             const sourceFile = lang.getProgram()?.getSourceFile(location.fileName);
 
@@ -625,6 +638,38 @@ export class RenameProviderImpl implements RenameProvider {
                         suffixText: '}'
                     };
                 }
+
+                if (snapshot.isSvelte5Plus) {
+                    const bindingShorthand = this.getBindingShorthand(
+                        snapshot,
+                        location.range.start
+                    );
+                    if (bindingShorthand) {
+                        const name = parent
+                            .getText()
+                            .substring(bindingShorthand.start, bindingShorthand.end);
+                        const start = {
+                            line: location.range.start.line,
+                            character: location.range.start.character - name.length
+                        };
+                        // If binding is followed by the closing tag, start is one character too soon,
+                        // else binding is ending one character too far
+                        if (parent.getText().charAt(parent.offsetAt(start)) === ':') {
+                            start.character++;
+                        } else {
+                            location.range.end.character--;
+                        }
+                        return {
+                            ...location,
+                            range: {
+                                start: start,
+                                end: location.range.end
+                            },
+                            prefixText: name + '={',
+                            suffixText: '}'
+                        };
+                    }
+                }
             }
 
             if (!prefixText || prefixText.slice(-1) !== ':') {
@@ -668,4 +713,17 @@ export class RenameProviderImpl implements RenameProvider {
             return location;
         });
     }
+
+    private getBindingShorthand(
+        snapshot: SvelteDocumentSnapshot,
+        position: Position,
+        svelteNode = snapshot.svelteNodeAt(position)
+    ) {
+        if (
+            svelteNode?.parent?.type === 'Binding' &&
+            svelteNode.parent.expression.end === svelteNode.parent.end
+        ) {
+            return svelteNode.parent.expression;
+        }
+    }
 }
diff --git a/packages/language-server/src/plugins/typescript/svelte-ast-utils.ts b/packages/language-server/src/plugins/typescript/svelte-ast-utils.ts
@@ -8,6 +8,7 @@ export interface SvelteNode {
     end: number;
     type: string;
     parent?: SvelteNode;
+    [key: string]: any;
 }
 
 type HTMLLike = 'Element' | 'InlineComponent' | 'Body' | 'Window';