[material-ui][Typography] Enforce responsive typography type checking in sx prop #42945
Conversation
Netlify deploy previewhttps://deploy-preview-42945--material-ui.netlify.app/ Bundle size report |
|
Reviewed CI build step failure and am working to resolve. |
…Link] Update interfaces sx prop type to fix failing build. Addresses components that extend Typography.
…Link] prettier formatting
…Link] prettier formatting
…Link] Alert Title SX Prop Definition Update
…Link] proptypes docs:api
|
Addressed many of CI failures. Could use an assist with understanding why the test_static is failing with the following prompt: pnpm proptypes` changes committed? I ran pnpm proptypes && pnpm docs:api to generate changes and committed. |
ZeeshanTamboli
added
typescript
component: Typography
There was a problem hiding this comment.
@Sergio16T Thanks for the pull request. I made the necessary changes. We needed to omit the sx prop from other component prop types that extend Typography props. Additionally, we kept the sx propType in Typography as PropTypes.object to avoid a large change, which works well.
Looks good to me. @DiegoAndai @siriwatknp to give a final review.
|
@ZeeshanTamboli Thank you for the feedback on the PR! Makes sense about omitting the |
ZeeshanTamboli
added
the
needs cherry-pick
| @@ -55,6 +55,9 @@ const typographyTest = () => { | |||
| <Typography component={CustomComponent} prop1="1" /> | |||
| {/* @ts-expect-error */} | |||
| <Typography component={CustomComponent} prop1="1" prop2="12" /> | |||
| <Typography sx={{ typography: { xs: 'body1', sm: 'h2', md: 'h1', lg: 'body2' } }} /> | |||
| {/* @ts-expect-error */} | |||
| <Typography sx={{ typography: { xs: 'body 1', sm: 'h2', md: 'h1', lg: 'body1' } }} /> | |||
There was a problem hiding this comment.
I'm curious if we can eventually use custom typography variants if they are re-declared in d.ts files. E.g. will the code above throw an error with this overrides:
export interface CustomTypographyVariants {
['H1 title']?: React.CSSProperties;
['Body L']?: React.CSSProperties;
}
declare module '@mui/material/styles' {
interface TypographyVariants extends CustomTypographyVariants {}
interface TypographyVariantsOptions extends CustomTypographyVariants {}
}
declare module '@mui/material/Typography' {
interface TypographyPropsVariantOverrides {
['H1 title']: true;
['Body L']: true;
}
}Or in other words, will TS compiler be "okay" with body 1 if we declare it beforehand?
There was a problem hiding this comment.
Hello @goodwin64! I believe adding custom typography variants is supported.
I added module override to unit test to confirm this and also locally tested adding the above code block in typography.d.ts
Pushed up unit test update.
There was a problem hiding this comment.
Awesome news! Thank you so much!
There was a problem hiding this comment.
@Sergio16T Thanks for the test. We handle TypeScript module augmentation tests separately because they apply globally and need isolated testing. For custom Typography variants, there's already a test file. I added this case there instead.
Edit: Even custom breakpoints are supported.
|
@ZeeshanTamboli, @DiegoAndai, @siriwatknp Commenting to keep this PR active. Awaiting final review. |
Upon reflection, I believe this is a breaking change. If users extend |
ZeeshanTamboli
removed
the
needs cherry-pick
github-actions
bot
added
the
PR: out-of-date
Fixes #42918
Typography Prop Extended to Enforce Typography Breakpoint Types.