sveltejs / language-tools

The Svelte Language Server, and official extensions which use it
MIT License
1.25k stars 199 forks source link

<svelte:element> parameter has an 'any' type with an inline handler #2003

Open AlbertMarashi opened 1 year ago

AlbertMarashi commented 1 year ago

Edit: I have hidden the old issue that was solved, but this thread contained two issues

Old issue

### Describe the bug Getting a typescript issue where `` is erroring when I use `href` attribute on an element that may either be an anchor link or a div. ``` Object literal may only specify known properties, and '"href"' does not exist in type 'HTMLAttributes'. ``` ![image](https://user-images.githubusercontent.com/17972275/209468174-ef40c062-cc10-455f-a421-271307cc5669.png) This causes no actual compilation errors, but it does appear as an error in the IDE which is annoying ### Reproduction https://github.com/AlbertMarashi/element-bug-1 ### Logs _No response_ ### System Info ```shell System: OS: macOS 12.6.2 CPU: (8) arm64 Apple M1 Pro Memory: 88.88 MB / 16.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 16.17.0 - /usr/local/bin/node npm: 8.15.0 - /usr/local/bin/npm Browsers: Chrome: 108.0.5359.124 Firefox: 105.0.1 Firefox Developer Edition: 109.0 Safari: 15.6.1 npmPackages: svelte: ^3.55.0 => 3.55.0 ``` ### Severity annoyance

I'm getting these issues with the e being untyped with svelte:elements

<svelte:element
    this={ unit ? "a" : "div" }
    class="paginator"
    class:active={ unit !== null }
    class:next={ direction === "next" }
    href={unit ? `/courses/${course.slug}/${unit.unit_slug}` : undefined}
    on:click={ pressed }
    on:keyup={ e => e.key === "Enter" && pressed() }>
...

Parameter 'e' implicitly has an 'any' type

https://github.com/sveltejs/language-tools/issues/2003#issuecomment-1537698051

msf-caesar commented 1 year ago

