benjamincanac / ui3

A UI Library for Modern Web Apps, powered by Vue & Tailwind CSS.
https://ui.nuxt.com
MIT License
54 stars 8 forks source link

fix: dynamic slots autocomplete #77

Closed plushdohn closed 4 months ago

plushdohn commented 4 months ago

๐Ÿ”— Linked issue

Fixes #5

โ“ Type of change

๐Ÿ“š Description

This PR adds type-safe autocomplete to the Tabs and Accordion dynamic slots. Like some other components, this requires the user to add as const in front of the slot's name when declaring the items.

Screenshot 2024-04-24 at 12 03 31

I tried to make the simplest type I could, but it's still more convoluted than I'd like. Unfortunately using the single generic T and its T['slots'] in the conditional type loses type-safety for some reason, so I had to add a second generic with the default value set to T['slots'].

Let me know if we want to extract this DynamicSlots type somewhere to be re-used and if I should make two separate PRs for each component instead of a single one.

I'm also not sure whether or not I should update the docs to include as const or if the docs for v3 are still the ones from v2.

Cheers!

๐Ÿ“ Checklist

plushdohn commented 4 months ago

Thanks for looking into this! Could we externalize the DynamicSlots type into types/utils.d.ts and do the same thing for Breadcrumb, DropdownMenu and NavigationMenu too? (They all have a slot?: string field in their items/links).

@benjamincanac I've applied the type to all components, but some casting was required to make the mapped proxySlots work in DropdownMenu. I tried redoing the type a few times but the compiler still wasn't happy.

Also now DynamicSlots takes care of mapping the arbitrary type (previously [key: string]: SlotProps<T>), I thought it makes the name more appropriate and makes it easier to reuse the type for all future components

benjamincanac commented 4 months ago

I've made a few changes inside your PR to improve slots examples but I don't get the autocomplete working ๐Ÿค”

plushdohn commented 4 months ago

I've made a few changes inside your PR to improve slots examples but I don't get the autocomplete working ๐Ÿค”

@benjamincanac did you add as const to the slot name?

benjamincanac commented 4 months ago

Oh no sorry, I've removed it I thought it was useless. There is no way to make it work without it?

benjamincanac commented 4 months ago

And also, don't worry about the docs its the old one from v2. We won't be able to run it until most of the @nuxt/ui-pro components are migrated as well.

plushdohn commented 4 months ago

Oh no sorry, I've removed it I thought it was useless. There is no way to make it work without it?

I don't think there is, as soon as you declare items without a const somewhere, Typescript decides that'll be a string forever ๐Ÿ™„

Screenshot 2024-04-24 at 18 37 45

But as const can also be put on the whole "item" object if one prefers:

Screenshot 2024-04-24 at 18 42 40

Apparently it doesn't work for the whole array as the components can't accept readonly props

benjamincanac commented 4 months ago

Thanks a lot!! ๐Ÿ˜Š