mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.42k stars 32.16k forks source link

[typescript] IsEmptyInterface type is broken since TS 3.5 with "strictNullChecks: false" #17543

Closed TiuSh closed 5 years ago

TiuSh commented 5 years ago

Current Behavior 😯

While updating Typescript to the latest version (3.6.3) I had this code failing where it worked with TS 3.3:

const useStyles = makeStyles<Theme>(theme => ({
  root: {
    display: "flex",
    "& .MuiButton-root, & .MuiIconButton-root": {
      marginRight: theme.spacing(1),
      "&:last-child": {
        marginRight: 0,
      },
    },
  },
}));

const ButtonGroup: React.FunctionComponent<Props> = ({ className, ...props }) => {
  const classes = useStyles(); // ERROR: Expected 1 arguments, but got 0.

  return <div className={classNames(classes.root, className)} {...props} />;
};

After a little investigation I found that it was related to IsEmptyInterface from @material-ui/types that was not working as expected with TS option "strictNullChecks": false:

import { IsEmptyInterface } from "@material-ui/types";

type ShouldBeTrue = IsEmptyInterface<{}>; 
// TS 3.3: true
// TS 3.6.3: false

type ShouldBeFalse = unknown extends {} ? true : false;
// TS 3.3: false
// TS 3.6.3: true

Expected Behavior 🤔

IsEmptyInterface<{}> should return true, even when "strictNullChecks": false.

Steps to Reproduce 🕹

Steps:

  1. Go to: https://typescript-play.js.org/?strictNullChecks=false&ts=3.3.3#code/KYDwDg9gTgLgBDAnmYcCCA7AJgHjQGjgCFCBhOAXgSgFdgA+S9OUGYbAZ2roCg44A-MRYg2nbsD79Bccq3ZYuMWpOnShy3mv4AuOADMAhgBsOq6XqOnzlk2YDcPHqEiwEyVAEkOAUQC2YEieGGxQRgDGwDgAKoxUmLhSANbAiBD6cNEiYopwGMAAbsBQMpqottb4UhzKAJYYAObZClxZGipwFWZV-DQYSRgQAO4YzeJtBnblEjz0jjxIKHAAygAWEDTGWETA0R1U3v6BiMGhEVEA3gC+cwseK+ub28AAYlNMfQPDo-Li16UdLrAexAA
  2. Note that ShouldBeTrue is of type true and ShouldBeFalse is of type false
  3. Go to: https://typescript-play.js.org/?strictNullChecks=false#code/KYDwDg9gTgLgBDAnmYcCCA7AJgHjQGjgCFCBhOAXgSgFdgA+S9OUGYbAZ2roCg44A-MRYg2nbsD79Bccq3ZYuMWpOnShy3mv4AuOADMAhgBsOq6XqOnzlk2YDcPHqEiwEyVAEkOAUQC2YEieGGxQRgDGwDgAKoxUmLhSANbAiBD6cNEiYopwGMAAbsBQMpqottb4UhzKAJYYAObZClxZGipwFWZV-DQYSRgQAO4YzeJtBnblEjz0jjxIKHAAygAWEDTGWETA0R1U3v6BiMGhEVEA3gC+cwseK+ub28AAYlNMfQPDo-Li16UdLrAexAA
  4. Note that ShouldBeTrue is of type false and ShouldBeFalse is of type true

Context 🔦

Actually, I'm not sure if it's an issue from Material-UI or typescript as I don't fully understand the meaning of the type:

/**
 * @internal
 *
 * check if a type is `{}`
 *
 * 1. false if the given type has any members
 * 2. false if the type is `object` which is the only other type with no members
 *  {} is a top type so e.g. `string extends {}` but not `string extends object`
 * 3. false if the given type is `unknown`
 */
export type IsEmptyInterface<T> = And<
  keyof T extends never ? true : false,
  string extends T ? true : false,
  unknown extends T ? false : true
>;

https://github.com/mui-org/material-ui/blob/514319e995cd86e39a5eb937afb5923509d4a07a/packages/material-ui-types/index.d.ts#L75L89

Your Environment 🌎

Tech Version
Material-UI v4.4.3
Typescript v3.6.3
eps1lon commented 5 years ago

If somebody can make the test suite pass with strictNullChecks: false I'll gladly merge this. Until then I'd like to refer to the docs that document this requirement: https://material-ui.com/guides/typescript/#typescript