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
91.73k stars 31.51k forks source link

[material-ui][theme] Array syntax doesn't work for values in styleOverrides in theme #41971

Open cjl750 opened 2 weeks ago

cjl750 commented 2 weeks ago

Steps to reproduce

Link to live example: https://codesandbox.io/p/sandbox/mui5-theme-limitations-skx78f

Steps:

  1. create a theme with createTheme, StyledEngineProvider and ThemeProvider
  2. add a key under components for a MUI component
  3. add a styleOverrides key for that component using array syntax

Current behavior

Currently, styles added as styleOverrides to a theme don't work if using array syntax, but more normal string syntax will work.

This is confusing because array syntax does work for custom keys in the theme. It also works when creating styles with makeStyles/withStyles from @mui/styles.

For example:

createTheme({
  components: {
    MuiTypography: {
      styleOverrides: {
        root: {
          padding: [[5, 10]], // doesn't work
          padding: '5px 10px', // works
       },
     }.
    },
  },
  myCustomKey: {
    padding: [[5, 10]], // works
  }
})

You can see examples of both these behaviors in the linked Codesandbox.

Expected behavior

Ideally, array syntax would work in styleOverrides like it does everywhere else. At a minimum, the limitation should be documented.

Context

I've been migrating my app from v4 to v5 and following the migration guide and ran into this issue. I did not see this mentioned anywhere in the breaking changes for styles and themes or anywhere else in the v5 docs.

Your environment

npx @mui/envinfo ``` System: OS: Windows 11 10.0.22631 Binaries: Node: 20.11.0 - C:\Program Files\nodejs\node.EXE npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD pnpm: 8.8.0 - C:\Program Files\nodejs\pnpm.CMD Browsers: Chrome: 123.0.6312.124 Edge: Chromium (123.0.2420.97) npmPackages: @emotion/react: ^11.11.4 => 11.11.4 @emotion/styled: ^11.11.5 => 11.11.5 @mui/base: 5.0.0-beta.40 @mui/core-downloads-tracker: 5.15.15 @mui/icons-material: ^5.15.15 => 5.15.15 @mui/material: ^5.15.15 => 5.15.15 @mui/private-theming: 5.15.14 @mui/styled-engine: 5.15.14 @mui/styles: ^5.15.15 => 5.15.15 @mui/system: 5.15.15 @mui/types: 7.2.14 @mui/utils: 5.15.14 @mui/x-date-pickers: ^7.1.1 => 7.1.1 @types/react: 18.2.78 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 typescript: 4.9.5 ```

I'm using Chrome 123.

Search keywords: theme, styleOverrides, array

ZeeshanTamboli commented 1 week ago

Where in the Material UI v4 documentation does it mention support for the array syntax?

cjl750 commented 1 week ago

I'm not sure that the v4 docs specifically call out support for the array syntax, but they say the styling system is built in JSS, and the JSS docs talk about it. So far I have been using it in my v4 project's theme with no issues.

Meanwhile the v5 docs say that migrating from JSS to Emotion is optional, and the section on theme updates in the v5 docs doesn't mention support was dropped.

This section does mention that support for the $ local rule reference syntax won't work. Perhaps around there would be a good place to mention the array syntax as well.

ZeeshanTamboli commented 1 week ago

@cjl750 Okay alright, thanks for the details. Since this issue wasn't previously brought up by users, we hadn't considered it before. Adding a mention about it in the section you pointed out seems like a good idea. Would you like to create a PR for it?

cjl750 commented 1 week ago

Would you like to create a PR for it?

Sure, I can take a shot at that later this week sometime, if that works.