joe-bell / cva

Class Variance Authority
https://cva.style
Apache License 2.0
5.78k stars 110 forks source link

I would like to ask why null was added back to VariantsConfig. #253

Closed YH-Seo closed 10 months ago

YH-Seo commented 10 months ago

Describe the bug

https://github.com/joe-bell/cva/commit/0683ee26442f6f0b2f3c964d5ff00f682127e8a0 In the above commit, the null type was added back to VariantsConfig at the same time as the test code was modified.

I am attaching a portion of the code where the problem occurs.

// define variants of Button
const ButtontVariants = cva(
  `
  ui-relative ui-box-border ui-rounded ui-outline-none ui-duration-500 active:ui-transition
`,
  {
    variants: {
      size: {
        small: `ui-px-[12px] ui-py-1`,
        medium: `ui-px-[12px] ui-py-[10px]`,
        large: `ui-px-[12px] ui-py-[14px]`,
      },
    },
  },
);

// define Button Component
export interface ButtonProps
  extends HTMLAttributes<HTMLButtonElement>,
    VariantProps<typeof ButtontVariants> {}

export function Button({
  size, // typeof size is... 'small' | 'medium' | 'large' | null | undefined
  ...buttonProps
}: ButtonProps): JSX.Element {
  ...
}

I define Variants of Button with cva and extend Button's props to VariantProps<typeof ButtonVariants> to receive this type in the component, but this is where the problem occurs.

In the example above, Button's props size, variant, and type always include null and undefined.

https://github.com/joe-bell/cva/commit/0683ee26442f6f0b2f3c964d5ff00f682127e8a0 Could you explain why null was added here?

Because of null, the type does not match, so there is an issue where null must be removed forcefully when implementing the component.

Thanks.

To Reproduce Copy the code above.

Expected behavior typeof size is... 'small' | 'medium' | 'large' | undefined

Screenshots image

Desktop (please complete the following information):

Smartphone (please complete the following information): .

Additional context .

joe-bell commented 10 months ago

It'll be removed in cva@beta 🙂

YH-Seo commented 10 months ago

@joe-bell I appreciate it.

Additionally, I found two StackOverflow articles that had the same questions as me. https://stackoverflow.com/questions/77357079/avoiding-null-value-in-storybook-controls https://stackoverflow.com/questions/77541927/class-variance-authority-variants-allows-null-value-but-shouldnt

I will share this good news with my friends. 😄

mcnaveen commented 9 months ago

Hi, Is this fixed already? I'm on v0.7.0 and I'm facing this issue.

joe-bell commented 9 months ago

https://github.com/joe-bell/cva/issues/253#issuecomment-1882982172