nextui-org / tailwind-variants

🦄 Tailwindcss first-class variant API
https://tailwind-variants.org
MIT License
2.42k stars 68 forks source link

Required slots #171

Closed DamianGlowala closed 8 months ago

DamianGlowala commented 8 months ago

Currently, there doesn't seem to be a way (or it isn't documented yet) to enforce specific set of slots on the type level.

E.g. given the below type:

export type Section =
    | "outer"
    | "wrapper"
    | "label";

I'd like to be able to somehow make tv fn aware of them and enforce their presence (+ provide autocompletion).

mskelton commented 8 months ago

I'm not sure I understand the use case. Why do you need to specify they are required? Isn't that the same difference as just specifying the slots?

DamianGlowala commented 8 months ago

That's the point - enforcing what I need to specify based on an external type holding the names of the slots.

Simplest analogical example would be using Record<Section, string> instead of Record<string, string> (TypeScript is preventing me here from forgetting sections, typos, or adding unneeded ones - lots of benefits).

LMK if this makes any sense! 😄

DamianGlowala commented 8 months ago

That's what I'd imagine: tv<Section>({ slots: { <autocompleted keys based on Section type> } })

... and this is the root of this change within the codebase (generic KeyT rather than a plain string in the Record type): type TVSlots<KeyT extends string> = Record<KeyT, ClassValue> | undefined;

mskelton commented 8 months ago

I really don't see the value here. If you don't define a slot, when you use the returned result and try to use a slot value that doesn't exist, TypeScript will warn. Specifying the types, then specifying them in the actual tv call just doesn't make sense and complicates the types more than they need to be.

Closing as not planned.

DamianGlowala commented 8 months ago

I have to disagree, even though I understand where you're coming from. My suggestion would be beneficial when the source of thruth for the slots of a component lives outside of tv. Otherwise, I have duplicated source of thruth which I have to manually synchronise (which in turn is error prone).

The Section type I mentioned above is used in a few places, not purely for the usage within tv. This is the key point.

However, I respect your point of view and if this still doesn't sound like a desirable addition, let this issue remain closed.

DamianGlowala commented 8 months ago

Current solution:

tv({
  slots: {
    ...
  } satisfies Record<Section, ClassValue>
})