Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Type Conflict with title prop in ListItem Component #159

Closed
5 of 6 tasks
jeffreyzli opened this issue Aug 5, 2023 · 0 comments
Closed
5 of 6 tasks

Type Conflict with title prop in ListItem Component #159

jeffreyzli opened this issue Aug 5, 2023 · 0 comments
Labels
React Issue with Tailwind Mobile React

Comments

@jeffreyzli
Copy link

Check that this is really a bug

  • I confirm

Reproduction link

https://stackblitz.com/edit/konsta-react-euvdy9?file=App.tsx

Bug description

Hello,

I've been working with the ListItem component in your KonstaUI library, and I've come across a type conflict that I believe may be a bug.

According to your documentation, the title prop for the ListItem component can take a string | number | React.ReactNode. However, when I pass a ReactNode to the title prop, I get the following TypeScript error:

Type 'Element' is not assignable to type 'string'.ts(2322)
index.d.ts(1943, 9): The expected type comes from property 'title' which is declared here on type 'IntrinsicAttributes & ListItemProps'

I noticed that ListItemProps extends React.HTMLAttributes, and that the title property in React.HTMLAttributes refers to the global HTML attribute title, which is of type string.

Here's the relevant part of the type definition:

interface ListItemProps {}
interface ListItemProps extends React.HTMLAttributes<HTMLElement> {}
interface ListItemProps extends Props {}

declare const ListItem: React.FunctionComponent<ListItemProps>;

It seems that there is a conflict between the title prop defined in Props (which can be string | number | React.ReactNode), and the title prop inherited from React.HTMLAttributes (which should be a string).

As a workaround, I considered using Omit to create a type that excludes the title property from React.HTMLAttributes, and then add a title property with the correct type to Props:

export interface Props extends Omit<React.HTMLAttributes<HTMLElement>, 'title'> {
    ...
    title?: string | number | React.ReactNode;
    ...
}

However, this feels more like a temporary fix than a proper solution. I think it would be best if the library could be refactored to avoid the name collision.

I hope this helps and look forward to hearing your thoughts.

Expected Behavior

Title should accept string | number | ReactNode, not just string.

The ListItem type definition is as follows:

  /**
   * Content of the list item "title" area
   */
  title?: string | number | React.ReactNode;

Actual Behavior

Any value that's not a string causes a type error for the title field. (This doesn't apply to the subtitle field, which has the same type within ListItem type definition, leading me to believe it's a type conflict with HTMLElement.

Konsta UI version

2.0.0

Platform/Target and Browser Versions

macOS

Validations

  • Follow our Code of Conduct
  • Read the docs.
  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.
  • Make sure this is a Konsta UI issue and not a framework-specific issue

Would you like to open a PR for this bug?

  • I'm willing to open a PR
@jeffreyzli jeffreyzli added the React Issue with Tailwind Mobile React label Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
React Issue with Tailwind Mobile React
Projects
None yet
Development

No branches or pull requests

1 participant