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

[material-ui][Typography] Enforce responsive typography type checking in sx prop #42945

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/mui-material/src/AlertTitle/AlertTitle.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { SxProps } from '@mui/system';
import { Theme, TypographyProps } from '..';
import { AlertTitleClasses } from './alertTitleClasses';

export interface AlertTitleProps extends TypographyProps<'div'> {
export interface AlertTitleProps extends Omit<TypographyProps<'div'>, 'sx'> {
/**
* The content of the component.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { OverrideProps, OverridableComponent } from '../OverridableComponent';
import { Theme } from '../styles';
import { DialogContentTextClasses } from './dialogContentTextClasses';

export interface DialogContentTextOwnProps extends Omit<TypographyTypeMap['props'], 'classes'> {
export interface DialogContentTextOwnProps
extends Omit<TypographyTypeMap['props'], 'classes' | 'sx'> {
/**
* Override or extend the styles applied to the component.
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/DialogTitle/DialogTitle.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Theme } from '../styles';
import { TypographyTypeMap } from '../Typography';
import { DialogTitleClasses } from './dialogTitleClasses';

export interface DialogTitleOwnProps extends Omit<TypographyTypeMap['props'], 'classes'> {
export interface DialogTitleOwnProps extends Omit<TypographyTypeMap['props'], 'classes' | 'sx'> {
/**
* The content of the component.
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/Link/Link.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Theme } from '../styles';
import { TypographyOwnProps } from '../Typography';
import { LinkClasses } from './linkClasses';

export interface LinkOwnProps extends DistributiveOmit<LinkBaseProps, 'classes'> {
export interface LinkOwnProps extends DistributiveOmit<LinkBaseProps, 'classes' | 'sx'> {
/**
* The content of the component.
*/
Expand Down
6 changes: 4 additions & 2 deletions packages/mui-material/src/Typography/Typography.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { OverridableStringUnion } from '@mui/types';
import { SxProps, SystemProps } from '@mui/system';
import { SxProps, SystemProps, Breakpoint } from '@mui/system';
import { Theme, TypeText } from '../styles';
import { OverrideProps, OverridableComponent } from '../OverridableComponent';
import { Variant } from '../styles/createTypography';
Expand Down Expand Up @@ -63,7 +63,9 @@ export interface TypographyOwnProps extends Omit<SystemProps<Theme>, 'color'> {
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx?: SxProps<Theme>;
sx?: SxProps<Theme> & {
typography?: string | Partial<Record<Breakpoint, TypographyProps['variant']>>;
};
/**
* Applies the theme typography styles.
* @default 'body1'
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/Typography/Typography.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ Typography.propTypes /* remove-proptypes */ = {
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx: PropTypes.oneOfType([
sx: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object, PropTypes.bool])),
PropTypes.func,
PropTypes.object,
Expand Down
3 changes: 3 additions & 0 deletions packages/mui-material/src/Typography/typography.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,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' } }} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@Sergio16T Sergio16T Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome news! Thank you so much!

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,5 @@ const theme = createTheme({
/* This variant is no longer supported */
// @ts-expect-error
<Typography variant="h3">h3</Typography>;

<Typography sx={{ typography: { md: 'poster', lg: 'h2' } }}>Custom variants</Typography>;