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

Wrong typings for makeStyles #13718

Closed
2 tasks done
bartekczyz opened this issue Nov 28, 2018 · 6 comments
Closed
2 tasks done

Wrong typings for makeStyles #13718

bartekczyz opened this issue Nov 28, 2018 · 6 comments
Labels
bug 🐛 Something doesn't work package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. typescript

Comments

@bartekczyz
Copy link

bartekczyz commented Nov 28, 2018

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

Expected Behavior

It should compile without errors

Current Behavior

I'm getting Type error: Expected 1 arguments, but got 2. TS2554 when using makeStyles with 2nd argument

Tech Version
@material-ui/core v3.6.0
@material-ui/styles v3.0.0-alpha.1
React v.16.7.0-alpha.2
TypeScript v3.1.6
@eps1lon eps1lon added bug 🐛 Something doesn't work typescript package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. labels Nov 28, 2018
@eps1lon
Copy link
Member

eps1lon commented Nov 28, 2018

Could you please add the snipped of your usage? Just to verify the options shape.

@bartekczyz
Copy link
Author

Sure,

makeStyles(styles); // works
makeStyles(styles, {});  // doesn't work

It should accept 2nd argument (https://github.com/mui-org/material-ui/blob/next/packages/material-ui-styles/src/makeStyles.js#L14) but it doesn't (https://github.com/mui-org/material-ui/blob/next/packages/material-ui-styles/src/index.d.ts#L113)

@eps1lon
Copy link
Member

eps1lon commented Nov 28, 2018

Oh come on. I meant your actual usage which I could add to the test. You're obviously not just adding an empty object.

@bartekczyz
Copy link
Author

This is NOT about the shape of options argument, but the fact that it's passed to the function. Try to pass an empty object and it will fail. If you want real usage, here it is

makeStyles(styles, {name: 'test'});

@eps1lon
Copy link
Member

eps1lon commented Nov 28, 2018

I just wanted to make sure that everything is covered. I didn't intend to make this sound like I'm somehow not believing you that this is an actualyl issue. Just wanted to make sure to make this right. I'm sure you wouldn't have been satisfied if I just added options?: {}.

@bartekczyz
Copy link
Author

It would be great to have options typed. However, all I wanted to say is that typings are not covering the code and it's impossible to pass any option using typescript.

eps1lon added a commit that referenced this issue Nov 28, 2018
Closes #13718 

* [styles] Add options definitions for makeStyles

* [styles] Improve makeStyles typings test coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. typescript
Projects
None yet
Development

No branches or pull requests

2 participants