illright / attractions

A pretty cool UI kit for Svelte
https://illright.github.io/attractions
MIT License
1.03k stars 37 forks source link

Can't pass class props to some components #333

Closed epavanello closed 2 years ago

epavanello commented 2 years ago

Hi, great work for that library! I saw that I can't pass class prop to Chip component also if it's defined but I don't found in the soruces the cause 🤦🏽‍♂️ image

illright commented 2 years ago

Hi! Could you please paste the error message that your editor gives you when you try to pass this class? Off the top of my head, I see no reason why it shouldn't work.

Also, are you using TypeScript?

aabounegm commented 2 years ago

The issue seems to be with TypeScript typings, since the prop is defined. The library we use to generate the types (sveld) was not able to handle the way we define the class prop (exporting a renamed variable), but we will check if the issue is fixed or try to come up with a workaround as soon as we can.

epavanello commented 2 years ago

I've found the difference between the two components, the first works only because TextField inherits props from htmlInput and Chip doesn't inherit nothing. So, the bug seems to be the typescript type generator that ignore class props.

epavanello commented 2 years ago

It seems related to this issue:

https://github.com/carbon-design-system/sveld/issues/50

epavanello commented 2 years ago

Hi! Could you please paste the error message that your editor gives you when you try to pass this class? Off the top of my head, I see no reason why it shouldn't work.

Also, are you using TypeScript?

Now I'm not on the pc, the error is class prop missing, and the d.ts file generated associated hasn't that prop, seems a bug of sveld that ignore class props

aabounegm commented 2 years ago

It seems related to this issue:

carbon-design-system/sveld#50

seems a bug of sveld that ignore class props

That is indeed the issue. I tried it and it seems like Sveld still does not pick up on prop aliases, and I cannot think of an easy way around that. It also sometimes doesn't make sense to use {...$$restProps} in some components (especially ones with more than one root element), so this workaround may not always fit. Unfortunately, there is nothing we can do until this is fixed on their side, but I will leave the issue open as a reminder for when that happens.

epavanello commented 2 years ago

Soon I'll try to fix the sveld bug

aabounegm commented 2 years ago

@epavanello That's alright. I had some time and took a shot at it with https://github.com/carbon-design-system/sveld/pull/62 :)