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

[typescript] Typography in theme typed as unknown #30678

Open
2 tasks done
Nicktho opened this issue Jan 18, 2022 · 21 comments
Open
2 tasks done

[typescript] Typography in theme typed as unknown #30678

Nicktho opened this issue Jan 18, 2022 · 21 comments
Labels
package: system Specific to @mui/system typescript

Comments

@Nicktho
Copy link

Nicktho commented Jan 18, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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 Nicktho added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 18, 2022
@Nicktho
Copy link
Author

Nicktho commented Jan 18, 2022

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
Copy link
Author

Nicktho commented Jan 18, 2022

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

@hbjORbj
Copy link
Member

hbjORbj commented Jan 18, 2022

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 hbjORbj added status: waiting for author Issue with insufficient information typescript and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 18, 2022
@hbjORbj hbjORbj changed the title [@mui/system] Typography in theme typed as unknown [typescript] Typography in theme typed as unknown Jan 18, 2022
@hbjORbj
Copy link
Member

hbjORbj commented Jan 18, 2022

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
Copy link
Author

Nicktho commented Jan 19, 2022

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
Copy link
Member

hbjORbj commented Jan 19, 2022

@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

@hbjORbj hbjORbj added status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it support: Stack Overflow Please ask the community on Stack Overflow and removed status: waiting for author Issue with insufficient information labels Jan 19, 2022
@github-actions
Copy link

👋 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
Copy link
Author

Nicktho commented Jan 19, 2022

@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
Copy link
Author

Nicktho commented Jan 19, 2022

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

@mnajdova
Copy link
Member

mnajdova commented Mar 2, 2022

@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 mnajdova reopened this Mar 2, 2022
@mnajdova mnajdova added package: system Specific to @mui/system and removed support: Stack Overflow Please ask the community on Stack Overflow status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it labels Mar 2, 2022
@mnajdova
Copy link
Member

mnajdova commented Mar 2, 2022

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

@Nicktho
Copy link
Author

Nicktho commented Mar 3, 2022

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
Copy link
Member

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:

  • consistent experience with other MUI packages

  • It works with system Box whereas generic don't

    // you can directly use Box from the system without the need to call `createBox`
    import { Box } from '@mui/system';
    
    // ...module augmentation
    
    // theme will be typed correctly
    <Box sx={theme => {...}}>

@mnajdova
Copy link
Member

mnajdova commented Mar 3, 2022

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
Copy link

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
Copy link
Member

cc @siriwatknp?

@siriwatknp
Copy link
Member

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
Copy link

johnta0 commented Aug 21, 2022

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
Copy link

lifedok commented Jan 4, 2023

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
Copy link
Member

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
Copy link
Member

We could work on this in v6 according to #30988.

@siriwatknp siriwatknp added this to the v6 milestone Jan 5, 2023
k3vnb added a commit to k3vnb/modular_montage that referenced this issue Jun 15, 2024
- unfortunately the MUI/system lib is not configured for overriding some theme property types, like 'palette' & 'typography'
- mui/material-ui#30678
- the styles property is added via module augmentation, allows for a type safe usage of theme in child components.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: system Specific to @mui/system typescript
Projects
Status: Backlog
Development

No branches or pull requests

9 participants