svelteuidev / svelteui

SvelteUI Monorepo
https://svelteui.dev
MIT License
1.3k stars 62 forks source link

TypeScript errors for native HTML attributes #275

Closed JBloss1517 closed 1 year ago

JBloss1517 commented 1 year ago

What package has an issue

@svelteuidev/core

A clear and concise description of what the bug is

When I specify native HTML attributes on SvelteUI components, they work as expected, but the TS compiler always complains about them as you can see in the image below.

image

Is there a way to pass the native attributes up from the root component?

TS Error Message `Type '{ type: string; }' is not assignable to type '{ class?: string; variant?: ButtonVariant; color?: SvelteUIColor; size?: SvelteUINumberSize; radius?: number | SvelteUINumberSize; ... 28 more ...; pr?: (string & Record<...>) | SvelteUINumberSize; }'. Object literal may only specify known properties, and '"type"' does not exist in type '{ class?: string; variant?: ButtonVariant; color?: SvelteUIColor; size?: SvelteUINumberSize; radius?: number | SvelteUINumberSize; ... 28 more ...; pr?: (string & Record<...>) | SvelteUINumberSize; }'.`

In which browser(s) did the problem occur?

Chrome

Steps To Reproduce

Add any native HTML attributes on the components

Do you know how to fix the issue

No

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Relevant Assets

No response

drewbitt commented 1 year ago

I investigated and button types were removed in v0.6 https://github.com/svelteuidev/svelteui/commit/9afb1f0079d83a67556dae4e582d27abb37a0a91 and the current implementation of intrinsic element attributes will show an error if $$restProps is spread to a native HTML Element, which is not a good approach. To mitigate the error something like https://github.com/carbon-design-system/carbon-components-svelte/blob/3c04f122b0ff154eedbe45b51c2d9a4eeccca24b/types/Button/Button.Skeleton.d.ts#LL4C56-L4C56 should be done adding svelte.JSX.HTMLAttribute HTML Element tag name lists to the component types in svelteui

BeeMargarida commented 1 year ago

Yes, you are right, this is a current problem. I started solving it in another branch, but I'll move it out of there and open a PR with a fix. If you want and can, you can also submit the fix you think is best.

Thank you for this bug report

JBloss1517 commented 1 year ago

@BeeMargarida if you have already started it, no need to duplicate work. But let me know if I can help. Working from @drewbitt comment, it seems like it would simply be modifying the types on the components from something like... export interface ButtonProps extends DefaultProps<HTMLButtonElement | HTMLAnchorElement> to export interface ButtonProps extends DefaultProps<svelte.JSX.HTMLAttributes<HTMLButtonElement | HTMLAnchorElement> but I haven't tested it yet, so it might not be that easy 😅

drewbitt commented 1 year ago

Note, I just saw that two weeks ago svelte renamed svelte.JSX.HTMLElements into the new svelte/elements package

Migration to new typings

Seems like the way to go but would bump the core requirement of svelte to 3.55 minimum, svelte2tsx to 0.6 min (minimum required Node version to 16, minimum required TypeScript version is now 4.9 if this is indeed breaking as listed in svelte2tsx and not just a deprecation), etc

BeeMargarida commented 1 year ago

This can be fixed with the svelte.JSX.HTMLAttributes or the new svelte/elements, but in fact a new migration will have to be made to provide correct typings for each component (slots, events, props), following something like this: https://github.com/sveltejs/language-tools/blob/master/docs/preprocessors/typescript.md#typing-components-authoring-packages

I'll make a quick fix to fix these problems (using svelte.JSX.HTMLAttributes for now - it will be ported to svelte 3.55 in the future), but I'll start working on the thing mentioned in the paragraph above.

JBloss1517 commented 1 year ago

@BeeMargarida you're awesome! Thank you!

BeeMargarida commented 1 year ago

This should be totally fixed in the new release 0.9.0