[4.1.0-beta] Incorrect method overload selected

[berickson1]

Relevant investigation data:
Material UI makeStyle function has 2 signatures, 1 where Props are empty object ({}), the second where props are a generic (default of {}). When using the spread operator for the object passed into makeStyle, the second more specific (and incorrect) signature is picked. This ONLY happens when there are other values provided before or after the spread operator. If the spread object is the only set of values in the object, the 'correct' overload is suggested.

I believe all 3 cases below should be equivalent and select the same function overload. This works perfectly in previous versions of typescript, so would indicate some form of bug or un-tracked breaking change.

TypeScript Version: 4.1.0-beta & 4.1.0-dev.20201014

Search Terms:
Material UI styles TS2554 overload type
Code

const useStylesBroken = makeStyles(theme =>
    createStyles({
        sizeSmall: {
            ...theme.typography.button, // This is simply a set of css styles - nothing relating to props
            height: '36px',
        },
    })
);
const useStylesWorks = makeStyles(theme =>
    createStyles({
        sizeSmall: {
            height: '36px',
        },
    })
);
const useStylesWorks2 = makeStyles(theme =>
    createStyles({
        sizeSmall: {
            ...theme.typography.button,
        },
    })
);

export function NotReallyAButton(): null {
    console.log(useStylesBroken());
    console.log(useStylesWorks());
    console.log(useStylesWorks2());
    return null
}

See https://github.com/berickson1/Playground/blob/master/materialUiSample.ts

Expected behavior:
Code compiles successfully (as in previous released typescript versions)
Actual behavior:
Incorrect overload chosen & compiler error:

materialUiSample.ts:27:17 - error TS2554: Expected 1 arguments, but got 0.

27     console.log(useStylesBroken());
                   ~~~~~~~~~~~~~~~~~

  node_modules/@material-ui/core/styles/makeStyles.d.ts:23:5
    23 ): (props: Props) => ClassNameMap<ClassKey>;
           ~~~~~~~~~~~~
    An argument for 'props' was not provided.

Playground Link:
Requires material-ui dependency installed, see repo at https://github.com/berickson1/Playground (yarn install && yarn build)
Related Issues:
#41049 <- Looks similar but only relates to promises, so is not applicable here

[RyanCavanaugh]

@berickson1 is there a test suite or something I can use when testing out modifications to the material-ui definitions to make sure the things I might propose don't break anything?

[RyanCavanaugh]

Bisected to #40002

Without an isolated repro I don't see us getting to this right away - my initial attempt to distill this down into something that didn't require the material-ui definitions wasn't successful

[RyanCavanaugh]

In the meantime I changed this

/**
 * `makeStyles` where the passed `styles` do not depend on props
 */
export default function makeStyles<Theme = DefaultTheme, ClassKey extends string = string>(
  style: Styles<Theme, { }, ClassKey>,
  options?: Omit<WithStylesOptions<Theme>, 'withTheme'>
): (props?: any) => ClassNameMap<ClassKey>;

to this

/**
 * `makeStyles` where the passed `styles` do not depend on props
 */
export default function makeStyles<Theme = DefaultTheme, ClassKey extends string = string>(
  style: Styles<Theme, never, ClassKey>,
//                     ^^^^^
  options?: Omit<WithStylesOptions<Theme>, 'withTheme'>
): (props?: any) => ClassNameMap<ClassKey>;

and got the desired behavior

[berickson1]

@berickson1 is there a test suite or something I can use when testing out modifications to the material-ui definitions to make sure the things I might propose don't break anything?

What I've shared is about as far as I've gotten to be honest. I was trying to prep our client codebase for typescript 4.1 and deal with the latest set of changes and ran into this. I'm only familiar with MUI types from a usage standpoint, but I can try to distill a specific example down either this weekend or early next week.

I largely wanted to get this on the radar for the TS team since MUI is a relatively popular UI component library and this introduces an (albeit unintentional) breaking change.

[berickson1]

