mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.04k stars 32.31k forks source link

Theme Composition doesn't work with css variables option #44134

Closed astjohnpaycor closed 10 hours ago

astjohnpaycor commented 1 month ago

Steps to reproduce

Link to live example: https://codesandbox.io/p/sandbox/muithemebug-2cllq4

Steps:

  1. Use createTheme function and pass cssVariables option in as true
  2. Use createTheme again, with the theme that you just created passed in, with the second parameter being the new arguments that you want to merge into the theme.
  3. See error.

Current behavior

The theme is broken because of this error "MUI: vars is a private field used for CSS variables support. Please use another name." and then cannot be used. This is because when the first one gets created it strips out the "cssVariables" argument and creates the vars key and values. So when you use createTheme again, it thinks that it is using the createThemeNoVars option and there's no way to tell it otherwise, and so complains that you used the reserved vars keyword.

Expected behavior

Per the theme composition instructions that are just below this anchor: https://mui.com/material-ui/customization/theming/#createtheme-options-args-theme

I would expect that I am able to build a theme that is based on the values of the previous theme, while also using css variables. This works as expected when you are not using css variables.

Context

We are trying to use theme composition to set styles that are based on the values being created by the theme, e.g. breakpoints, and we also want to be able to use css variables in our theme.

Your environment

npx @mui/envinfo ``` System: OS: macOS 15.0 Binaries: Node: 20.15.1 - /opt/homebrew/opt/node@20/bin/node npm: 10.7.0 - /opt/homebrew/opt/node@20/bin/npm pnpm: Not Found Browsers: Chrome: 130.0.6723.59 Edge: 129.0.2792.89 Safari: 18.0 ``` I primarily use Firefox Developer Edition, but this is not a browser issue.

Search keywords: createTheme cssVariables composition

siriwatknp commented 1 month ago

@mnajdova I am not convinced that this is a bug. It's more a docs improvement because you'd want to set cssVariables: true at the last createTheme for highest performant.

@astjohnpaycor Does the change below work for your use case?

let theme = createTheme({
-  cssVariables: true,
  typography: {
    h1: {
      fontFamily: "Verdana, Arial, sans-serif",
    },
  },
});

theme = createTheme(theme, {
+  cssVariables: true,
  typography: {
    h1: {
      [theme.breakpoints.only("lg")]: {
        fontSize: "2.5rem",
        lineHeight: "106%",
      },
    },
  },
});
astjohnpaycor commented 1 month ago

@siriwatknp No, that doesn't work because the theme has already been created when you use the subsequent createTheme calls, so passing in the cssVariables property later doesn't do anything. It doesn't go back through and recreate the theme with the css variables. If that is expected behavior, then that would also be a bug, but I don't think it's expected behavior since this does clarify that only the first argument gets processed and that you have to do deep merging if you're trying to fully re-create the theme with all the options: https://mui.com/material-ui/customization/theming/#createtheme-options-args-theme

siriwatknp commented 1 month ago

@astjohnpaycor Can you explain in more details about your use case for composing themes multiple times?

astjohnpaycor commented 1 month ago

@siriwatknp Yes, just like the example in the docs we want to use the values that are being created by the theme to compose additional theme values. One example is with the breakpoints, like in the sandbox example above, but other examples include the one from the documentation where we want to use the theme palette colors while composing other styling values, especially since MUI is filling in values that we aren't specifically overriding.

import { createTheme } from '@mui/material/styles';

let theme = createTheme({
  palette: {
    primary: {
      main: '#0052cc',
    },
    secondary: {
      main: '#edf2ff',
    },
  },
});

theme = createTheme(theme, {
  palette: {
    info: {
      main: theme.palette.primary.dark,
    },
  },
});
siriwatknp commented 1 week ago

Sorry, it should be this:

let theme = createTheme({
-  cssVariables: true,
  typography: {
    h1: {
      fontFamily: "Verdana, Arial, sans-serif",
    },
  },
});

theme = createTheme({
  …theme,
+  cssVariables: true,
  },
  {
  typography: {
    h1: {
      [theme.breakpoints.only("lg")]: {
        fontSize: "2.5rem",
        lineHeight: "106%",
      },
    },
  },
});
github-actions[bot] commented 10 hours ago

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.