Open DiegoAndai opened 4 months ago
@mnajdova @siriwatknp @brijeshb42 what do you think about this one?
We could also plan to make the CssVarsProvider the only provider in v7. If we wish to do that, we would have to stabilize it in v6 as well as deprecate the ThemeProvider, marking it for removal in v7.
Involved discussion: mui/pigment-css#127
I agree to stabilize the API. The big task left is to handle the performance. createTheme
takes ~0.x ms but extendTheme
takes ~2-5ms.
The best way I found to optimize this (for MUI system) is that the extendTheme
should be called at build-time via a CLI.
In that case, a user friendly way to do this would be to have a bundler plugin to inject the themes. It would be similar to a subset of what the zero runtime plugin does.
I added an overview of the steps to the description. I also added it to the v6 milestone. It doesn't require breaking changes, but we have to deprecate ThemeProvider
as soon as possible to give users time to adjust. Because of this, we'll schedule this work alongside the v6 work.
@siriwatknp, May I assign this one to you? You seem to have better context for it right now, having worked on the CssVarsProvider and the zero-runtime side.
@siriwatknp, May I assign this one to you? You seem to have better context for it right now, having worked on the CssVarsProvider and the zero-runtime side.
Yes, please assign it to me.
I dove into the logic in: https://github.com/mui/material-ui/pull/41223.
The behavior of the returned value by:
const { mode, systemMode } = useColorScheme();
is probably not right. I would expect these two should be undefined/null on the first render, unless { noSsr: true }
is provided as an option. Otherwise, how are you supposed to handle hydration? The main downside is that it mean that the component renders twice, but this is more sound. And developers could still use { noSsr: true }
like useMediaQuery
provides.
Also, I noticed that we don't memo the theme when we update the mode-related value. This means that we render all the descendants of
Deprecate ThemeProvider in favor of CssVarsProvider
Are we sure about this? It doesn't sound compatible with keeping emotion running on the side.
Deprecate ThemeProvider in favor of CssVarsProvider
I don't think we need to deprecate ThemeProvider
, it's still useful for testing purpose.
It doesn't sound compatible with keeping emotion running on the side.
Why? The current CssVarsProvider works with Emotion.
I don't think we need to deprecate ThemeProvider, it's still useful for testing purpose.
The purpose of deprecating it would be to be able to eventually remove it, to avoid confusing users about which one they should use, and to avoid duplicating work on maintaining and supporting two different theme providers.
Hey @siriwatknp! I'm looking to organize the missing work to close this issue, on the weekly meeting we discussed with the team the following tasks:
The last two are to avoid users becoming confused with two providers. We want them to use CssVarsProvider.
Do you agree with these remaining tasks? Did I miss something? If you agree we can go ahead and create separate issues for them.
Optimize extendTheme (https://github.com/mui/material-ui/issues/41070#issuecomment-1945346567). Is this still needed? Or was it fixed already?
Pigment CSS fixes this at build time, so I think we can close the issue.
Update the docs to replace ThemeProvider with CssVarsProvider
Do you mean all of the places in the demos?
Deprecate ThemeProvider in favor of CssVarsProvider
Got it. Sounds good to me.
Do you mean all of the places in the demos?
Yes
Got it. Sounds good to me.
Should we create separate issues for these?
As we will rely heavily on component tokens with Material Design 3 implemented through CSS variables, we should stabilize the CssVarsProvider API and remove the "Experimental" flag.
Steps
v6
extendTheme
: https://github.com/mui/material-ui/issues/41070#issuecomment-1945346567ThemeProvider
in favor ofCssVarsProvider
ThemeProvider
withCssVarsProvider
v7
ThemeProvider
Search keywords: CssVarsProvider experimental stable