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

[Typescript] Adding component to the ListTypeMap interface #17646

Closed

Conversation

domodevelopment
Copy link

Fixes #17579.

@mui-pr-bot
Copy link

No bundle size changes comparing 05c533b...9f6b3a2

Generated by 🚫 dangerJS against 9f6b3a2

@joshwooding
Copy link
Member

Thanks, but pretty sure this isn't the solution. cc @eps1lon

@eps1lon
Copy link
Member

eps1lon commented Oct 1, 2019

There's a lot more involved here. First we would need to verify that this doesn't break createElement calls which means testing this approach in https://github.com/mui-org/material-ui/blob/master/packages/material-ui/test/typescript/OverridableComponent.spec.tsx. Then we need to be aware that this would fundamentally work different compared to createElement calls i.e. additional props from the component would be rejected/not type checked if passed to an *Props prop. This is an issue since it's highly confusing for anybody that isn't aware of the type implementation which is basically every library consumer.

I think for *Props we could implement polymorphic type checking as well but I haven't looked into it. I would appreciate if someone could look into this but I'd caution against expecting this takes no more than an hour.

@oliviertassinari oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI typescript and removed typescript labels Oct 9, 2019
@oliviertassinari
Copy link
Member

@domodevelopment Did you had the chance to look at the problem?

@oliviertassinari
Copy link
Member

@domodevelopment I'm closing, for now. Feel free to open a new pull request when you find some time :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Select] MenuListProps missing property 'component'
6 participants