[styles] Support string templates in TypeScript #16940
Conversation
rogerclotet
commented
Aug 8, 2019
rogerclotet
commented
- Support string templates in Typescript
- Migrate StringTemplates demo to Typescript (TypeScript demos #14897)
- I have followed (at least) the PR section of the contributing guide.
oliviertassinari
added
docs
|
No bundle size changes comparing 0168a6d...079f8f7 |
rogerclotet
force-pushed
the
styles/makeStyles
branch
from
9a517e1 to
079f8f7
Compare
oliviertassinari
changed the title
There was a problem hiding this comment.
makeStyles doesn't support string templates without a plugin, so updating the types to always allow strings isn't exactly correct.
cc @eps1lon
Shouldn't update the hook, should update the Styles type instead.
https://github.com/mui-org/material-ui/blob/5dc2402c9df6f148756129bdabdb1e2fd8363713/packages/material-ui-styles/src/withStyles/withStyles.d.ts#L46-L47
| @@ -41,6 +41,6 @@ export default function makeStyles< | |||
| Props extends {} = {}, | |||
| ClassKey extends string = string | |||
| >( | |||
| styles: Styles<Theme, Props, ClassKey>, | |||
| styles: Styles<Theme, Props, ClassKey> | Record<ClassKey, string>, | |||
There was a problem hiding this comment.
Should update @material-ui/core as well
https://github.com/mui-org/material-ui/blob/5dc2402c9df6f148756129bdabdb1e2fd8363713/packages/material-ui/src/styles/makeStyles.d.ts#L11
You're right, maybe this change is not correct. What should I do then, is there a way of making it available only in those cases? Should we even bother with that or leaving it as a javascript-only feature instead?
Updating the Thank you for your feedback! |
|
I think we should only document how to augment the types with the plugin. Those types shouldn't be added to the library definitions if they require a runtime plugin. Hopefully this "just" works with type augmentation. Otherwise we might have to adjust the types. @merceyz made some changes to the types recently to improve type augmentation so he probably knows better how to make this work. |
|
It's currently not possible to augment a type, you can only add things to an interface using augmentation. You could however do something like this microsoft/TypeScript#28078 (comment) |
|
How do you guys want to move forward? I tends to think that almost nobody use the template approach. Until it has support for nested rules, it's not very useful. Also, moving to styled-components will make the concern obsolete. So maybe, we should ignore the problem? |
|
I agree, I don't think most people will use string templates, I just wanted to cover this in Typescript, but you can close the PR if you think it's not worth the hassle. Please update the issue as well marking this as not needed. |
|
I have updated the related issue. |
