(fix) better error message for missing type definition (#1360)

#1352

Author: dummdidumm

packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts

@@ -194,24 +194,25 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
         const start = fragment.offsetAt(fragment.getGeneratedPosition(range.start));
         const end = fragment.offsetAt(fragment.getGeneratedPosition(range.end));
         const errorCodes: number[] = context.diagnostics.map((diag) => Number(diag.code));
-        const codeFixes = lang.getCodeFixesAtPosition(
-            tsDoc.filePath,
-            start,
-            end,
-            errorCodes,
-            {},
-            userPreferences
-        );
-
-        const componentQuickFix = errorCodes.includes(2304) // "Cannot find name '...'."
-            ? this.getComponentImportQuickFix(start, end, lang, tsDoc.filePath, userPreferences) ??
-              []
-            : [];
+        let codeFixes = errorCodes.includes(2304) // "Cannot find name '...'."
+            ? this.getComponentImportQuickFix(start, end, lang, tsDoc.filePath, userPreferences)
+            : undefined;
+        codeFixes =
+            // either-or situation
+            codeFixes ||
+            lang.getCodeFixesAtPosition(
+                tsDoc.filePath,
+                start,
+                end,
+                errorCodes,
+                {},
+                userPreferences
+            );
 
         const docs = new SnapshotFragmentMap(this.lsAndTsDocResolver);
         docs.set(tsDoc.filePath, { fragment, snapshot: tsDoc });
 
-        const codeActionsPromises = codeFixes.concat(componentQuickFix).map(async (fix) => {
+        const codeActionsPromises = codeFixes.map(async (fix) => {
             const documentChangesPromises = fix.changes.map(async (change) => {
                 const { snapshot, fragment } = await docs.retrieve(change.fileName);
                 return TextDocumentEdit.create(
@@ -311,7 +312,7 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
         lang: ts.LanguageService,
         filePath: string,
         userPreferences: ts.UserPreferences
-    ): ts.CodeFixAction[] | undefined {
+    ): readonly ts.CodeFixAction[] | undefined {
         const sourceFile = lang.getProgram()?.getSourceFile(filePath);
 
         if (!sourceFile) {
@@ -326,7 +327,10 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
             },
             (node): node is ts.JsxOpeningLikeElement | ts.JsxClosingElement | ts.Identifier =>
                 this.configManager.getConfig().svelte.useNewTransformation
-                    ? ts.isNewExpression(node.parent) && ts.isIdentifier(node)
+                    ? ts.isCallExpression(node.parent) &&
+                      ts.isIdentifier(node.parent.expression) &&
+                      node.parent.expression.text === '__sveltets_2_ensureComponent' &&
+                      ts.isIdentifier(node)
                     : ts.isJsxClosingElement(node) || ts.isJsxOpeningLikeElement(node)
         );
 

packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts

@@ -267,16 +267,16 @@ function isNoJsxCannotHaveMultipleAttrsError(diagnostic: Diagnostic) {
  * Some diagnostics have JSX-specific nomenclature. Enhance them for more clarity.
  */
 function enhanceIfNecessary(diagnostic: Diagnostic): Diagnostic {
-    if (diagnostic.code === DiagnosticCode.CANNOT_BE_USED_AS_JSX_COMPONENT) {
+    if (
+        diagnostic.code === DiagnosticCode.CANNOT_BE_USED_AS_JSX_COMPONENT ||
+        (diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
+            diagnostic.message.includes('Svelte2TsxComponent'))
+    ) {
         return {
             ...diagnostic,
             message:
                 'Type definitions are missing for this Svelte Component. ' +
-                // eslint-disable-next-line max-len
-                "It needs a class definition with at least the property '$$prop_def' which should contain a map of input property definitions.\n" +
-                'Example:\n' +
-                '  class ComponentName { $$prop_def: { propertyName: string; } }\n' +
-                'If you are using Svelte 3.31+, use SvelteComponentTyped:\n' +
+                '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;}> {}\n\n' +
                 'Underlying error:\n' +

packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/component-invalid/expected.json

@@ -0,0 +1,13 @@
+[
+    {
+        "range": {
+            "start": { "line": 13, "character": 1 },
+            "end": { "line": 13, "character": 11 }
+        },
+        "severity": 1,
+        "source": "ts",
+        "message": "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;}> {}\n\nUnderlying error:\n'DoesntWork' cannot be used as a JSX component.\n  Its instance type 'DoesntWork' is not a valid JSX element.\n    Property '$$prop_def' is missing in type 'DoesntWork' but required in type 'ElementClass'.",
+        "code": 2786,
+        "tags": []
+    }
+]

packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/component-invalid/expectedv2.json

@@ -0,0 +1,13 @@
+[
+    {
+        "range": {
+            "start": { "line": 13, "character": 1 },
+            "end": { "line": 13, "character": 11 }
+        },
+        "severity": 1,
+        "source": "ts",
+        "message": "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;}> {}\n\nUnderlying error:\nArgument of type 'typeof DoesntWork' is not assignable to parameter of type 'new (args: { target: any; props?: any; }) => Svelte2TsxComponent<any, any, any>'.\n  Type 'DoesntWork' is missing the following properties from type 'Svelte2TsxComponent<any, any, any>': $$prop_def, $$events_def, $$slot_def, $on, and 5 more.",
+        "code": 2345,
+        "tags": []
+    }
+]

packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/component-invalid/imported.svelte

@@ -0,0 +1 @@
+<p>hi</p>

packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/component-invalid/input.svelte

@@ -0,0 +1,14 @@
+<script lang="ts">
+    import { SvelteComponentTyped } from "svelte";
+    import Imported from './imported.svelte';
+
+    class Works extends SvelteComponentTyped<any, any, any> {}
+    class DoesntWork {}
+</script>
+
+<!-- valid -->
+<Works />
+<Imported />
+
+<!-- invalid -->
+<DoesntWork />

packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/element-attributes.only/expected.json renamed to packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/element-attributes/expected.json

@@ -72,32 +72,32 @@ export class InlineComponent {
             this.addNameConstDeclaration = () =>
                 (this.startTransformation[0] = `{ const ${this._name} = __sveltets_2_createComponentAny({`);
             this.startEndTransformation.push('});');
-        } else if (this.node.name === 'svelte:component') {
-            this._name = '$$_sveltecomponent' + this.computeDepth();
-            this.startTransformation.push(
-                `{ const ${this._name}_ = new `,
-                [this.node.expression.start, this.node.expression.end],
-                '({ target: __sveltets_2_any(), props: {'
-            );
+        } else {
+            const isSvelteComponentTag = this.node.name === 'svelte:component';
             // We don't know if the thing we use to create the Svelte component with
             // is actually a proper Svelte component, which would lead to errors
             // when accessing things like $$prop_def. Therefore widen the type
             // here, falling back to a any-typed component to ensure the user doesn't
             // get weird follup-errors all over the place. The diagnostic error
-            // should still error on the new X() part.
-            this.startEndTransformation.push(`}});${this._name}_;`);
-            this.addNameConstDeclaration = () =>
-                (this.startEndTransformation[0] = `}});const ${this._name} = __sveltets_2_typeAsComponent(${this._name}_);`);
-        } else {
-            this._name = '$$_' + this.node.name + this.computeDepth();
-            const nodeNameStart = this.str.original.indexOf(this.node.name, this.node.start);
+            // will be on the __sveltets_2_ensureComponent part, giving a more helpful message
+            this._name =
+                '$$_' +
+                (isSvelteComponentTag ? 'sveltecomponent' : this.node.name) +
+                this.computeDepth();
+            const constructorName = this._name + 'C';
+            const nodeNameStart = isSvelteComponentTag
+                ? this.node.expression.start
+                : this.str.original.indexOf(this.node.name, this.node.start);
+            const nodeNameEnd = isSvelteComponentTag
+                ? this.node.expression.end
+                : nodeNameStart + this.node.name.length;
             this.startTransformation.push(
-                '{ new ',
-                [nodeNameStart, nodeNameStart + this.node.name.length],
-                '({ target: __sveltets_2_any(), props: {'
+                `{ const ${constructorName} = __sveltets_2_ensureComponent(`,
+                [nodeNameStart, nodeNameEnd],
+                `); new ${constructorName}({ target: __sveltets_2_any(), props: {`
             );
             this.addNameConstDeclaration = () =>
-                (this.startTransformation[0] = `{ const ${this._name} = new `);
+                (this.startTransformation[2] = `); const ${this._name} = new ${constructorName}({ target: __sveltets_2_any(), props: {`);
             this.startEndTransformation.push('}});');
         }
     }