language-tools icon indicating copy to clipboard operation
language-tools copied to clipboard

Improve DX for extending HTML elements with Typescript props

Open dan-cooke opened this issue 2 years ago • 11 comments

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

  • export let variant: $$Props['variant'] - prevents having to type Type defintiions twice, but still not 100% amazing solution, as you still have to type that out for every prop you are adding

Importance

nice to have

dan-cooke avatar May 16 '23 10:05 dan-cooke

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

PuruVJ avatar May 16 '23 10:05 PuruVJ

@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

dan-cooke avatar May 16 '23 10:05 dan-cooke

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? 😅

PuruVJ avatar May 16 '23 10:05 PuruVJ

@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

dan-cooke avatar May 16 '23 10:05 dan-cooke

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

PuruVJ avatar May 16 '23 10:05 PuruVJ

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

dan-cooke avatar May 16 '23 10:05 dan-cooke

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.

dummdidumm avatar May 16 '23 10:05 dummdidumm

@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!

dan-cooke avatar May 16 '23 14:05 dan-cooke

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: .. }

dummdidumm avatar May 16 '23 14:05 dummdidumm

@dummdidumm you are totally right, it does seem weird

dan-cooke avatar May 16 '23 17:05 dan-cooke

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.

carstenjaksch avatar Nov 08 '23 15:11 carstenjaksch