[CssVarsProvider][Material UI] Exclude dark mode variables from :root stylesheet

[siriwatknp]

close #34084

Before: https://codesandbox.io/s/material-ui-forked-84f6p7?file=/index.tsx

After: https://codesandbox.io/s/create-react-app-with-typescript-forked-nqvqnu?file=/src/index.tsx

Root cause

There are CSS variables that are intended to be used in dark mode only:

• overlays-{number}
• palette-AppBar-darkBg
• palette-AppBar-darkColor

Currently, the application works perfectly if the default mode is light because the above variables exist only in dark stylesheet

:root {...}

[data-color-scheme="dark"] {
  --mui-overlays-1: ...; // only available in dark mode
  ...
}

However, if the defaultMode="dark" the variables are applied to the :root stylesheet as well which causes the Paper and AppBar that consume those variables to behave differently

Changes

This PR introduces an internal option in MUI System which lets Material UI excludes those variables from being generated to the :root stylesheet.

Alternative

The other way to solve this is to provide those variables in light mode with unknown values like this:

setColor(palette.AppBar, 'darkBg', 'var(--unknown')); 
setColor(palette.AppBar, 'darkColor', var(--unknown')); 

The downside is that it increases the bundle size (+26 variables) which does not sound right to me. Excluding them from the :root stylesheet sounds like a better solution.

---

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

[mui-bot]

Details of bundle changes

@material-ui/core: parsed: +0.05% , gzip: +0.07%

Generated by 🚫 dangerJS against f8884d3

[mnajdova]

Could we add test in the CssVarsProvider about this logic? Also, how do we make sure we remove the variables only in light mode? Adding tests for both cases would be great.

I would argue that the alternative approach is easier to learn and apply rather than adding new API, and at this moment bundle size doesn't really concerns me that much.

[siriwatknp]

Could we add test in the CssVarsProvider about this logic? Also, how do we make sure we remove the variables only in light mode? Adding tests for both cases would be great.

Added 2 regression tests

• Test the default dark mode to ensure that the variables exist in dark mode.
• The default mode is dark and then switches to light to ensure that those variables does not exist in light mode.

I would argue that the alternative approach is easier to learn and apply rather than adding new API, and at this moment bundle size doesn't really concerns me that much.

It might create confusion for developers and be prone to errors:

• Introducing the unknown variables (do we need to discuss the name?)
• Need to document the unknown variables
• Developer could accidentally remove the variables in the light mode which will cause unexpected result.
• Bundle size will grow if you have more than light and `dark.

I rather have this done internally, so developers do not need to worry about these edge cases.

[mnajdova]

I am wondering if people will ever need this as feature on their side. If yes, we could export this so that they can extend/override it.

[mnajdova]

Left some comments for consideration in the future.