Remember that you can use the {#if } blocks for different cases, and mix some properties in this way. This will allow you to have more control when splitting your element and deciding how you want it to compile. Don't forget to keep this in mind.

AlbertMarashi commented 1 year ago

https://github.com/sveltejs/language-tools/issues/2003

The point of svelte:element is to remove the need for unnecessary {#if} blocks

yastanotheruser commented 1 year ago

This is an editor/language server issue. I replicated the same example using Vite and TypeScript and got no errors neither from the language server nor the build.

AlbertMarashi commented 1 year ago

I'm getting this issue in tonnes of components and it's quite annoying since it creates all of these unfixable typescript errors everywhere.

Valexr commented 1 year ago

https://github.com/sveltejs/language-tools/blob/master/docs/preprocessors/typescript.md#im-using-an-attributeevent-on-a-dom-element-and-it-throws-a-type-error

jasonlyu123 commented 1 year ago

You can fix the error if you type the tag variable as a literal type like

let tag: 'a' | 'button' = 'a'
AlbertMarashi commented 1 year ago

@dummdidumm could this get some attention please? I'm getting littered with tonnes of these errors across my project Object literal may only specify known properties, and '"href"' does not exist in type 'HTMLAttributes<any>'.

I feel like it's a common usecase to use svelte:elements to conditionally render an a tag or some other kind of element which responds to on:click events.

I'm also getting these issues with the e being untyped with svelte:elements

<svelte:element
    this={ unit ? "a" : "div" }
    class="paginator"
    class:active={ unit !== null }
    class:next={ direction === "next" }
    href={unit ? `/courses/${course.slug}/${unit.unit_slug}` : undefined}
    on:click={ pressed }
    on:keyup={ e => e.key === "Enter" && pressed() }>
...

Parameter 'e' implicitly has an 'any' type

Some related issues: https://github.com/sveltejs/language-tools/issues/1913 https://github.com/sveltejs/language-tools/issues/1977 https://github.com/sveltejs/svelte/pull/8335

jasonlyu123 commented 1 year ago

The Object literal may only specify known properties, and '"href"' does not exist in type 'HTMLAttributes<any>' error can be fixed if you use literal type for element tag. If it doesn't, You're likely still using svelte-check v2 or extension v106. Please update to the latest version.

As for the event handler, It seems like TypeScript can't infer it from the union signature. Not sure what we can do about it. But it works if you move it to the script tag with an implicit parameter type. TypeScript will error with the wrong parameter type but can't infer the exact type for an inline handler. Transferring because this can't be fixed in svelte/element but has something to do with the svelte2tsx internal helper.

AlbertMarashi commented 1 year ago

Thanks. Those are working now, but it would be nice to get the typing of events in svelte:elements in inline handlers.

dummdidumm commented 1 year ago

Minimum reproducible for the events issue:


type EventHandler<E extends Event = Event, T extends EventTarget = Element> =
(event: E & { currentTarget: EventTarget & T}) => any;

type A = {
    'on:click'?: EventHandler<Event,HTMLDivElement>;
}
type B = {
    'on:click'?: EventHandler<Event,HTMLAnchorElement>;
}

const a: A | B = {'on:click': e => e}
AlbertMarashi commented 11 months ago

Closing this because I think is a valid solution

let tag: 'a' | 'button' = 'a'
dummdidumm commented 9 months ago

Reopening because of https://github.com/sveltejs/language-tools/issues/2003#issuecomment-1655511426

AlbertMarashi commented 4 months ago

@dummdidumm, this also happens with non-standard HTML elements which are also valid HTML, eg:

<thread-row on:keypress={ e => ... }>

Parameter e implicitly has any type

jasonlyu123 commented 4 months ago

It's most likely because there isn't a type definition for the custom element and unrelated to this issue. You'll need to enhance the typing https://svelte.dev/docs/typescript#enhancing-built-in-dom-types. You can use the HTMLAttributes type for the custom element if it inherits an HTML element.

AlbertMarashi commented 4 months ago

@jasonlyu123

export interface SvelteHTMLElements {
    a: HTMLAnchorAttributes;
    abbr: HTMLAttributes<HTMLElement>;
    ...
    [name: string]: { [name: string]: any }; // this is really annoying
}

This part really messes with everything, why can't the catch all case simply derive from the base HTMLAttributes<HTMLElement> instead of being any because that creates a TONNE of issues with typing everywhere in my app because I use non-standard HTML elements - which are valid HTML..

What is a workaround here?

AlbertMarashi commented 4 months ago

I had to some wack hacky template string to get it to work

    interface SvelteHTMLElements {
        [name: `${string}${
            | "a"
            | "b"
            | "c"
            | "d"
            | "e"
            | "f"
            | "g"
            | "h"
            | "i"
            | "j"
            | "k"
            | "l"
            | "m"
            | "n"
            | "o"
            | "p"
            | "q"
            | "r"
            | "s"
            | "t"
            | "u"
            | "v"
            | "w"
            | "x"
            | "y"
            | "z"}`,
        ]: HTMLAttributes<HTMLElement>
    }
AlbertMarashi commented 4 months ago

It's most likely because there isn't a type definition for the custom element and unrelated to this issue. You'll need to enhance the typing https://svelte.dev/docs/typescript#enhancing-built-in-dom-types. You can use the HTMLAttributes type for the custom element if it inherits an HTML element.

I'm not using "custom elements", i'm just using non-standard HTML elements which are also valid HTML, why can't they by default derive their fields and options off HTMLAttributes<HTMLElement>?

export interface SvelteHTMLElements {
    a: HTMLAnchorAttributes;
    abbr: HTMLAttributes<HTMLElement>;
    ...
-   [name: string]: { [name: string]: any };
+   [name: string]: HTMLAttributes<HTMLElement>;
}

I believe this should be updated in the svelte code to be like so

jasonlyu123 commented 4 months ago

That won't work for SVG elements.

AlbertMarashi commented 4 months ago

@jasonlyu123 yeah with my workaround, sure, but I don't understand why the fallback case shouldn't be HTMLAttributes<HTMLElement> instead of { [name: string]: any } which creates a tonne of parameter has an 'any' type issues across the app when I use valid but non-standard HTML elements

jasonlyu123 commented 4 months ago

No. What I meant is that HTMLAttributes<HTMLElement> won't work for SVG. SVG elements have very different attributes.

AlbertMarashi commented 4 months ago

@jasonlyu123, why wouldn't SVG Elements have their own key in SvelteHTMLElements which would take precedence?

jasonlyu123 commented 4 months ago

The [name: string] is the part for all the rest. that may include SVG. The known SVG elements are listed. If you want non-standard HTML elements to be typed other people might want non-standard SVG to be typed. And since they're all unsupported the name is unknown and can't be written out to take precedence.

AlbertMarashi commented 4 months ago

image

This is extremely annoying and I feel like the correct behavior in this scenario is to default to HTMLElement because that's how browsers treat it regardless.

image

As you can see here, on the MDN docs SVG elements still extend from HTML elements, and inherit their interface & properties.

So I think turning them to HTMLAttributes<HTMLElement> still makes sense

dummdidumm commented 4 months ago

They don't extend from HTML elements, the extend from elements. Very different. The solution for this is to type your own, as explained here: https://svelte.dev/docs/typescript#enhancing-built-in-dom-types

AlbertMarashi commented 4 months ago

@dummdidumm does it make sense then to do:

interface SvelteHTMLElements {
  [name: string]: HTMLAttributes<Element>
  // or
  [name: string]: HTMLAttributes<Element> & { [key: string]: any }
}
dummdidumm commented 4 months ago

Element is not necessarily a HTMLElement, as such HTMLAttributes is wrong, because that's for everything that's from HTMLElement, not Element