Typescript 3.5 causes makeStyles problem

[nmain]

• This is not a v0.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When using typescript 3.5, code that worked in typescript 3.4 is now rejected:

const useStyles = makeStyles((theme: Theme) =>
	createStyles({
		root: {
			paddingTop: theme.spacing(0.5),
			paddingBottom: theme.spacing(0.5),
		},
	})
);

export default function MyComponent(props: any) {
	const { root } = useStyles(); // ERROR: Expected 1 arguments, got 0
	return <div className={root}>{props.children}</div>;
}

Expected Behavior 🤔

useStyles() should work with 0 arguments

The problem seems to come from a change in something that's commented on in makeStyles.d.ts:

 * If a style callback is given with `theme => stylesOfTheme` then typescript
 * infers `Props` to `any`.
 * If a static object is given with { ...members } then typescript infers `Props`
 * to `{}`.
 *
 * So we require no props in `useStyles` if `Props` in `makeStyles(styles)` is
 * inferred to either `any` or `{}`

In typescript 3.5, Props is inferred to object now.

Steps to Reproduce 🕹

I cannot reproduce this on codesandbox.io right now. I suspect it's because I can't get the right typescript version there. I can however reproduce this from a new empty create-react-app, using just the packages below and the code above.

Your Environment 🌎

    "@material-ui/core": "^4.0.1",
    "@material-ui/icons": "^4.0.1",
    "@material-ui/styles": "^4.0.1",
    "typescript": "^3.5.1"

[eps1lon]

3.5 announcement is scheduled for tomorrow (holiday for me). Will have a look on friday and see what changed.

[nmain]

3.5 was actually released today; I guess they changed. No hurry of course, but 3.5.1 is the latest tag on npm right now.

[eps1lon]

But I hope you understand that the latest breaking release will always take some time to adjust to. I recommend you never upgrade to the latest typescript versions in production. Releases in typescript don't follow semantic versioning. Minor releases are almost always breaking.

[nmain]

I do understand. I upgraded to typescript 3.5.1 on a separate branch of my app, saw this issue, and switched back to my main branch. My app remains on 3.4.5 and can stay there for the foreseeable future. I wrote it up right away because, might as well have the information available for anyone who is interesting in looking into it.

[Kimahriman]

Just a little digging I've done. Issue seems to be with IsEmptyInterface, specifically the third clause

unknown extends T ? false : true

Simply testing this out with the default value of {}:

type Test = unknown extends {} ? false : true

Test is true in 3.4.5 and false in 3.5.1

Not sure what the right fix would be. I assume it's due to microsoft/TypeScript#30637 (Also the first listed "Breaking change" on https://devblogs.microsoft.com/typescript/announcing-typescript-3-5/)

[ghost]

I've got exactly the same issue. Workaround is to just pass in an empty object, so it becomes useStyles({})

[eps1lon]

Well it's a breaking change. We now have to figure out how to support to incompatible versions of typescript. I think that's where typeVersions comes in. I hope this doesn't require a complete fork but only one for how we determine if a given styles type requires props.

[merceyz]

My app remains on 3.4.5

@nmain Since you're on that version of typescript you don't need to specify the theme type and you can omit createStyles. Doesn't solve the issue, but might clean up your app ;)

[merceyz]

Actually I was wrong, it does solve the issue.

You didn't specify that you're importing from @material-ui/styles.
When I import from @material-ui/core/styles or remove createStyles it works fine.

Fixed in #15990

[HorusGoul]

I'm using the latest version (4.0.2) and typescript@3.5.1 and the issue seems to be still there. Am I the only one still affected?

A screenshot of the error:

[PieterBoeren]

Am I the only one still affected?

No, I'm also having the same error with these versions.

[merceyz]

@HorusGoul @PieterBoeren Could you provide an example the reproduces the problem? The one from the OP doesn't cause issues. Also, since you're using a newer version of typescript you can omit createStyles and get around the issue

[PieterBoeren]

I use this syntax (which fails):

const useStyles = makeStyles(() => {

  return {
    root: {
      backgroundColor: 'red',
    }
  };
});

However, it works when using:

const useStyles2 = makeStyles({
  root: {
    backgroundColor: 'red',
  },
});

But I like the first syntax (which was working before typescript 3.5), because you can then add the theme object very quickly when needed, but this also fails now:

makeStyles((theme: Theme) => ...)

Just to be clear: I don't use "createStyles" at all.

[HorusGoul]

@merceyz I'm using the same syntax

const useStyles = makeStyles(theme => ({
  ...
});

And just to clarify, I'm importing makeStyles from '@material-ui/core/styles'

[merceyz]

Provide a codesandbox please, I can't reproduce with those snippets.
Tried to reproduce in the docs and a fresh project using create-react-app

[PieterBoeren]

Can't reproduce it in a CodeSandbox, but based on a standard app create via create-react-app, adding

"strictPropertyInitialization": false,
"strictNullChecks": false

to the tsconfig breaks it. Removing the two lines (or setting them to "true") and it works...

[merceyz]

Thank you, I can reproduce it now. Working on a fix

[merceyz]

@PieterBoeren Your issue isn't the same as what this issue was for. See #16088 (comment)

TL;DR: material-ui doesn't support "strictNullChecks": false

[Kimahriman]

@merceyz Do you know what the use case is for checking

unknown extends T ? false : true

in IsEmptyInterface? That's what seems to differ based on whether strictNullChecks is true or false now. I don't really know enough about how Props is ever used with styles

[eps1lon]

Remove it and run the test. Should fail because it considers unknown to be an empty interface.

3. false if the given type is unknown

[Kimahriman]

Yeah I just meant what's the practical use case for that. Obviously if this only supports strictNullChecks that's fine, but it's that check that's causing the problem with strictNullChecks turned off, so really just curious how you could trigger that in actual code or what it protects against.

Basically:

• TS 3.4.5, strictNullChecks: true: unknown extends {} == false
• TS 3.4.5, strictNullChecks: false: unknown extends {} == false
• TS 3.5.1, strictNullChecks: true: unknown extends {} == false
• TS 3.5.1, strictNullChecks: false: unknown extends {} == true

Again, just wondering, not saying anything is wrong.

[eps1lon]

Good question. I think my thinking just went (back in 3.3): "no inferrable means {} so check for that. Anything else is considered required." It's not about a specific case but more about principle of least astonishment with the given constraints.

There's a case to be made to also accept 0 arguments if the expected type is unknown because you can handle undefined in that case (not that you ever see it at runtime). So yeah we could expand StylesRequireProps to also evaluate to true if the type is unknown.

My problem currently is that the types and tests between core/styles and styles are fractured so I'm extra careful about changes. And if those changes only serve unsupported environments I have to defer that for now.

But once we resolve this we can definitely revisit that.

[ghost]

Any updates?

[eps1lon]

@sashankaryal The original issue was fixed in 4.0.2. If the issue still persists in later versions please open a separate issue and fill out the template.