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.02k stars 32.3k forks source link

Extending the outer theme with a nested ThemeProvider throws an error if css variables are active #44429

Open mattstobbs opened 5 days ago

mattstobbs commented 5 days ago

Steps to reproduce

Steps:

  1. Open this link to live example: (required) https://codesandbox.io/p/sandbox/exciting-rhodes-zk9kwk
  2. Enable css variables in the outer ThemeProvider
  3. Nest a ThemeProvider within the outer ThemeProvider
  4. Attempt to extend the outer theme following the guidance in Theming (the codesandbox is adapted straight from that example)
  5. When createTheme is called with the outer theme spread into it, an error is thrown. This seems to be because the outer theme has a vars field, which is not allowed as an input to createTheme.

Current behavior

The following error is thrown:

MUI: `vars` is a private field used for CSS variables support.
Please use another name.

Expected behavior

An error should not be thrown and the outer theme should be able to be extended even if CSS variables are enabled.

Context

We're trying to extend the outer theme with additional styles, as described in the docs.

I'm happy to raise a PR to address this issue, once a fix has been agreed.

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: Extending theme

siriwatknp commented 4 days ago

The goal of having CSS variables is to have a single instance of createTheme for your app. If you want to customize some part of the app with different values, you can override some tokens to a specific scope:

<ThemeProvider theme={theme}>

  ….

      <div style={{ '--mui-palette-primary-main': … }}>
        …
      </div>

</ThemeProvider>

create nested themes would create a huge size of stylesheet with duplicate tokens.

However, if you really want to do it, you can get away with this:

<ThemeProvider
  theme={(theme: Theme) => {
    const { vars, …rest } = theme;
    return createTheme({
      ...rest,
      palette: {
        ...rest.palette,
        primary: {
          main: green[500],
        },
      },
    })
  }}
>
  <Checkbox defaultChecked />
  <Checkbox defaultChecked color="secondary" />
</ThemeProvider>
mattstobbs commented 4 days ago

@siriwatknp Thanks for your reply!

Your response answers the case in the attached sandbox. For additional context for the situation we're in, we're not looking to override the tokens within the inner theme provider, we mostly override the components (e.g. set the default variant for text fields to "outlined"). Destructuring vars from the theme works well in that case though.

aarongarciah commented 4 hours ago

@mattstobbs yeah, you can override component props in nested theme providers as shown in this demo: https://codesandbox.io/p/sandbox/clever-ives-f42vn3?workspaceId=d6fa470f-2079-42a1-aa65-469b49bbde6c

@siriwatknp I wonder if we could improve the logic to avoid throwing an error. I'd expect to be able to spread one theme object into another. I wonder if ignoring vars when passed to createTheme and throwing a warning instead of an error could be better.