mui / material-ui

Materialย UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.56k stars 31.91k forks source link

[typescript] Typography in theme typed as `unknown` #30678

Open Nicktho opened 2 years ago

Nicktho commented 2 years ago

Duplicates

Latest version

Current behavior ๐Ÿ˜ฏ

At the moment the typography key in the system's theme is typed as unknown:

node_modules/@mui/system/createTheme/createTheme.d.ts

export interface ThemeOptions {
  shape?: ShapeOptions;
  breakpoints?: BreakpointsOptions;
  direction?: Direction;
  mixins?: unknown;
  palette?: Record<string, any>;
  shadows?: unknown;
  spacing?: SpacingOptions;
  transitions?: unknown;
  components?: Record<string, any>;
  typography?: unknown;
  zIndex?: Record<string, number>;
}

Expected behavior ๐Ÿค”

It should be typed to use TypographyVariants and TypographyVariantOptions or a @mui/system alternative so that we get type safety when using theme.typography like with @mui/material

Steps to reproduce ๐Ÿ•น

No response

Context ๐Ÿ”ฆ

No response

Your environment ๐ŸŒŽ

n/a

Nicktho commented 2 years ago

On top of this, i am unable to access theme.typography in a component because it is type unknown and createTheme does not accept a generic to describe the shape of the theme

Nicktho commented 2 years ago

Also createBox does not allow us to modify the theme shape too

hbjORbj commented 2 years ago

i am unable to access theme.typography in a component because it is type unknown

What do you mean you can't access theme.typography?

hbjORbj commented 2 years ago

Can you provide a codesandbox that shows the error and what you want to achieve clearly?

You can fork this template (https://material-ui.com/r/issue-template-latest).

Nicktho commented 2 years ago

Hi @hbjORbj, please see this sandbox: https://codesandbox.io/s/loving-violet-nguoc?file=/src/Demo.tsx

Notice the type failing on fontSize. This is also true for custom theme properties as well as the other properties that are typed 'unknown'

hbjORbj commented 2 years ago

@Nicktho

Hi, createBox is not meant for users to use. You should import Box from @mui/material and use ThemeProvider from @mui/material/styles to pass a theme.

Please check out https://codesandbox.io/s/lucid-agnesi-9gt0d?file=/src/Demo.tsx

github-actions[bot] commented 2 years ago

๐Ÿ‘‹ Thanks for using MUI Core!

We use GitHub issues exclusively as a bug and feature requests tracker, however, this issue appears to be a support request.

For support, please check out https://mui.com/getting-started/support/. Thanks!

If you have a question on StackOverflow, you are welcome to link to it here, it might help others. If your issue is subsequently confirmed as a bug, and the report follows the issue template, it can be reopened.

Nicktho commented 2 years ago

@hbjORbj Is this the wrong repo to post system issues? As far as I'm aware createBox is a public api, as seen here in the docs: https://mui.com/system/box/#create-your-own-box-component

Nicktho commented 2 years ago

@mnajdova has helped in the past with @mui/system issues on this repo.

mnajdova commented 2 years ago

@Nicktho I agree we should improve the typings on these utils. Especially as we envision those would be used in different design systems. We could either, add typings that could be changed by module augmentation, or provide generics.

mnajdova commented 2 years ago

@siriwatknp from Joy's standpoint, what would work better?

Nicktho commented 2 years ago

Thanks for reopening @mnajdova! We ended up extending Theme itself to get it to work in our design system but a generic or module augmentation would be much much less painful

siriwatknp commented 2 years ago

We need to be clear on what package are we talking about but from the MUI system perspective, the typography should be a Record<TypographyScale, CSSObject> where the consumer can extends the TypographyScale via module augmentation.

declare module "@mui/system" {
  interface TypographyScaleOverrides {
    body: CSSObject;
    ...your choice
  }
}

@siriwatknp from Joy's standpoint, what would work better?

I lean toward module augmentation because:

mnajdova commented 2 years ago

So, looks like we need to update the types for the typography inside the system's ThemeOptions and that should solve the issue, as developers can use module augmentation. I wonder if we even need to update it, as it can be changed anyway to whatever structure the developers need using module augmentaiton, no?

mgoetz-nerdery commented 2 years ago

Hi, I am running into this issue now using MUI 5.8.6, even when I try to use module augmentation. I get this error when I try module augmentation:

Subsequent property declarations must have the same type.  Property 'typography' must be of type 'unknown', but here has type 'Typography | undefined'.

Has anyone found a workaround for this issue?

mnajdova commented 1 year ago

cc @siriwatknp?

siriwatknp commented 1 year ago

Hi, I am running into this issue now using MUI 5.8.6, even when I try to use module augmentation. I get this error when I try module augmentation:

Subsequent property declarations must have the same type.  Property 'typography' must be of type 'unknown', but here has type 'Typography | undefined'.

Has anyone found a workaround for this issue?

Please share the code snippet or a reproducible sandbox.

johnta0 commented 1 year ago

Hi @mgoetz-nerdery . I was experiencing the same issue but I could workaround by the code below. In my case theme.transitions didn't work because its type is 'unknown'.

...
      transition: (theme.transitions as any).create('width'),
...
lifedok commented 1 year ago

I have the same problem. Through Box and Typography works. But through styled breaks the build of the project.

You can do as suggested above. But we are now rewriting the project and this method is more like a crutch or a temporary solution.

Screenshot 2023-01-04 at 15 00 29
siriwatknp commented 1 year ago

I have the same problem. Through Box and Typography works. But through styled breaks the build of the project.

Please use styled from @mui/material/styles;

- import { styled } from '@mui/system';
+ import { styled } from '@mui/material/styles'; // or '@mui/material'
siriwatknp commented 1 year ago

We could work on this in v6 according to https://github.com/mui/material-ui/discussions/30988.