formaat-design / reshaped

Community repository for storing examples, reporting issues and tracking roadmap
https://reshaped.so
97 stars 3 forks source link

Add undefined to <Badge> onDismiss Typescript type #252

Closed vladoyoung closed 1 month ago

vladoyoung commented 1 month ago

Is your feature request related to a problem? Please describe. <Badge> TS props do not support conditional onDismiss value.

Describe the solution you'd like Simply add undefined as an option in TS

Describe alternatives you've considered At the moment it works with the error. But would be nice to get rid of it.

Additional context https://codesandbox.io/p/sandbox/epic-hill-j49832 I've replicated the code from my application, but Codesandbox does not flag the TS error I'm getting in my editor.

In my case I want to display the dismiss option only when the user is in edit/interactive mode.

Here's the TS error from my editor:

image
vladoyoung commented 1 month ago

It actually showed up in Codesandbox:

image

blvdmitry commented 1 month ago

The codesandbox link doesn't open for me but the types are not exactly as you describe them here. There are multiple considerations we make:

type BaseProps = {
    size?: "small" | "medium" | "large";
    icon?: IconProps["svg"];
    endIcon?: IconProps["svg"];
    rounded?: boolean;
    hidden?: boolean;
    className?: G.ClassName;
} & Pick<ActionableProps, "href" | "onClick" | "attributes">;

type WithChildren = BaseProps & {
    children: React.ReactNode;
    color?: "critical" | "warning" | "positive" | "primary";
    variant?: "faded" | "outline";
};

type WithEmpty = BaseProps & {
    color: "critical" | "warning" | "positive" | "primary";
    children?: never;
    variant?: never;
};

type WithDismissible = {
    onDismiss: () => void;
    dismissAriaLabel: string;
};

type WithoutDismissible = {
    onDismiss?: never;
    dismissAriaLabel?: never;
};

Can you share your code snippet that causes the error? Might it be that you have an ariaLabel missing in that case or have the aria label defined but not the onDismiss function?

vladoyoung commented 1 month ago

I've fixed the share permissions for the Codesandbox. Can you try accessing it again?

Here are some screenshots too just in case:

image image

Now that I'm looking at it again, it seems like the error in the Codesandbox is different from the one from my editor (in the first comment), where it basically says that I can't have undefined in onDismiss

The problem here is not an error that stops the application from workings. It's a error/warning from TS.

What I'm suggesting is changing

type WithDismissible = {
    onDismiss: () => void;
    dismissAriaLabel: string;
};

to

type WithDismissible = {
    onDismiss: () => void | undefined;
    dismissAriaLabel: string;
};

I'm using a @ts-ignore for a fix for now

blvdmitry commented 1 month ago

Makes sense, what's actually causing the error is very strict types that expect the ariaLabel to only be passed if there a dismiss handler. Making it also possible to pass when your handler is undefined fixes your case with the following fix:

type WithoutDismissible = {
    onDismiss?: never;
    dismissAriaLabel?: string;
};

Going to release a patch later today

blvdmitry commented 1 month ago

This fix should be out now in 2.11.10, lmk if you face any additional edge cases with it