rob-balfre / svelte-select

Svelte Select. A select component for Svelte
https://svelte-select-examples.vercel.app
Other
1.26k stars 175 forks source link

Use generic types for component props declarations #450

Closed bartektelec closed 11 months ago

bartektelec commented 1 year ago

I have been using this package for a while now, and really appreciate the work done so far. One of issues I find battling on daily basis is following strict TypeScript rules, where any isn't really wanted. Instead of doing this:

interface SelectProps {
  value?: any;
  items?: any[]
}

we could gain a lot from using a generic type

 interface SelectOption<T = number> {
    label: string;
    value: T;
    index: number
  }

  interface SelectProps<T extends SelectOption> {
    items: T[];
    value: T | null;
  }

the T type could be inherit to use in many other props like getSelectionLabel.

Also, I find getSelectionLabel is typed to always return a string, however it doesn't let you handle cases when option is null (cleared) eventhough it is handled in runtime.

I'd be happy to help, to hopefully include those improvements in v5.

rob-balfre commented 1 year ago

@bartektelec I have very little experience with TS so any pointers going forward would be great thanks. SvelteKit actually generates the type files for me. Feel free to raise PRs now as I'm mostly just bug fixing now. Thanks

rob-balfre commented 1 year ago

@bartektelec getSelectionLabel has been removed in favour of named slots in latest beta. Hopefully it's improvement! Thanks

Ennoriel commented 1 year ago

For the moment, the types seem to be asserted at compilation if the props have a default value but it might be too restrictive (an empty array will have the type any[] I guess).

I have been using an old v5 version for some time now and had difficulties to understand every type of an option/item in the list (it can be an object with different label/value attributes).

I could definitely help on that if you'd like (with the latest solution). There are different solutions:

bartektelec commented 1 year ago

I've done some research recently and apparently you can specify generic types by using interface $$Props and interface $$Slots naming, however I think there are still some issues when it comes to discriminated union types. I'm not sure if v5 does the same thing as v4, but in v4 every primitive type gets converted to

{label: string; index: number: value: T}

and any object stays in the same form.

Deploying a separate definitely typed @types/ package shouldn't be neccessary. What we do on @hurtigruten/svelte-table is we provide an additional d.ts file, separate to component that uses a js syntax. But converting to <script lang='ts'> is probably the best way to do it

Ennoriel commented 1 year ago

I just checked and it there is no generics in v4 nor in v5 but yes it should be doable.

I can take a try to migrate the lib to ts but it means that all future development and maintenance should be made in typescript. Are you okay with that @rob-balfre ?

I am already maintaining 2 small libs and I am willing to help on this one too ;)