Skip to content
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

Type issue on Nav component #349

Open
ivan-dalmet opened this issue May 4, 2023 · 11 comments
Open

Type issue on Nav component #349

ivan-dalmet opened this issue May 4, 2023 · 11 comments

Comments

@ivan-dalmet
Copy link
Member

image

@Junaid300
Copy link

Can you give little detail about this issue? You want us to add proper type for this instead of giving it type any?right? I think i can work on this issue.

@yoannfleurydev
Copy link
Member

Can you give little detail about this issue? You want us to add proper type for this instead of giving it type any?right? I think i can work on this issue.

Yes, that's it. We try to avoid any in the codebase.

@Junaid300
Copy link

Junaid300 commented May 10, 2023

Yes, it is better to remove any type from our code base. I spend sometime on it and i think we may need to adjust our Item according to the Types Flex or MenuItem. I tried a quick trick

const Item: typeof Flex = Flex

I start getting errors on our Item element , it means we need to check the Item element according to our types.
Correct me if i did something wrong

@Junaid300
Copy link

@yoannfleurydev , i am stuck with this issue can you assist me what can i do ? Can you give me some pointers for this issue?

@yoannfleurydev
Copy link
Member

@yoannfleurydev , i am stuck with this issue can you assist me what can i do ? Can you give me some pointers for this issue?

Hey sorry for the delay, I'm a bit busy at the moment, I'm probably gonna check that this week (maybe on Friday). I let you know what I'll find.

@Junaid300
Copy link

Junaid300 commented May 16, 2023

Hey, no worry. I explored the types little bit in depth and I got a clue. Flex is typed as ComponentWithAs<"div", FlexProps> and
MenuItem is typed as ComponentWithAs<"div",MenuItemProps>

According to me, this should work but it throw error

  const Item: ComponentWithAs<"div", FlexProps> | ComponentWithAs<"button", MenuItemProps> = isMenu ? MenuItem : Flex;

@yoannfleurydev
Copy link
Member

@Junaid300 sorry, got a lot on my plate with a huge Start UI PR and some improvements. I'm checking that tomorrow.

@yoannfleurydev
Copy link
Member

Hey, no worry. I explored the types little bit in depth and I got a clue. Flex is typed as ComponentWithAs<"div", FlexProps> and MenuItem is typed as ComponentWithAs<"div",MenuItemProps>

According to me, this should work but it throw error

  const Item: ComponentWithAs<"div", FlexProps> | ComponentWithAs<"button", MenuItemProps> = isMenu ? MenuItem : Flex;

I tried this solution too, but yeah, got Expression produces a union type that is too complex to represent. ts(2590). Because the union of those 2 types generates a type with too much attributes.

@Junaid300
Copy link

If we remove {...rest} from <Item>, it start working and when i consoled rest, it gives empty object. Weird..
I am not sure, may be we need to play with the Component props type too.

@Junaid300
Copy link

@yoannfleurydev , any update on this ?

@yoannfleurydev
Copy link
Member

@yoannfleurydev , any update on this ?

Hi, no sorry, no solution at the moment on that issue. But in my head, I think about a rewrite of how we handle this part to avoid strange types. Mixing MenuItem and Flex to me isn't a good idea anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants