[styled-engine] Fix style overrides variants type #45478

siriwatknp · 2025-03-04T11:53:14Z

The issue is that Omit<CSSOthersObject, 'variants'> doesn't work as expected as CSSOthersObject is a mapped type, which Omit has known issues with:

"in keyof" type mapping incorrectly interacts with indexers microsoft/TypeScript#42436
Using keyof with key remapping to exclude index signatures doesn't return known keys microsoft/TypeScript#41966

In a nutshell: By design, Omit won't remove variants from CSSOthersObject, so the type defined in CSSObjectWithVariants is not used.

Solution and discussion below

mui-bot · 2025-03-04T11:55:44Z

Netlify deploy preview

https://deploy-preview-45478--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against e91681e

siriwatknp · 2025-03-05T02:47:12Z

docs/data/material/getting-started/templates/marketing-page/components/Features.js

@@ -42,7 +42,7 @@ const items = [
 const Chip = styled(MuiChip)(({ theme }) => ({
  variants: [
    {
-      props: ({ selected }) => selected,
+      props: ({ selected }) => !!selected,


Have to fix this because the type of selected is boolean | undefined.

Should we loosen the type like of props? to be () => boolean | null | undefined?

I don't mind the !! to cast into boolean

Does this mean this is a breaking change?

siriwatknp · 2025-03-05T02:50:56Z

packages/mui-styled-engine/src/index.d.ts

+  | ComponentSelector
+  | Keyframes
+  | SerializedStyles
+  | CSSPropertiesWithMultiValues


This is a myth to me. If not adding CSSPropertiesWithMultiValues explicitly as a union, the spec fail for recreating nested theme.

What do you mean with "myth"?

At first, I thought that CSSObject is enough because it extends CSSPropertiesWithMultiValues. However, if I excluded CSSPropertiesWithMultiValues from the union type, I got this error from createTheme.spec:

Here is the full error:

Argument of type 'Theme' is not assignable to parameter of type 'Omit<ThemeOptions, "components"> & Pick<CssVarsThemeOptions, "colorSchemes" | "defaultColorScheme" | "components"> & { ...; }'. Type 'Theme' is not assignable to type 'Pick<CssVarsThemeOptions, "colorSchemes" | "defaultColorScheme" | "components">'. Types of property 'components' are incompatible. Type 'Components<BaseTheme> | undefined' is not assignable to type 'Components<Omit<Theme, "palette" | "components"> & CssVarsTheme> | undefined'. Type 'Components<BaseTheme>' is not assignable to type 'Components<Omit<Theme, "palette" | "components"> & CssVarsTheme>'. Types of property 'MuiAlert' are incompatible. Type '{ defaultProps?: Partial<AlertProps> | undefined; styleOverrides?: Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", BaseTheme>> | undefined; variants?: { ...; }[] | undefined; } | undefined' is not assignable to type '{ defaultProps?: Partial<AlertProps> | undefined; styleOverrides?: Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", Omit<Theme, "palette" | "components"> & CssVarsTheme>> | undefined; variants?: { ...; }[] | undefined; } | undefined'. Type '{ defaultProps?: Partial<AlertProps> | undefined; styleOverrides?: Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", BaseTheme>> | undefined; variants?: { ...; }[] | undefined; }' is not assignable to type '{ defaultProps?: Partial<AlertProps> | undefined; styleOverrides?: Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", Omit<Theme, "palette" | "components"> & CssVarsTheme>> | undefined; variants?: { ...; }[] | undefined; }'. Types of property 'styleOverrides' are incompatible. Type 'Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", BaseTheme>> | undefined' is not assignable to type 'Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", Omit<Theme, "palette" | "components"> & CssVarsTheme>> | undefined'. Type 'Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", BaseTheme>>' is not assignable to type 'Partial<OverridesStyleRules<keyof AlertClasses, "MuiAlert", Omit<Theme, "palette" | "components"> & CssVarsTheme>>'. Types of property 'root' are incompatible. Type 'Interpolation<AlertProps & Record<string, unknown> & { ownerState: AlertProps & Record<string, unknown>; } & { theme: BaseTheme; }>' is not assignable to type 'Interpolation<AlertProps & Record<string, unknown> & { ownerState: AlertProps & Record<string, unknown>; } & { theme: Omit<Theme, "palette" | "components"> & CssVarsTheme; }>'. Type 'CSSObject & { variants?: { props: Partial<AlertProps & Record<string, unknown> & { ownerState: AlertProps & Record<string, unknown>; } & { ...; }> | ((props: Partial<...> & { ...; }) => boolean); style: CSSObject | ((args: { ...; }) => CSSObject); }[] | undefined; }' is not assignable to type 'Interpolation<AlertProps & Record<string, unknown> & { ownerState: AlertProps & Record<string, unknown>; } & { theme: Omit<Theme, "palette" | "components"> & CssVarsTheme; }>'. Type 'CSSObject & { variants?: { props: Partial<AlertProps & Record<string, unknown> & { ownerState: AlertProps & Record<string, unknown>; } & { ...; }> | ((props: Partial<...> & { ...; }) => boolean); style: CSSObject | ((args: { ...; }) => CSSObject); }[] | undefined; }' is not assignable to type 'Keyframes'. Type 'CSSObject & { variants?: { props: Partial<AlertProps & Record<string, unknown> & { ownerState: AlertProps & Record<string, unknown>; } & { ...; }> | ((props: Partial<...> & { ...; }) => boolean); style: CSSObject | ((args: { ...; }) => CSSObject); }[] | undefined; }' is missing the following properties from type '{ name: string; styles: string; anim: number; toString: () => string; }': name, styles, animts(2345)

I don't fully understand why.

The error is not related too much to the type that's being added, this is suspicious, let me check it again locally and I will report back.

Based on this type: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/styles/createTheme.ts#L38 the error is expected in my opinion. The issue is that in createTheme, we return this type: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/styles/createThemeNoVars.d.ts#L82 which has different definition for components than the input of createTheme. We should probably loosen the input type for createTheme, to accept a Theme, if we are allowing this use-case. We should also make sure that all proposed options from https://mui.com/material-ui/customization/theming/#nesting-the-theme are type safe.

DiegoAndai

Thanks for taking a look at this @siriwatknp! 😊

I have a couple of questions

DiegoAndai · 2025-03-05T17:16:44Z

packages/mui-styled-engine/src/index.d.ts

+  | Keyframes
+  | SerializedStyles
+  | CSSPropertiesWithMultiValues
+  | (CSSObject & {


Is there a benefit in inlining the variants here instead of defining it separately?

This is required, otherwise variants is not type-safety. You can try in this Playground

…ltiValues

DiegoAndai · 2025-03-07T18:58:43Z

Hey @siriwatknp! I took a look at this, and I managed to remove CSSPropertiesWithMultiValues from Interpolation by removing components from the Theme interface, see daa9579.

The CssVarsTheme and Joy's Theme types do not have the components property, so I wonder if it being included in Material's Theme was a mistake.

Do you think this is the correct approach?
This fixed the createTheme(theme issue, but it brought up a bunch of other ones 😅 I will continue taking a look on Monday

siriwatknp · 2025-03-10T02:08:08Z

Hey @siriwatknp! I took a look at this, and I managed to remove CSSPropertiesWithMultiValues from Interpolation by removing components from the Theme interface, see daa9579.

I suggest not removing it as it will introduce another issue for people who read theme.components.* like this:

const theme = createTheme({
  component: { … }
})

console.log(theme.components); // << TS error.

Also, removing the components is technically incorrect because the theme object contains components node (in JS).

…esWithMultiValues" This reverts commit daa9579.

DiegoAndai · 2025-03-14T19:10:10Z

@siriwatknp I closed #45346 in favor of this PR, lets move forward with this solution

@mnajdova may I ask you to review this?

mnajdova

I will report soon with my findings.

mnajdova · 2025-03-24T15:29:45Z

docs/data/material/getting-started/templates/marketing-page/components/Features.js

@@ -42,7 +42,7 @@ const items = [
 const Chip = styled(MuiChip)(({ theme }) => ({
  variants: [
    {
-      props: ({ selected }) => selected,
+      props: ({ selected }) => !!selected,


Does this mean this is a breaking change?

mnajdova · 2025-03-24T15:30:58Z

packages/mui-material/src/styles/createTheme.spec.ts

@@ -218,6 +218,7 @@ const theme = createTheme();
            variants: [
              {
                props: ({ ownerState }) => ownerState.color === 'primary',
+                style: {},


Wasn't style required before?

mnajdova · 2025-03-24T15:47:40Z

packages/mui-styled-engine/src/index.d.ts

+  | ComponentSelector
+  | Keyframes
+  | SerializedStyles
+  | CSSPropertiesWithMultiValues


The error is not related too much to the type that's being added, this is suspicious, let me check it again locally and I will report back.

poc

9618878

siriwatknp added the typescript label Mar 4, 2025

siriwatknp added 4 commits March 4, 2025 21:52

fix types

04ea73d

docs ts format

cade9e8

fix types

d7ec526

fix types

1537b32

siriwatknp commented Mar 5, 2025

View reviewed changes

DiegoAndai reviewed Mar 5, 2025

View reviewed changes

Test alternative fix which doesn't require adding CSSPropertiesWithMu…

daa9579

…ltiValues

DiegoAndai added 2 commits March 14, 2025 15:50

Revert "Test alternative fix which doesn't require adding CSSProperti…

cbe257a

…esWithMultiValues" This reverts commit daa9579.

Merge branch 'master' into fix/styleoverrides-variants-type

e91681e

DiegoAndai mentioned this pull request Mar 14, 2025

[styled-engine] Fix style overrides variants type #45346

Closed

DiegoAndai changed the title ~~[POC] Fix Interpolation types~~ styled-engine] Fix style overrides variants type Mar 14, 2025

DiegoAndai changed the title ~~styled-engine] Fix style overrides variants type~~ [styled-engine] Fix style overrides variants type Mar 14, 2025

DiegoAndai requested a review from mnajdova March 14, 2025 19:09

DiegoAndai self-assigned this Mar 14, 2025

DiegoAndai marked this pull request as ready for review March 14, 2025 19:09

mnajdova reviewed Mar 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[styled-engine] Fix style overrides variants type #45478

[styled-engine] Fix style overrides variants type #45478

siriwatknp commented Mar 4, 2025 •

edited by DiegoAndai

Loading

mui-bot commented Mar 4, 2025 •

edited

Loading

siriwatknp Mar 5, 2025

DiegoAndai Mar 5, 2025

mnajdova Mar 24, 2025

siriwatknp Mar 5, 2025

DiegoAndai Mar 5, 2025

siriwatknp Mar 6, 2025 •

edited

Loading

mnajdova Mar 24, 2025

mnajdova Mar 24, 2025

DiegoAndai left a comment

DiegoAndai Mar 5, 2025

siriwatknp Mar 6, 2025 •

edited

Loading

DiegoAndai commented Mar 7, 2025

siriwatknp commented Mar 10, 2025 •

edited

Loading

DiegoAndai commented Mar 14, 2025

mnajdova left a comment

mnajdova Mar 24, 2025

mnajdova Mar 24, 2025

mnajdova Mar 24, 2025

[styled-engine] Fix style overrides variants type #45478

Are you sure you want to change the base?

[styled-engine] Fix style overrides variants type #45478

Conversation

siriwatknp commented Mar 4, 2025 • edited by DiegoAndai Loading

mui-bot commented Mar 4, 2025 • edited Loading

Netlify deploy preview

Bundle size report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoAndai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

DiegoAndai commented Mar 7, 2025

siriwatknp commented Mar 10, 2025 • edited Loading

DiegoAndai commented Mar 14, 2025

mnajdova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp commented Mar 4, 2025 •

edited by DiegoAndai

Loading

mui-bot commented Mar 4, 2025 •

edited

Loading

siriwatknp Mar 6, 2025 •

edited

Loading

siriwatknp Mar 6, 2025 •

edited

Loading

siriwatknp commented Mar 10, 2025 •

edited

Loading