huntabyte / shadcn-svelte

shadcn/ui, but for Svelte. ✨
https://shadcn-svelte.com
MIT License
5.1k stars 313 forks source link

Type errors when using `exactOptionalPropertyTypes` #868

Closed Stadly closed 7 months ago

Stadly commented 7 months ago

Describe the bug

With Exact Optional Property Types enabled, the TypeScript checks fail for many components.

Let's take tooltip-content.svelte as an example.

<script lang="ts">
    import { Tooltip as TooltipPrimitive } from "bits-ui";
    import { cn, flyAndScale } from "$lib/utils";

    type $$Props = TooltipPrimitive.ContentProps;

    let className: $$Props["class"] = undefined;
    export let sideOffset: $$Props["sideOffset"] = 4;
    export let transition: $$Props["transition"] = flyAndScale;
    export let transitionConfig: $$Props["transitionConfig"] = {
        y: 8,
        duration: 150,
    };
    export { className as class };
</script>

<TooltipPrimitive.Content
    {transition}
    {transitionConfig}
    {sideOffset}
    class={cn(
        "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md",
        className
    )}
    {...$$restProps}
>
    <slot />
</TooltipPrimitive.Content>

$$Props["transition"] includes undefined, since transition is optional in $$Props. This means that transition can be undefined. This results in an error since <TooltipPrimitive.Content transition={undefined}> is not allowed.

I think the best solution might be to wrap the type with NonNullable:

    export let transition: NonNullable<$$Props["transition"]> = flyAndScale;

Reproduction

https://stackblitz.com/edit/github-ztpytl?file=src%2Flib%2Fcomponents%2Fui%2Ftooltip%2Ftooltip-content.svelte

Run npm run check. Then all components with the issue will be listed.

Logs

No response

System Info

System:
    OS: Linux 5.15 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
    Memory: 3.49 GB / 15.50 GB
    Container: Yes
    Shell: 5.2.15 - /bin/bash
  Binaries:
    Node: 20.11.1 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.4 - /usr/local/bin/npm
  npmPackages:
    @sveltejs/kit: ^2.5.0 => 2.5.0 
    bits-ui: ^0.19.5 => 0.19.5 
    formsnap: ^0.5.1 => 0.5.1 
    svelte: ^4.2.12 => 4.2.12 
    sveltekit-superforms: ^2.6.2 => 2.8.0

Severity

annoyance

huntabyte commented 7 months ago

This is not the best solution. What if you don't want a transition at all?

This can be handled locally in your project if important to you, but the defaults will stay as they are.

Stadly commented 7 months ago

That's a good point - <TooltipContent transition={undefined}> should of course be allowed. The problem is then that bits-ui does not allow transition to be undefined.

That can be changed here: https://github.com/huntabyte/bits-ui/blob/c245c996aa6f1779178560fc9cff16dfdef3a399/src/lib/internal/types.ts#L106

transition?: T; needs to be changed to transition?: T | undefined; for bits-ui to to allow transition={undefined} when exactOptionalPropertyTypes is turned on.

What do you think?