svelte-select icon indicating copy to clipboard operation
svelte-select copied to clipboard

Use generic types for component props declarations

Open bartektelec opened this issue 3 years ago • 5 comments

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.

bartektelec avatar Aug 09 '22 13:08 bartektelec

@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 avatar Aug 11 '22 03:08 rob-balfre

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

rob-balfre avatar Aug 11 '22 03:08 rob-balfre

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:

  • commit the types to Definitely typed (bad)
  • write JS doc (I never used it)
  • convert the lib to typescript (what I'd do)

Ennoriel avatar Aug 15 '22 14:08 Ennoriel

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

bartektelec avatar Aug 15 '22 20:08 bartektelec

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 ;)

Ennoriel avatar Aug 16 '22 17:08 Ennoriel