[POC] Fix Interpolation types

[siriwatknp]

• I have followed (at least) the PR section of the contributing guide.

[mui-bot]

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 1537b32

[siriwatknp]

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?

[DiegoAndai]

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

[siriwatknp]

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

[DiegoAndai]

What do you mean with "myth"?

[siriwatknp]

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.

[DiegoAndai]

Thanks for taking a look at this @siriwatknp! 😊

I have a couple of questions

[DiegoAndai]

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

[siriwatknp]

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

[DiegoAndai]

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.

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

[siriwatknp]

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).