-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[TypeScript] getCssVar autocomplete for Material UI #33464
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, tested here - https://codesandbox.io/s/basicbuttons-demo-material-ui-forked-u9f18y?file=/demo.tsx
Would be great if we can add a *.spec.ts
where we test that we can invoke the getCssVar
with valid theme value and with custom var too. We can even go one step further and test that module augmentation will be affected too.
Added! I changed my mind to make it strict first (only work with the values inside the theme). Developers can augment by: declare module "@mui/material/styles" {
interface ThemeCssVarOverrides {
customToken: true
}
}
theme.getCssVar('customToken') // 🎉 or cast with theme.getCssVar('palette-primary-main', 'var(--custom-token)' as any) |
? V extends string | number | ||
? keyof T | ||
: keyof V extends string | number | ||
? `${keyof T}-${ConcatDeep<Split<V>>}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not at all a Typescript expert, but doesn't seem to work with Typescript 3.x and the docs say that this is Typescript 3.5 compatible.
This isn't the only place, but most are in here. @mui/material
5.9.1
, fails latest TS 3.x, fine latest TS 4.x.
node_modules/@mui/material/styles/experimental_extendTheme.d.ts:304:9 - error TS1110: Type expected.
304 ? `${keyof T}-${ConcatDeep<Split<V>>}`
~~~
node_modules/@mui/material/styles/experimental_extendTheme.d.ts:304:18 - error TS1005: '}' expected.
304 ? `${keyof T}-${ConcatDeep<Split<V>>}`
~
node_modules/@mui/material/styles/experimental_extendTheme.d.ts:304:19 - error TS1128: Declaration or statement expected.
304 ? `${keyof T}-${ConcatDeep<Split<V>>}`
~
node_modules/@mui/material/styles/experimental_extendTheme.d.ts:304:22 - error TS1005: ';' expected.
304 ? `${keyof T}-${ConcatDeep<Split<V>>}`
~
node_modules/@mui/material/styles/experimental_extendTheme.d.ts:304:43 - error TS1005: '(' expected.
304 ? `${keyof T}-${ConcatDeep<Split<V>>}`
~
node_modules/@mui/material/styles/experimental_extendTheme.d.ts:398:1 - error TS1160: Unterminated template literal.
398
node_modules/@mui/system/Unstable_Grid/GridProps.d.ts:95:90 - error TS1005: '>' expected.
95 declare type CustomBreakpoints = Partial<Record<Breakpoint, boolean | GridSize> & Record<`${Breakpoint}Offset`, GridSize>>;
~~~
node_modules/@mui/system/Unstable_Grid/GridProps.d.ts:95:123 - error TS1109: Expression expected.
95 declare type CustomBreakpoints = Partial<Record<Breakpoint, boolean | GridSize> & Record<`${Breakpoint}Offset`, GridSize>>;
~
Found 8 errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it break even if you don't use those APIs? If yes, please open an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm not using extendTheme
but tsc -noEmit
was causing the above output with a couple of 3.x versions. It's also not a problem for me in the particular project I ran into with, I can just upgrade the typescript version without pain (and have done so...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the info.
Sandbox: https://codesandbox.io/s/create-react-app-with-typescript-forked-tlp4uv?file=/src/App.tsx