-
Notifications
You must be signed in to change notification settings - Fork 304
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
refactor: use class
as prop to prevent class duplication with cn
function
#241
refactor: use class
as prop to prevent class duplication with cn
function
#241
Conversation
class
as prop to prevent class duplicationclass
as prop to prevent class duplication with cn
function
This PR Depends on this change 👇 in Please
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
import { AccordionContent, type AccordionContentProps } from 'radix-vue' | ||
import { cn } from '@/lib/utils' | ||
|
||
const props = defineProps<AccordionContentProps & { class?: string }>() | ||
const props = defineProps<AccordionContentProps & { class?: HTMLAttributes['class'] }>() |
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.
I worry this HTMLAttributes
would cause some issue for detypes
😂
@sadeghbarati do upgrade the |
I'm not sure whether this is the right place or not, but i found some bugs that I traced back to this commit. I'm using vue 3.3.8, haven't upgraded to vue 3.4 yet. So in SelectItem.vue, the problem is in this code
If I'm using radix-vue version 1.2.3, this breaks as the type of forwardedProps become CustomRefImpl when I console.log it, and the value of it becomes an object like this:
To get around this, I tested two solutions:
Also, whenever i add a component using |
close #35
close #310
This might take a bit longer cause some of the classes/styles changed in main shadcn-ui so I have to bring them in here too
Previously we used destructure
useAttrs
like 👇 , to fix classduplication
But destructure props or attrs will cause lose reactivity
We could make
attrs
reactive withcomputed
ortoRefs
,But making Always read-only (inside the component) thing, reactive/mutable doesn't make sense
Now we are using class in prop and delegated/rest
delegated/rest spread props
After this rfcs [Core Team RFC] Reactive Props Destructure vuejs/rfcs#502 comes out of experimental we will have this code
Details
And after complex types support for defineProps, we will have this, hopefully xd
Details
Complex types + generics (not real code 😂) for example take look at this react article
https://www.totaltypescript.com/pass-component-as-prop-react
Details