-
Notifications
You must be signed in to change notification settings - Fork 942
fix(Icon): improve name type
#5498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit: |
|
Do you have an example of the object you're passing so I can try? The |
name type
|
E.g. something like this: <script setup lang="ts">
import { LucideArchive } from 'lucide-vue-next';
</script>
<template>
<UButton :icon="LucideArchive"/>
</template>Anything really imported from 'lucide-vue-next' and passed down to |
|
Maybe just adding |
| export interface IconProps { | ||
| name: string | object | ||
| name: string | any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export interface IconProps { | |
| name: string | object | |
| name: string | any | |
| import type { Component } from 'vue' | |
| export interface IconProps { | |
| name: string | Component |
The type definition uses any instead of properly supporting functional components, which disables TypeScript type checking and doesn't match the PR description that states it should "accept Function".
View Details
Analysis
Icon component type definition loses TypeScript type safety
What fails: IconProps.name type definition uses string | any instead of string | Component, which disables TypeScript type checking and allows invalid values like numbers and booleans to be passed without errors.
How to reproduce:
import type { Component } from 'vue'
// Current broken implementation (string | any)
interface BrokenIconProps {
name: string | any
}
// This incorrectly passes type checking:
const badProps: BrokenIconProps = { name: 123 } // Should error
const badProps2: BrokenIconProps = { name: true } // Should error
// Fixed implementation (string | Component)
interface FixedIconProps {
name: string | Component
}
// These correctly fail type checking:
// @ts-expect-error
const goodProps: FixedIconProps = { name: 123 } // Error: Type 'number' not assignableResult with current any type: TypeScript allows any value to be assigned to name without warnings, including 123, true, or other invalid values.
Expected behavior: TypeScript should reject non-component values. Using string | Component (from Vue's official type exports) provides proper type safety for component definitions.
Note: The component logic at lines 25-26 correctly handles both strings and components at runtime with typeof name === 'string' checks, but TypeScript type safety was removed when the prop type was changed from string | Component (commit 96dbce7) to string | any (commit 3dc5ab9). This fix restores proper type checking while maintaining backward compatibility with the existing runtime behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll put back Component when possible!
π Linked issue
β Type of change
π Description
Quickfix
IconPropstype so that Vue won't overload the console with complains whenIcongets a functional component (e.g. from Lucide):Tested locally, the warning is gone now.
π Checklist