-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
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
[styles] Improve component props inference of styled #20830
Conversation
Details of bundle changes.Comparing: 5627473...7b2b408 Details of page changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test to packages/material-ui-styles/src/styled/styled.spec.tsx
that would be failing without this change?
Yes sure. |
const renderedMyComponent = ( | ||
<React.Fragment> | ||
<MyComponent className="test" /> | ||
<StyledMyComponent theme={MyThemeInstance} /> | ||
<StyledInferedPropsMyComponent defaulted="Hi!" className="test" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an issue. className
isn't required. It should pass without it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you looked at source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have got about what you.
className
is required by MyComponentProps
. Non required className
is overridden by this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed it but I am not sure, should we change original props?
@vlazh Thanks |
These changes allow using props with type checking in
ComponentCreator
:Without that changes a ts error occurs:
Property 'status' does not exist on type '{ theme: DefaultTheme; }'.ts(2339)