-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
[core] Generify props with component property #16487
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
Conversation
No bundle size changes comparing fba5135...5bb044c |
[Avatar] Generify AvatarProps [BottomNavigationAction] Generify BottomNavigationActionProps [Button] Allow omitting type parameter of ButtonTypeMap [ButtonBase] Generify ButtonBaseProps [Breadcrumbs] Generify BreadcrumbsProps [ButtonGroup] Generify ButtonGroupProps [CardActionArea] Generify CardActionAreaProps [Chip] Generify ChipProps [Divider] Generify DividerProps [ExpansionPanelSummary] Generify ExpansionPanelSummaryProps [Fab] Generify FabProps [FormControl] Generify FormControlProps [FormLabel] Generify FormLabelProps [IconButton] Generify IconButtonProps [Link] Generify LinkProps [List] Generify ListProps [StepButton] Generify StepButtonProps [Tab] Generify TabProps [TablePagination] Generify TablePaginationProps [TableSortLabel] Generify TableSortLabelProps [TabScrollButton] Generify TabScrollButtonProps [Tabs] Generify TabsProps [ToggleButton] Generify ToggleButtonProps
9ce5662
to
19af011
Compare
I'm in doubt how to omit or override some propreties of D. For example, I have custom IconButton with integrated react-router Link component, and |
import { IconButtonTypeMap as MuiIconButtonTypeMap } from '@material-ui/core/IconButton'
import { ForwardRefLink } from '../compat/ForwardRefLink'
import { ExtendButtonBaseTypeMap } from '@material-ui/core/ButtonBase'
import { OverrideProps } from '@material-ui/core/OverridableComponent'
export type IconButtonTypeMap<
P = {},
D extends React.ElementType = MuiIconButtonTypeMap['defaultComponent']
> = ExtendButtonBaseTypeMap<{
props: P &
MuiIconButtonTypeMap['props'] & {
(custom props...)
}
defaultComponent: D
classKey: MuiIconButtonTypeMap['classKey']
}>
export type IconButtonPropsWithTo<P = {}> = OverrideProps<
IconButtonTypeMap<P & { to: LinkProps['to'] }, typeof ForwardRefLink>,
typeof ForwardRefLink
>
export type IconButtonPropsWithoutTo<P = {}> = OverrideProps<
IconButtonTypeMap<P>,
IconButtonTypeMap['defaultComponent']
>
export type IconButtonProps<P = {}> = IconButtonPropsWithoutTo<P & { to?: undefined }> | IconButtonPropsWithTo<P>
function IconButton(props: IconButtonPropsWithTo): JSX.Element
function IconButton(props: IconButtonPropsWithoutTo): JSX.Element
function IconButton(props: IconButtonProps) {
...
} hmm... looks ugly 😂 |
@ypresto tried conditional types? Example: type IconButtonProps<P> = P extends {to: string} ? A : B;
// more variants
type X<P> =
P extends X1 ? A :
P extends X2 ? B :
P extends X3 ? C :
D |
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.
Really appreciate that you pushed this effort. This looks good to me. Just would like to clarify the default types usage.
@ypresto Much appreciated, thanks. |
It seems that after this PR, <TextField onFocus={({ target }) => target.select()} /> This is failing now because the event is just a FYI: I'm using TS 3.6.2 |
@jraoult Please open a new issue and fill out the template. This would help a lot. |
This PR apply #15193 to all1 components not only Button. (See #15827 (comment))
It would fix component={...} prop type mismatch issue (#15827) when you use Props type and component props at same time (e.g.
(props: IconButtonProps) => <IconButtonProps {...props} component={...} />
).New generic parameters have default value to keep original (non-parametric) type.
(ListItem.d.ts is more difficult as it switches div and li by button flag, so kept unchanged.)
Edit @eps1lon:
Closes #16846
1