I did some digging and tracked the behaviour back to 1 specific line in the material-ui .d.ts. Commenting it out fixes everything in my example (but it's still needed for some font face overrides in general).

/**
 * Allows the user to augment the properties available
 */
export interface BaseCSSProperties extends CSS.Properties<number | string> {
  //'@font-face'?: JSSFontface | JSSFontface[];
}

Below is my (somewhat stream of consciousness) dive into the underlying MUI types a bit - may or may not be useful for investigations, but doesn't hurt to post

A bit more info:
createStyle takes 2 generics, the 2nd being Props.
This uses

export type StyleRules<Props extends object = {}, ClassKey extends string = string> = Record<
  ClassKey,
  // JSS property bag
  | CSSProperties
  // JSS property bag where values are based on props
  | CreateCSSProperties<Props>
  // JSS property bag based on props
  | PropsFunc<Props, CreateCSSProperties<Props>>
>;

The problematic type in there is CreateCSSProperties<Props>. This defines a type with two index accessors - one from extending BaseCreateCSSProperties, the other defined inline. Removing either of these index signatures makes the compiler happy.

This is about as far as I've been able to make it today.

[berickson1]

After a short delay, I have a contrived standalone repro. I've updated the git repo above and created a playground link:
https://www.typescriptlang.org/play?#code/C4TwDgpgBAUgynAYgewHbAGYEMDG0C8UA3gFBRQD0AVFAM7IC20YATspC8AJYS1QAWEFhAA0UACbJeUVMmBQGWYMCFQMyFlGFhkUKhRIBfANwkSoSFAAKbMLUQBXVDgA8N9nwgAPFanF9kACMAKwgcYDEAFQA+KEIAClYPAC5rW1oASjjYyNMSaioyGgBBABtS5AB3PmBBKAdaVWBdLAcAcyZ0KFrmWyFuaSwANywuUqxA0ogigy50IWw8KDghtvcOAdoXSIAZCFQ22rioeNpgFjm2qAAyYkMsgB8oAAZY0nJArEbSuYg4fi4GGAAH5Urt9od+KZyBgxioWKC6OdLjc7qZDCRvDpOFA5vDFtB1v0eFtwQcjgkzhcDqiiPcoE9XmQoN5fP5lqsiZwSds9uT+G8MSQ8QtcNAAEJfCAAYQQXM2LJ8+3Z8p5qAcDECqieVMub2ZFAoADkAPKRACiqQAkhhugC+Fw+MEGvJ1ZqhGIeoqsAwwFMoDhGGAxrwDRQAOQAAXU6AAtATw4j4Eg0JgxQzYAgUOgCQBtAC60O6vBBqWT2bTeAL6LMIpYBKgsrgqukrOVfEljSbLb470oFCgZQqlSgdggDkkdAgU3CGj4WD8Cgg4i4WCgAEcHEISWGoAADJwAa1klVQe9xfAay7ocyWkTgEikfFk8iw5SqUC+tC4bVQKIXuLzPW6aVFwtTIA4wC7gBcziN4N6-koDjCN0uhoNAoFHDBfjwd+iHAMhEAAHRQJE9oXqhn7viOFjQLQWAYNM5CGgoyDiA4-qtB0+zAEoXBoERu6RGE-B-jgb6lCAUCVBhC6vqgUmHhAUkvt04DXnM+6djKcp9NyvDns0UBalAyC2rRu57t2embOegSQXajqPrwqDhvIlQaIeJFkdAsG4T+qBIShqAQMuNS6IGQxCLu2nWewxKDHwMnlCRACqtAOBJUkgBB0kQaU4j1I0WlSnFGwkrmSk5basW6fF+m0Pme67vZ8i1E5HkcYVqkSR+X4BSiABKEC4MARFlQl4WNnV5WhuQlWpLqBz5qkR4nqgGYTQ1NbCkBDbSsISg6c2Nk8usnhKn4AQhGE8iEHS0TMm2V1QLFh0qFtmxuOk+rMQOQ4fmOE66N8t1zp+i5MCua6btuc1QAtSLUm0K3MuQTxvSNH0zZN30eNElXKWZr2le9x09njdjRPmaObWTn1nT9O20STXb0zjDWUxdbLXaE4THA9AvMrmViAVAVXE7VJ31ZsqPkOjrPk6dvAizT8sZudjjOFzYhSz2qvROieQs3AoBTINHG8Fzio86ZN38-dhixIQw2Biw4guMyS1tCIzJPAz8Mscmo42VJnxXJUggoSMpRbvOKGfI0hVoCHHh+427PS7NWznY9htmHBODjChGBOOE-EbTgZOmyAUw5+kNvtnc0TxF7Zu8KkNfm5b9f4yQGSd+3Ft11z+f5AO5oABrFAAslYOzmlAqVwMUADii9wJExSDfeJCBqgZxqFgSmROAyBtCwWBgPwIBdxAqQB3whAACyfvOCl5Pvh+BGwSmoHfxwq5Yz+O3Wg8Q+zfgAF5-EUOUVIfZyBESQdgE+Z8L5XxvnfMQLEyJOSct+X0klPxTnkMTHAtA+BnFrtIWMMg5AAhpMIcY3AaRGSSHYWmggfz8GAKkAARAAZgAGxgC8Hw325BDC+3uKYL+7lPKXDvgAJkAdXUB4CvZcGgXAWBpR4G0yQURFBEBT46HQdfW+7dsEDlwQ6ShXBCFSTXI0UhtpyGUNAVAWhL4GFXCYXxVhuh2G0GZFIowGRZFoEPh5Fgh5FHt1UcAu+YCIFaJgRJfR6suFtB4fw4RojxGhOkRE8epF1K0CrlwMA8hn5EWeJ+C+oUaj8CUHaXg0AsAoQgJuN8Xi1KQAqRcapUBakAEZAKMRYPOCQgJJk8SgDHLcpkNpemEG7cQYgIBRRWfwCCOS2lSVoLsrqxloCBl9HxSY0wcHkScmuQIYFTK2jXPvZEUVCreB9H6UQxkHJgXDHwFcZwxhTEKpIU8ag2AMGIYdUoCgjoXF6Q4LgRUICxnEo0PeUT5BYlusuO+ozUi0WJjEuJBwAGEB-sgP+d9IkHxxV4SA4R8XtyUUS9SJKFHkoSYQUl8TqFKNMEAA

[berickson1]

The title here should be updated - this doesn't appear to be related to method overloads based on the distilled sample. It appears that inferred generics are improperly selected due to the usage of the spread operator in object declaration.

[danvk]

I also ran into this issue today when I tried out the TS 4.1 RC. In case it's helpful, here's my repro. This type checks in TS 4.0.2 but not 4.1RC.

import {createStyles, makeStyles} from '@material-ui/core/styles';
import React from 'react';

export function HeaderBreadcrumbs() {
  const classes = useStyles();
  //              ~~~~~~~~~~~ Expected 1 arguments, but got 0. ts(2554)
  return <div className={classes.outer}>Hello!</div>;
}

const useStyles = makeStyles(theme =>
  createStyles({
    outer: {
      '& .MuiLink-root': {
        ...theme.typography.h6,  // this line is key!
        color: 'inherit',
      },
    },
  }),
);

[timbuckley]

This bug persists on 4.1.2

[ahejlsberg]

I'm going to close this issue. The repro is quite complex and it isn't clear to me what the types are trying to achieve, plus it looks like the working cases in the repro actually rely on type inference failing and defaulting to the {} constraint.