refactor(NcChip): properly document and type slots and emits #6812
Conversation
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
susnux
added
3. to review
| /** | ||
| * The actions slot can be used to add custom actions (`NcAction*`) to the chips actions. | ||
| */ | ||
| actions: Slot |
There was a problem hiding this comment.
Slot cannot be required
| actions: Slot | |
| actions?: Slot |
There was a problem hiding this comment.
Also, slots?. doesn't make sense, when the type is : Slot, because Slot cannot be null/undefined.
There was a problem hiding this comment.
actions?: Slot
Why? Should be pretty fine according to the documentation: https://vuejs.org/api/sfc-script-setup#defineslots
There was a problem hiding this comment.
Why?
Slot is () => VNode[] which cannot be undefined
It means that !!slots.actions is the same as true
Should be pretty fine according to the documentation: https://vuejs.org/api/sfc-script-setup#defineslots
Documentation is correct only for simple cases where slots object is never used, and when this type is only used for slot names and props.
It's probably done for simplicity or a possibility in the future to add slot content typing on compile-time.
Example:
const slots = defineSlots<{
foo(): any
}>()
// inferred as "true"
// should be "boolean"
const hasFoo = !!slots.foo
// Throws "slots.foo is not a function" because slots are never required in Vue
// Should be warned by TS
// Inferred as any
// Should be VNode[]
const fooContent = slots.foo()| */ | ||
| emit('close') | ||
| } | ||
| default: Slot |
There was a problem hiding this comment.
| default: Slot | |
| default?: Slot |
| * Make sure that the icon is not exceeding a height of `--chip-size`. | ||
| * For round icons a exact size of `var(--chip-size)` is recommended. | ||
| */ | ||
| icon: Slot |
There was a problem hiding this comment.
| icon: Slot | |
| icon?: Slot |
| const hasActions = () => Boolean(slots.actions?.()) | ||
| const hasIcon = () => Boolean(props.iconPath || props.iconSvg || !!slots.icon?.()) |
There was a problem hiding this comment.
The same as in previous components, we should not render slots several times, and we moved from default to icon slots in such cases to make it easy for component users to pass the slot dynamically. It is only a problem fo the default slot.
| const hasActions = () => Boolean(slots.actions?.()) | |
| const hasIcon = () => Boolean(props.iconPath || props.iconSvg || !!slots.icon?.()) | |
| const hasActions = () => !!slots.actions | |
| const hasIcon = () => Boolean(props.iconPath || props.iconSvg || !!slots.icon) |
☑️ Resolves
🏁 Checklist
stable8for maintained Vue 2 version or not applicable