Material-ui: Typescript Definition Updates in 4.9.0: potentially incorrect

[ririvas]

Hello,

I recently pulled the latest @material-ui/core package, version 4.9.0 and my typescript compilation process began to fail. Specifically, my custom Theme override.

The media query definitions failed.

mixins: {
            toolbar: {
                background: '#fff',
                minHeight: 36,
                '@media (min-width:0px) and (orientation: landscape)': {
                    minHeight: 24
                },
                '@media (min-width:600px)': {
                    minHeight: 48
                }
            },
        },

Perhaps we deprecated this functionality but the docs say otherwise. And the release notes don't comment on this change.

For this particular issue, I can track it to the change #19263 . And specifically, this file: https://github.com/mui-org/material-ui/commit/d30e6bbb1083500d2d88d1d619060c857e5c501e#diff-353a3bd7b4e96490bd34f7442f3e08c6

Here is a codesandbox demo illustrating the type error.

[eps1lon]

Perhaps we deprecated this functionality but the docs say otherwise. And the release notes don't comment on this change.

It wasn't clear in #19263 if people actually make use of it. We never got any feedback on this via types, tests or docs contributions. Seems like didn't cover all our docs with types.

I will check out if we can safely widen the type.

[jameschensmith]

Right there with @ririvas. The fixed app bar solution using mixins doesn't even work anymore.

[jologz]

I have a simple setup:

That is now giving me this error:

 Overload 1 of 2, '(style: Styles<Theme, {}, "toolbarMargin" | "myDrawer">, options?: Pick<WithStylesOptions<Theme>, "flip" | "element" | "defaultTheme" | ... 
6 more ... | "classNamePrefix"> | undefined): (props?: any) => Record<...>', gave the following error.
    Argument of type '(theme: Theme) => 
{ toolbarMargin: React.CSSProperties; myDrawer: { width: number; }; }' is not assignable to parameter of type 'Styles<Theme, {}, "toolbarMargin" | "myDrawer">'.      Type '(theme: Theme) => { toolbarMargin: React.CSSProperties; myDrawer: { 
width: number; }; }' is not assignable to type 'StyleRulesCallback<Theme, {}, "toolbarMargin" | "myDrawer">'.
        Call signature return types '{ toolbarMargin: CSSProperties; myDrawer: { width: number; }; }' and 'Record<"toolbarMargin" | "myDrawer", CSSProperties | 
CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)>' are incompatible.
          The types of 'toolbarMargin' are incompatible between these types.    
            Type 'CSSProperties' is not 
assignable to type 'CSSProperties | CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)'.
              Type 'CSSProperties' is not assignable to type 'CreateCSSProperties<{}>'.
                Index signature is missing in type 'CSSProperties'.
  Overload 2 of 2, '(styles: Styles<Theme, {}, "toolbarMargin" | "myDrawer">, options?: Pick<WithStylesOptions<Theme>, "flip" | "element" | "defaultTheme" | ... 6 more ... | "classNamePrefix"> | undefined): (props: {}) => Record<...>', gave the following error.
    Argument of type '(theme: Theme) => 
{ toolbarMargin: React.CSSProperties; myDrawer: { width: number; }; }' is not assignable to parameter of type 'Styles<Theme, {}, "toolbarMargin" | "myDrawer">'.      Type '(theme: Theme) => { toolbarMargin: React.CSSProperties; myDrawer: { 
width: number; }; }' is not assignable to type 'StyleRulesCallback<Theme, {}, "toolbarMargin" | "myDrawer">'.  TS2769   

    15 | }
    16 | 
  > 17 | const useStyles = makeStyles((theme: Theme) => ({
       |                              ^ 
    18 |     toolbarMargin: theme.mixins.toolbar,
    19 |     myDrawer: {
    20 |         width: 20,

[ririvas]

@eps1lon, let me know if I can help in anyway

[eps1lon]

@eps1lon, let me know if I can help in anyway

You can try reverting the part of #19263 that changed the type of theme.mixins.toolbar while adding a type test for it. Test would be creating a createMixings.spec.ts with a theme such as from your initial issue description. Should throw in yarn typescript and pass after you reverted that particular change.

[seandearnaley]

also experiencing this issue

[oliviertassinari]

@seandearnaley What problem? Do you have reproduction?

[seandearnaley]

@seandearnaley What problem? Do you have reproduction?

@oliviertassinari so when I updated my @material-ui/core version to 4.9.0 I started getting type definition errors on makeStyles(). I checked the release notes and docs and I can't see any breaking changes I should make. It appears to be the same aforementioned issue on this thread. Rolling back the version to ^4.8.3 gets rid of the error.

import { makeStyles } from '@material-ui/core/styles';

const drawerWidth = 240;

export const useStyles = makeStyles(theme => ({
  appBarSpacer: theme.mixins.toolbar,
  content: {
    padding: '10px',
    height: '100vh',
    overflow: 'auto',
    width: '100vw',
  },
  root: {
    display: 'flex',
  },
  navRoot: {
    flexGrow: 1,
  },
  navPaper: {
    padding: theme.spacing(2),
    textAlign: 'left',
    color: theme.palette.text.secondary,
  },
  paper: {
    padding: theme.spacing(2),
    display: 'flex',
    overflow: 'auto',
    flexDirection: 'column',
  },
  toolbar: {
    paddingRight: 24, // keep right padding when drawer closed
  },
  toolbarIcon: {
    display: 'flex',
    alignItems: 'center',
    justifyContent: 'flex-end',
    padding: '0 8px',
    ...theme.mixins.toolbar,
  },
  appBar: {
    zIndex: theme.zIndex.drawer + 1,
    transition: theme.transitions.create(['width', 'margin'], {
      easing: theme.transitions.easing.sharp,
      duration: theme.transitions.duration.leavingScreen,
    }),
  },
  appBarShift: {
    marginLeft: drawerWidth,
    width: `calc(100% - ${drawerWidth}px)`,
    transition: theme.transitions.create(['width', 'margin'], {
      easing: theme.transitions.easing.sharp,
      duration: theme.transitions.duration.enteringScreen,
    }),
  },
  menuButton: {
    marginRight: 36,
  },
  menuButtonHidden: {
    display: 'none',
  },
  title: {
    flexGrow: 1,
  },
  drawerPaper: {
    position: 'relative',
    whiteSpace: 'nowrap',
    width: drawerWidth,
    transition: theme.transitions.create('width', {
      easing: theme.transitions.easing.sharp,
      duration: theme.transitions.duration.enteringScreen,
    }),
  },
  drawerPaperClose: {
    overflowX: 'hidden',
    transition: theme.transitions.create('width', {
      easing: theme.transitions.easing.sharp,
      duration: theme.transitions.duration.leavingScreen,
    }),
    width: theme.spacing(7),
    [theme.breakpoints.up('sm')]: {
      width: theme.spacing(9),
    },
  },
  container: {
    paddingTop: theme.spacing(4),
    paddingBottom: theme.spacing(4),
  },
  fixedHeight: {
    height: 240,
  },
  cardContainerRoot: {
    flexGrow: 1,
    '& > *': {
      margin: theme.spacing(1),
    },
  },
  gridItem: {
    alignItems: 'center',
  },
  extendedIcon: {
    marginRight: theme.spacing(1),
  },
  fabAdd: {
    margin: '0px',
    top: 'auto',
    right: '20px',
    bottom: '20px',
    left: 'auto',
    position: 'fixed',
  },
  modal: {
    position: 'absolute',
    width: 400,
    backgroundColor: theme.palette.background.paper,
    border: '2px solid #000',
    boxShadow: theme.shadows[5],
    padding: theme.spacing(2, 4, 3),
  },
}));

[seandearnaley]

any updates on this?

[ririvas]

Hi,
I forked the library and started attempting to implement what @eps1lon suggested. Hopefully, I can wrap that up tomorrow by the end of the day. Then I will issue a pull request.

I've just been going through the changes made and the thread on improving type performance. Seems like I will go for a simple revert, but I wanted to make sure I understood the context

If someone can address it quicker, please go ahead.

[seandearnaley]

Thanks @ririvas sorry about asking for updates, I'm actually in no rush I just thought it was something I may have did, or some doc I missed, thanks for working on it.

[MarcNq]

We temporarily solved the issue by casting theme.mixins.toolbar to Material-UI's CSSProperties instead of React.CSSProperties:

import {makeStyles} from '@material-ui/core/styles'
import {CSSProperties} from '@material-ui/styles'

const useStyles = makeStyles((theme: Theme) => ({
  root: {
    toolbar: theme.mixins.toolbar as CSSProperties,
  }
}))

[eps1lon]

Closed with #19491