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

[AppBar] Fix type support of overridable component #25456

Merged
merged 5 commits into from Mar 31, 2021
Merged

[AppBar] Fix type support of overridable component #25456

merged 5 commits into from Mar 31, 2021

Conversation

heleg
Copy link
Contributor

@heleg heleg commented Mar 21, 2021

Added the OverridableComponent to AppBar.

Can somebody help me to understand why proptypes generator insistently removes component prop and @default annotation from js file? It's not easy to understand how typescript-to-proptypes and generateProptypes.ts works, so I gave up.

@mui-pr-bot
Copy link

mui-pr-bot commented Mar 21, 2021

No bundle size changes

Generated by 🚫 dangerJS against 0555248

@oliviertassinari
Copy link
Member

Can somebody help me to understand why proptypes generator insistently removes component prop and @default annotation from js file? It's not easy to understand how typescript-to-proptypes and generateProptypes.ts works, so I gave up.

Removing the component sounds correct. On the other hand, removing the default color value, seems to be unexpected. cc @eps1lon.

@heleg
Copy link
Contributor Author

heleg commented Mar 21, 2021

I just started to figure out how it works. Please help me to understand why yarn docs:api removed all the AppBar api docs. That's why CI didn't pass. It doesn't look like I changed anything that could make such thing.

@eps1lon eps1lon self-assigned this Mar 22, 2021
@oliviertassinari oliviertassinari added component: app bar This is the name of the generic UI component, not the React module! typescript labels Mar 27, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 27, 2021

I had a look. It seems that the generation scripts were failing because the types are wrong. I have pushed a commit to follow the other components in the codebase. Not sure if it's enough but it seems to work. I'm moving to a different effort. A thorough review would be great.

@oliviertassinari oliviertassinari changed the title [AppBar] [WIP] Fix type support of overridable component [AppBar] Fix type support of overridable component Mar 27, 2021
@oliviertassinari
Copy link
Member

I have rebased to leverage the latest fix Sebastian did.

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 31, 2021

I'm moving forward with it as the issue we asked the help of Sebastian for is no longer reproducing. We manage to get away without by using the standard way the component types in the rest of the codebase. We also have new test cases in case we need to further iterate.

@oliviertassinari oliviertassinari merged commit d55d273 into mui:next Mar 31, 2021
@heleg
Copy link
Contributor Author

heleg commented Mar 31, 2021

@oliviertassinari Sorry for not participating anymore.. Too many new things for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: app bar This is the name of the generic UI component, not the React module! typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants