[system] Allow callbacks when defining styleOverrides inside the theme

[mnajdova]

• I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Currently the styleOverrides defined in the theme are applied on top of the styles defined for the component, so they would always win.

This means that if developers add overrides for the root element those will always be applied on top of whatever styles were defined in the base component (even styles specific for some props combination).

For example, the code below would override the background even when the color: 'primary' prop is set:

const theme = createTheme({
  components: {
    MuiBadge: {
      styleOverrides: {
        badge: {
          backgroundColor: "white"
        }
      }
    }
  }
});

Would be great if developers can define styles based on combination of props.

Examples 🌈

With the example below, the developers would be able to provide styls overrides that match the condition props.color !== 'defualt.

const theme = createTheme({
  components: {
    MuiBadge: {
      styleOverrides: {
        root: (props) => ({
          ...(props.color !== 'default' && { backgroundColor: 'white' }),
        })
    }
  }
});

---

The discussion started in #27369.

[mnajdova]

After spending a bit more time thinking about this, I think that we should consider the following: Currently we have two APIs for overrides in the theme

1. theme.components[Component].styleOverrides - these are the style overrides that basically existed from 4.0

const theme = createTheme({
  components: {
    MuiButton: {
      styleOverrides: {
        root: { color: 'pink' }
      }
    }
  }
})

2. theme.components[Component].variants - new API added shortly before v5 was released.

const theme = createTheme({
  components: {
    MuiButton: {
      variants: [
        { props: { color: 'primary', customProp: true }, style: { ... } }, 
      ]
    }
  }
})

Both of them were working with JSS and are working with emotion now. In my opinion, we should be dropping the styleOverrides long term in favor of the variants. Why? The variants API is much more flexible. With the ability we have now in v5 for extending colors/sizes etc, the styleOverrides is not scaling. For applying styles on the slots, we can use a classes subselectors, for example:

const theme = createTheme({
  components: {
    MuiButton: {
      variants: [
        {
          { props: { color: 'primary', customProp: true },
          style: {
            textTransform: 'none',
            border: `2px dashed grey${blue[500]}`,
+           '& .MuiButton-startIcon': {
+             textDecoration: 'underline',
+           },
          },
        },
      ],
    },
  },
});

I was exploring even new API with #28107, but it is not really necessary at this moment.

The main motivation for this is that we should not have two APIs for doing the same thing. We could not drop the stylesOverrides when they were added, as it would require tremendous work in terms of breaking changes, but we could plan it for v6. If we agree that this is our long term goal, it doesn't make sense to introduce more API to it.

---

I will leave the issue open still to see if there will be any traction.

[hrgui]

Allowing callbacks when defining styleOverrides supports the use case if developers want to deviate or augment from the material-ui theme standard and extend on top of it.

For example, suppose if we had a primaryFontFamily and a secondaryFontFamily for typography, rather than just 1 fontFamily. Let's make the Button the secondaryFontFamily.

let theme = createTheme({
  typography: {
    primaryFontFamily: '"Times New Roman", serif',
    secondaryFontFamily: 'cursive'
  },
  palette: {
    primary: {
      // Purple and green play nicely together.
      main: purple[500]
    },
    secondary: {
      // This is green.A700 as hex.
      main: "#11cb5f"
    }
  }
});

theme = createTheme(theme, {components: {
  MuiButton: {
    styleOverrides: {
      root: {
        fontFamily: theme.typography.secondaryFontFamily
      }
    }
  }
}});

While this does make the Button secondaryFontFamily, suppose we want to change secondaryFontFamily to something else only for a certain page or site. If we did the following within the ThemeProvider:

<ThemeProvider theme={createTheme(theme, {typography: {secondaryFontFamily: "'sans-serif'"}})}>
</ThemeProvider>

It does not change the Button despite reading from theme.typography.secondaryFontFamily. We would need to make a completely custom Button component in order to support this functionality.

To avoid making a completely custom Button, we can support callbacks when defining styleOverrides:

const theme = createTheme(
{
  typography: {
    primaryFontFamily: '"Times New Roman", serif',
    secondaryFontFamily: 'cursive'
  },
  palette: {
    primary: {
      // Purple and green play nicely together.
      main: purple[500]
    },
    secondary: {
      // This is green.A700 as hex.
      main: "#11cb5f"
    }
  },
components: {
  MuiButton: {
    styleOverrides: {
      root:({theme}) => ({
        fontFamily: theme.typography.secondaryFontFamily
      })
    }
  }
});

Here's a sandbox that provides why this use-case is useful:

https://codesandbox.io/s/createtheme-supports-a-callback-usecase-tuibt?file=/demo.tsx

Is it possible to do the same codesandbox with variants?

[mnajdova]

@hrgui yes this is already possible, see https://codesandbox.io/s/createtheme-supports-a-callback-usecase-forked-h9zkc?file=/demo.tsx

[jakelauer]

@mnajdova Looks like that case (using a callback in the variants styles) is not included in the TypeScript types, as you can see in your example which shows theme as any.

[mnajdova]

@jakelauer thanks for pointing this out. The second arg of createTheme is object so we cannot really do anything there, but I have improved the types for the first argument in #29610. Also, if you take a look on the tests in #29610 you will notice that we don't even need second argument any more.

[oliviertassinari]

Currently we have two APIs for overrides in the theme [...] In my opinion, we should be dropping the styleOverrides long term in favor of the variants. Why?

@mnajdova but are the use cases equivalent? One seem to be about customizing all the instances on different slots, while the other to adapt the root slot style to a specific combination of props.

It makes me think of this #29581 (comment)

[mnajdova]

all the instances on different slots

@oliviertassinari I don't understand this really.

adapt the root slot style to a specific combination of props.

This makes me think of #28107, currently is possible using nested selectors.

---

One API should be enough long term, I cannot imagine scenario that is possible with the stylesOverrides and it is not with the variants.

I would lean towards closing this issue to avoid confusion to be honest.

[jakelauer]

As long as there is a callback option, I believe one API is sufficient. My use case was in attempting to use the current theme's color specifications while modifying the styles, i.e. "set button background to semi-transparent for the current theme's color" is a use-case that requires callbacks because it can't be achieved without access to the theme's color.

[Brodzko]

Just a question regarding #29610 - I tried this in our app with casting everything to any and in runtime it does indeed work, but in addition, I get access to all props of the component, including ownerState which I inferred is how components are styled internally from glancing over a few of them.

Given this, is there a specific reason why variants need to have props as a matcher for when to apply? Wouldn't it indeed be simpler to support a callback like this in style, call it always and leave the logic on when to apply what styles on the implementer?

I suppose this is what styleOverrides as a callback would accomplish? I think I'm starting to get why this discussion is taking place.

[mnajdova]

@Brodzko thanks for testing this out. I intentionally typed only the theme in the callback, so that people would fall back into using the props key. There are multiple benefits:

• the information about when some styles are applied can be statically extracted (as the props are object)
• it's declarative
• it opens room for potentially introducing caching based on the values without the need to recalculate the styles

If we ignore these points, we may as well remove the key from the theme, as the same can be achieved using the styled() utility more or less :)

[Brodzko]

• the information about when some styles are applied can be statically extracted (as the props are object)
• it's declarative
• it opens room for potentially introducing caching based on the values without the need to recalculate the styles

@mnajdova I agree all these points, however I don't think going with styled would have the same effect, particularly regarding DX. I prefer using MUI components directly with no wrappers, even if they are simple styled wrappers.

Example:

• We have added a new variant for Chip called twoTone using ChipPropsVariantOverrides
• We still want the Chip to work with all possible color options.
• Chip's styled directly depend on chosen color

Having to define each possible combination of props: { variant: 'twoTone', color: ${insert-color} } is impractical and simply accessing it from props (ownerState?) in the style callback under one props: { variant: 'twoTone' } would be better.

But of course then we're back at why keep the props matcher at all - or at least when to use which approach 🤷 . If this approach is inherently wrong for some reason, I'm happy to learn what would be a better way to go.

[mnajdova]

This is one way of how I would go about it. You can spread this in the variants. We can debate if this is more readable or not, but it is an option.

  ['primary', 'secondary', 'danger'].map(
    (color) => ({ 
      props: { variant: 'twoTone', color },
      style ({ theme }) => ({ background: theme.palette[color].main})
    })
  ),

I would wait with updating the types to accept all props, until we actually see a valid reason that it is needed.

[Darhagonable]

Shouldn't the typing match what's actually provided? If only the theme is allowed as as argument in the callback shouldn't the api itself only give you that and nothing else?

My understanding from the corresponding PR is that styleOverrides is deprecated and replaced with variants even though styleOverrides is still being used in the docs?

An example use case I don't see a typescript solutions for is when you want the a button to use the main and light shade of the color prop like here:

• CodeSandbox styleOverrrides version: https://codesandbox.io/s/createtheme-styleoverrides-callback-type-error-9gqwl?file=/demo.tsx
• CodeSandbox variants version: https://codesandbox.io/s/createtheme-variants-style-callback-type-error-79sjt?file=/demo.tsx

One wants this working even for custom palettes one adds.

Am i missing something obvious?

[Darhagonable]

This is one way of how I would go about it. You can spread this in the variants. We can debate if this is more readable or not, but it is an option.

  ['primary', 'secondary', 'danger'].map(
    (color) => ({ 
      props: { variant: 'twoTone', color },
      style ({ theme }) => ({ background: theme.palette[color].main})
    })
  ),

I would wait with updating the types to accept all props, until we actually see a valid reason that it is needed.

I guess you mean like this? Now variants are getting incompatible typing. and the code gets very ugly compared to passing the color prop as callback argument.
https://codesandbox.io/s/colorprop-mapping-version-typeerror-16m79?file=/demo.tsx