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

[Select] property 'SelectDisplayProps' type is too restrictive: should allow data-* attributes #34154

Closed
2 tasks done
teetotum opened this issue Aug 31, 2022 · 3 comments
Closed
2 tasks done
Labels
component: select This is the name of the generic UI component, not the React module! external dependency Blocked by external dependency, we can’t do anything about it typescript waiting for 👍 Waiting for upvotes

Comments

@teetotum
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

I'm on version 5.5.1 (according to my package-lock.json)

TypeScript will complain when I try to set data-testid via SelectDisplayProps.
Currently I need to silence TypeScript; either like this:

<Select
    SelectDisplayProps={{ ['data-testid']: 'my-id' } as any}
/>

or like in the first example of this issue

<Select
    SelectDisplayProps={{
        // @ts-ignore
        "data-testid": "demo-simple-select-helper-data-testid"
    }}
/>

Expected behavior 🤔

It should be possible to set any data-* attributes without getting a TypeScript error

<Select
    SelectDisplayProps={{ ['data-testid']: 'my-id' }}
/>

Steps to reproduce 🕹

No response

Context 🔦

No response

Your environment 🌎

No response

@teetotum teetotum added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 31, 2022
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Aug 31, 2022

There is similar issue like this in #18874. It's a limitation of TypeScript. Upvote microsoft/TypeScript#28960 to get this issue fixed.

You can also extend the SelectDisplayProps as shown in StackOverflow example.

@ZeeshanTamboli ZeeshanTamboli added external dependency Blocked by external dependency, we can’t do anything about it component: select This is the name of the generic UI component, not the React module! typescript and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 31, 2022
@teetotum
Copy link
Author

teetotum commented Aug 31, 2022

But shouldn't the SelectDisplayProps type rather be defined to be a combination of React.HTMLAttributes<HTMLDivElement> with some HTMLDataAttributes type in the material-ui d.ts file? So to allow those attributes?
Like this:

type DataAttributeKey = `data-${string}`;
type HTMLDataAttributes = Record<DataAttributeKey, string>;

SelectDisplayProps?: React.HTMLAttributes<HTMLDivElement> & HTMLDataAttributes;

@ZeeshanTamboli
Copy link
Member

But shouldn't the SelectDisplayProps type rather be defined to be a combination of React.HTMLAttributes<HTMLDivElement> with some HTMLDataAttributes type in the material-ui d.ts file? So to allow those attributes? Like this:

type DataAttributeKey = `data-${string}`;
type HTMLDataAttributes = Record<DataAttributeKey, string>;

SelectDisplayProps?: React.HTMLAttributes<HTMLDivElement> & HTMLDataAttributes;

@teetotum It's a good proposal👍. But template literal types — data-${string} — are available since TypeScript v4.4 and the minimum version of TypeScript that MUI requires is 3.5 (docs). It may throw an error for users who use TypeScript version less than 4.4 with MUI.

I'll add "waiting for upvotes" label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! external dependency Blocked by external dependency, we can’t do anything about it typescript waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

2 participants