Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Added explicit undefined type to support exactOptionalPropertyTypes option #149

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

totto2727
Copy link
Contributor

@totto2727 totto2727 commented Apr 24, 2023

Description

When enabling exactOptionalPropertyTypes in tsconfig.json, a type error occurs and some features become unavailable.

Example: The following error occurs when passing a variable that can be undefined as an object in the argument of a cva component function.

components/ui/Button/index.tsx:41:50 - error TS2412: Type 'string | undefined' is not assignable to type 'undefined' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the type of the target.

41       className={buttonVariants({ variant, size, className })}
                                                    ~~~~~~~~~


Found 1 error in components/ui/Button/index.tsx:41

This also makes it difficult to handle defaultVariants.

components/ui/Button/index.tsx:41:33 - error TS2345: Argument of type '{ variant: "default" | "destructive" | "outline" | "secondary" | "ghost" | "link" | null | undefined; size: "default" | "sm" | "lg" | null | undefined; }' is not assignable to parameter of type '(ConfigVariants<{ variant: { default: string; destructive: string; outline: string; secondary: string; ghost: string; link: string; }; size: { default: string; sm: string; lg: string; }; }> & ClassProp) | undefined'.
  Type '{ variant: "default" | "destructive" | "outline" | "secondary" | "ghost" | "link" | null | undefined; size: "default" | "sm" | "lg" | null | undefined; }' is not assignable to type 'ConfigVariants<{ variant: { default: string; destructive: string; outline: string; secondary: string; ghost: string; link: string; }; size: { default: string; sm: string; lg: string; }; }> & { ...; }' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
    Type '{ variant: "default" | "destructive" | "outline" | "secondary" | "ghost" | "link" | null | undefined; size: "default" | "sm" | "lg" | null | undefined; }' is not assignable to type 'ConfigVariants<{ variant: { default: string; destructive: string; outline: string; secondary: string; ghost: string; link: string; }; size: { default: string; sm: string; lg: string; }; }>' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Types of property 'variant' are incompatible.
        Type '"default" | "destructive" | "outline" | "secondary" | "ghost" | "link" | null | undefined' is not assignable to type '"default" | "destructive" | "outline" | "secondary" | "ghost" | "link" | null'.
          Type 'undefined' is not assignable to type '"default" | "destructive" | "outline" | "secondary" | "ghost" | "link" | null'.

41       className={buttonVariants({ variant, size })}
                                   ~~~~~~~~~~~~~~~~~


Found 1 error in components/ui/Button/index.tsx:41
My Code
import type { VariantProps } from 'class-variance-authority'

import { forwardRef } from 'react'

import { cva } from 'class-variance-authority'

const buttonVariants = cva(
  'inline-flex items-center justify-center rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:opacity-50 disabled:pointer-events-none ring-offset-background',
  {
    variants: {
      variant: {
        default: 'bg-primary text-primary-foreground hover:bg-primary/90',
        destructive:
          'bg-destructive text-destructive-foreground hover:bg-destructive/90',
        outline:
          'border border-input hover:bg-accent hover:text-accent-foreground',
        secondary:
          'bg-secondary text-secondary-foreground hover:bg-secondary/80',
        ghost: 'hover:bg-accent hover:text-accent-foreground',
        link: 'underline-offset-4 hover:underline text-primary',
      },
      size: {
        default: 'h-10 py-2 px-4',
        sm: 'h-9 px-3 rounded-md',
        lg: 'h-11 px-8 rounded-md',
      },
    },
    defaultVariants: {
      variant: 'default',
      size: 'default',
    },
  }
)

export type ButtonProps = React.ButtonHTMLAttributes<HTMLButtonElement> &
  VariantProps<typeof buttonVariants>

const Button = forwardRef<HTMLButtonElement, ButtonProps>(
  ({ className, variant, size, ...props }, ref) => (
    <button
      className={buttonVariants({ variant, size })}
      ref={ref}
      {...props}
    />
  )
)
Button.displayName = 'Button'

export { Button, buttonVariants }

Explicitly adding undefined to some union types solved the problem.
fix: Added explicit undefined type to support exactOptionalPropertyTy…

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Follow the Style Guide.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).

