sveltejs / language-tools

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

Improve DX for extending HTML elements with Typescript props #2016

Open dan-cooke opened 1 year ago

dan-cooke commented 1 year ago

Describe the problem

I am finding it quite awful when extending base HTML elements.

For example I want to create a custom <Button /> that has a single new prop called variant

Sofar I can see the only method for doing this is as follows:

<script lang="ts">
    // Import from elements package - nice so far
    import type { HTMLButtonAttributes } from 'svelte/elements';

    // Ok have to export this prop for consumers to use
   // Note the type annotation here too
    export let variant: 'primary' | 'secondary' = 'primary'

    // Okay... i have to extend this interface now to actually extend the buton props
   // but typescript complains so I have to redeclare the variant type here too
   // with the exact same type definition?
   // this just doubles the amount of code you have to write for thsi kind of thing

    interface $$Props extends Partial<HTMLButtonAttributes> {
        variant: 'primary' | 'secondary';
    }
</script >

I have only ever seen this example given when this question has come up on Discord, and on StackOverflow - I feel like there has to be a better way

Describe the proposed solution

If possible the Svelte compiler could combine the defined $$Props definition with everything that is exported from the component - that would completely solve this issue

To extend it would be amazing if you could just do something like

export let $$BaseProps: HTMLButtonAttributes;

Then all Svelte has to do is say

interface $$Props extends typeof $$BaseProps (HTMLButtonAttributes), OtherPropsExported {}

Alternatives considered

Importance

nice to have

PuruVJ commented 1 year ago

You can always do export let variant: $$Props['variant'] to avoid typing twice

dan-cooke commented 1 year ago

@PuruVJ good point - I will add this to alternatives considered actually - because that does solve 50% of the issue.. but its still not amazing that you have to type variant twice

PuruVJ commented 1 year ago

Hmm i kinda agree though. I've not used $$Props much, but whenever i have, I've always wished there was a $$RestProps. @dummdidumm too late to implement something like $$RestProps? 😅

dan-cooke commented 1 year ago

@PuruVJ forgive my ignorance.. but would a $$RestProps interface really solve the issue here?

That would still result in having to declare variant twice

interface $$RestProps {
   variant: 'primary' | 'secondary'
}

interface $$Props extends HTMLButtonAttributes {}

export let variant: $$RestProps['variant'];

Still not the ideal solution in my opinion - unless i'm misunderstanding your proposal

PuruVJ commented 1 year ago

With $$RestProps you shouldn't need to redeclare any props, it should be used just for extending from html attributes props, in theory

dan-cooke commented 1 year ago

@PuruVJ of couse - that would be amazing if it could be implemented!

dummdidumm commented 1 year ago

Inventing another interface convention is not the solution here. The solution here is to make tooling smarter that it knows that export let variant is of the defined typed without you needing to specify it. I started this in https://github.com/sveltejs/language-tools/pull/1212 a while ago but did run into some problems, so didn't finish yet.

dan-cooke commented 1 year ago

@dummdidumm I had originally suggested the opposite, ie. the language tools can combine $$Props with everything that is exported from the component by default

As opoosed to defining types in $$Props and then having to export them seperately as well.

For example


interface $$Props extends HTMLButtonAttributes {
    variant: 'primary' | 'secondary'
} 

// Inferred by the tool chain
export let variant

Is more boilerplate than just

interface $$Props extends HTMLButtonAttributes {}

export let variant: 'primary' | 'secondary'

But arguably its harder to reason about.

Either way - I am in favour of your solution for sure, I would be willing to have a look to help push it along!

dummdidumm commented 1 year ago

Semantically that does not make sense to me, because you are implementing an interface. The interface is what determines what the component can do. I think of

interface $$Props {..}

export let foo..;

as

interface $$Props {..}

const props: $$Props = { foo: .. }
dan-cooke commented 1 year ago

@dummdidumm you are totally right, it does seem weird

carstenjaksch commented 10 months ago

Excuse me if that is not the same issue, but it is another use case I think. Here is my code:

<svelte:element
    this={tag}
    role="button"
    tabindex="0"
    {href}
    on:click
>
    <slot />
</svelte:element>

TypeScript doesn't like the href attribute on here:

Argument of type '{ role: "button"; tabindex: number; href: string | undefined; "on:click": undefined; }' is not assignable to parameter of type 'HTMLAttributes<any>'.
  Object literal may only specify known properties, and 'href' does not exist in type 'HTMLAttributes<any>'.ts(2345)

I have tried to extend HTMLAttributes with an optional href attribute, but I don't know how to use this new type on svelte:element.

import type { HTMLAttributes } from 'svelte/elements';
interface ButtonAttributes extends HTMLAttributes<any> {
    href?: string;
}

I feel there is some documentation missing on how to extend such types and/or the DX should really be improved here.