@vercel
Copy link

vercel bot commented Apr 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 10:43am
example-astro-with-tailwindcss ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 10:43am
example-react-with-css-modules ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 10:43am
example-react-with-tailwindcss ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 10:43am
example-svelte ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 10:43am
example-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 10:43am

@joe-bell
Copy link
Owner

Thank you so much! This is something I've been meaning to tackle for some time. I'll take a deeper look soon 🙏

@joe-bell
Copy link
Owner

joe-bell commented Apr 26, 2023

I'll take care of commitlint 🫡

Copy link
Owner

@joe-bell joe-bell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legend, thanks again 🔥

@joe-bell joe-bell merged commit c88e3ec into joe-bell:main Apr 26, 2023
sebald referenced this pull request in sebald/pattern-analyzer May 11, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [class-variance-authority](https://github.com/joe-bell/cva) |
[`0.5.2` ->
`0.6.0`](https://renovatebot.com/diffs/npm/class-variance-authority/0.5.2/0.6.0)
|
[![age](https://badges.renovateapi.com/packages/npm/class-variance-authority/0.6.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/class-variance-authority/0.6.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/class-variance-authority/0.6.0/compatibility-slim/0.5.2)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/class-variance-authority/0.6.0/confidence-slim/0.5.2)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>joe-bell/cva</summary>

### [`v0.6.0`](https://github.com/joe-bell/cva/releases/tag/v0.6.0)

[Compare
Source](https://github.com/joe-bell/cva/compare/v0.5.3...v0.6.0)

#### What's Changed

- `cx` → `clsx` by [@&#8203;joe-bell](https://github.com/joe-bell) in
[https://github.com/joe-bell/cva/pull/152](https://github.com/joe-bell/cva/pull/152)

**`cva` now uses `clsx` under-the-hood** to concatenate classes with
**no breaking changes** to the current experience and **no increase to
bundle-size**.

    The existing `cx` export still exists, but as an alias of `clsx`

    Bringing additional benefits of:

1. Provides additional support for booleans and variadic strings within
`class` or `className` props

        ```ts
const button = cva([true && "button-base", false && "not-rendered"]);
        // => 'button-base'

const buttonConsumer = button({ class: [true && "extra-class"] });
        // => 'button-base extra-class'
        ```

2. Provides support for [object](https://github.com/lukeed/clsx#usage)
syntax within `class` or `className` props

        ```ts
        const button = cva({ foo: true, bar: false });
        // => 'foo baz'
        ```

**Full Changelog**:
joe-bell/cva@v0.5.3...v0.6.0

### [`v0.5.3`](https://github.com/joe-bell/cva/releases/tag/v0.5.3)

[Compare
Source](https://github.com/joe-bell/cva/compare/v0.5.2...v0.5.3)

#### What's Changed

- fix: issue [#&#8203;147](https://github.com/joe-bell/cva/issues/147)
where map files are not present for esm files by
[@&#8203;pfried](https://github.com/pfried) in
[https://github.com/joe-bell/cva/pull/148](https://github.com/joe-bell/cva/pull/148)

    > Thank you [@&#8203;pfried](https://github.com/pfried)!

- fix: Added explicit undefined type to support
exactOptionalPropertyTypes option by
[@&#8203;totto2727](https://github.com/totto2727) in
[https://github.com/joe-bell/cva/pull/149](https://github.com/joe-bell/cva/pull/149)

> This has been a long standing issue for me and I'm **so** grateful to
[@&#8203;totto2727](https://github.com/totto2727) for making the fix

#### New Contributors

- [@&#8203;pfried](https://github.com/pfried) made their first
contribution in
[https://github.com/joe-bell/cva/pull/148](https://github.com/joe-bell/cva/pull/148)
- [@&#8203;totto2727](https://github.com/totto2727) made their first
contribution in
[https://github.com/joe-bell/cva/pull/149](https://github.com/joe-bell/cva/pull/149)

**Full Changelog**:
joe-bell/cva@v0.5.2...v0.5.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/sebald/pattern-analyzer).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS42Ni4xIiwidXBkYXRlZEluVmVyIjoiMzUuNjYuMyIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants