search

mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.04k stars 31.64k forks source link

[RFC] v5 styling solution 💅 #22342

Closed mnajdova closed 3 years ago

mnajdova commented 3 years ago

This RFC is a proposal for changing the styling solution of Material-UI in v5.

TL:DR; the core team proposes we go with emotion

What's the problem?

What are the requirements?

Whatever styling engine we choose to go with we have to consider the following factors:

It would be nice if it can support the following:

What are our options?

Comparison

Performance

Here are benchmarks with dynamic styles of several popular libraries (note the Material-UI v4 only use static styles which have good performance):

PR for reference: https://github.com/mnajdova/react-native-web/pull/1

Based on the performance, I think that we should eliminate: JSS (currently wrapped in @material-ui/styles), styletron, and fela. That would leave us with:

Dynamic props

Based on the open issues, it seems that Aphrodite doesn't support dynamic props: https://github.com/Khan/aphrodite/issues/141 which in my opinion means that we should drop that one from our options too, leaving us with:

npm

While styled-components and emotion are both libraries are pretty popular, react-styletron at the time or writing is much behind with around 12500 downloads per week (this in my opinion is a strong reason why we should eliminate it, as if we decide to go with it, the community will again need to have two different styling engine in their apps).

Here is the list rang by the number of Weekly downloads at the time of writing:

Note that storybook has a dependency on emotion. It significantly skews the stats.

Support concurrent mode

SSR

Stars

Trafic on the documentation

SimilarWeb estimated sessions/month:

Users feedback

Based on the survey, 53.8% percent are using the Material-UI styles (JSS), which is not a surprise as it is the engine coming from Material-UI. However, we can see that 20.4% percent are already using styled-components, which is a big number considering that we don't have direct support for it. Emotion is used by around 1.9% percent of the developers currently based on the survey.

Having these numbers we want to push with better support for styled-components, so this is something we should consider.

Browser support

Bundle size

What's the best option?

Default engine

Even if we decide to support multiple engines, we would still need to advocate for one by default and have one documented in the demos.

styled-components

Pros:

Cons:

emotion

Pros:

Cons:

Support multiple

We may try to support multiple CSS-in-JS solutions, by providing our in house adapters for them. Some things that we need to consider is that, that we may have duplicate work on the styles, as the syntax is different between them (at least jss compared to styled-components/emotion). We will reuse the theme object no matter what solution we will pick up.

The less involved support for this may come from the usage of the styled, as people may do some webpack config to decide which one to use - (this is just something to consider).

Additional comments

Deterministic classnames on the components that can be targeted for custom styles

Regarding how the classes look and how developers may target them, I want to show a comparison of what we currently have and how the problem can be solved with the new approach.

As an example, I will take the Slider component. Here is currently how the generated DOM look like:

Each of the classes has a very well descriptive semantic and people can use these classes for overriding the styles of the component.

On the other hand, emotion, styled-components or any other similar library will create some hash as a class name. For us to solve this and offer the developers the same functionality for targeting classes, each of the components will add classes that can be targeted by the developers based on the props.

This would mean that apart from the classes generated by emotion, each component will still have the classes that we had previously, like MuiSlider-root & MuiSlider-colorPrimary, the only difference would be that this classes will now be used purely as selectors, rather than defining the styles for the components. This could be implemented like a hook - useSliderClasses

Conclusion

No matter which solution we would choose, we would use the styled API, which is supported by the two of them. This will allow us down the road to have easier support for styled + unstyled components (probably with webpack aliases, like for using preact).

After we investigated the two options we had in the end, the core team proposes we go with emotion. Some key elements:

A small migration cost between styled-components and emotion

Developers already using styled-components should be able to use emotion with almost no effort.

There are different ways for adding overrides other than wrapper components

The support of cx + css from emotion can be beneficial for developers to use it as an alternative for adding style overrides if they don't want to create wrapper components.

Concurrent mode is for sure supported :+1:

Kudos to @ryancogswell for doing a deeper investigation on this topic. So far we did not find anything in @emotion's code that would give us concern that concurrent mode wouldn't work. We were also looking into createGlobalStyle from styled-components as a comparison to emotion's Global component. It is doing most of its work during render (inherently problematic for Strict/Concurrent Mode) and just using useEffect for removing styles in its cleanup function. createGlobalStyle needs a complete rewrite before it will be usable in concurrent mode -- it isn't OK for it to add styles during render if that render is never committed. It looks like someone has tried rewriting it with some further changes in the last month, so we will need to follow this progress.

How is the specificity handled

Emotion's docs recommend doing composition of CSS into a single class rather than trying to leverage styles from multiple class names. In v5, our existing global class names would be applied without any styles attached to them. The composition of emotion-styled components automatically combines the styles into a single class. This potentially gets rid of these stylesheet order issues at least internal to the styles defined by Material-UI, because every component's styles are driven by a single class name :+1:. So we would have the global class names (for developers to target in various ways for customizations) and then a single generated (by emotion) class name per element that would consolidate all the CSS sources flowing into it. Specificity is then handled by emotion based on the order of composition. All compositions using emotion (whether render-time or definition-time composition) results in a single class on the element. styled-components does NOT work this way concerning render-time composition (definition-time composition does get combined into a single class). The same composition in styled-components results in multiple classes applied to the same element and the specificity does not work as I would have intended.

Alternatives

What do you think about it?

eps1lon commented 3 years ago

which will decrease the rendering performance of Material UI and sites using it by a 3-5x factor.

You've created a benchmark that has this factor. It does not follow that every single component will show this decrease. Please try to avoid jumping to conclusions since this misleading information spreads very easily from such a visible issue.

Using the devtools extension I saw 140ms for mount with emotion vs 120ms for mount with makeStyles.

satazor commented 3 years ago

You've created a benchmark that has this factor. It does not follow that every single component will show this decrease. Please try to avoid jumping to conclusions since this misleading information spreads very easily from such a visible issue.

You are right, see my previous comment.

You've created a benchmark that has this factor. It does not follow that every single component will show this decrease. Please try to avoid jumping to conclusions since this misleading information spreads very easily from such a visible issue.

Using the devtools extension I saw 140ms for mount with emotion vs 120ms for mount with makeStyles.

I've updated the benchmark to use actualDuration instead of baseDuration, so we should now see values more inline with what is shown in the devtools profiler. The baseDuration measures the time without memoizations, while actualDuration is the opposite. Please note that this change only affects the rerender performance, and now I'm seeing makeStyles 6-8x faster for rerenders, which means it has better caching/memoization? However, I'm not getting the values you are seeing. Can you try with the updated links?

Screenshot 2020-09-22 092415 Screenshot 2020-09-22 092514

oliviertassinari commented 3 years ago

@satazor I don't think that your test cases are equivalent. We should be good.

A couple of changes you can try to understand that and that will reduce the difference of performance:

satazor commented 3 years ago

Indeed, @oliviertassinari. It seems the performance going forward with emotion/styled-components is more in the 1.5x-2x factor max even for simple components, such as Typography, which I have implemented here: https://codesandbox.io/s/css-in-js-comparison-forked-lr3sr?file=/src/components/EmotionTypography.js.

Check the production build & benchmark at: https://csb-lr3sr-7lp24bj5l.vercel.app/

makeStyles is 1.5-2x faster on mount and 3-4x faster on rerenders. This is potentially something to keep an eye on, but I guess the deviation will be much smaller for more complex components.

So, in conclusion, I'm no longer that worried about the performance and I'm looking forward to see how this effort will look like.

satazor commented 3 years ago

@mnajdova Here's the production build for the List test: https://csb-lr3sr-3zi42510w.vercel.app/?components=1000, copied of the PR you mentioned. Codesandbox link: https://codesandbox.io/s/css-in-js-comparison-forked-6s4nl

makeStyles seems 1.7x faster on mount, and 2.2x faster on rerender. I’m not getting the 10% difference you are seeing. Am I doing something wrong?

oliviertassinari commented 3 years ago

@satazor Interesting, thanks for putting it together. I have used this benchmark with #22435 to compare the performance of 

import Slider from '@material-ui/core/Slider';

vs (emotion).

import SliderStyled from '@material-ui/lab/SliderStyled';

vs (styled-components).

import SliderStyled from '@material-ui/lab/SliderStyled';
Capture d’écran 2020-09-23 à 17 23 23 Capture d’écran 2020-09-23 à 17 25 07 Capture d’écran 2020-09-23 à 17 23 54

At this point, it's hard to tell why it's slower. The bottleneck might not be on the styles. And mind that I have run it in dev mode ⚠️!

oliviertassinari commented 3 years ago

I have added two new pages to have a look at the performance of the WIP emotion version of the Slider:

Once built for production, the stats seem to be similar to https://github.com/mui-org/material-ui/issues/22342#issuecomment-697540546, it's about x1.6 slower (but CSS is fully dynamic). Note that you can play with it the WIP emotion version with:

Capture d’écran 2020-09-23 à 18 56 01 Capture d’écran 2020-09-23 à 18 55 48

Also, note that we know that we could make the current JSS version x1.1 faster by using makeStyles instead of withStyles: #15023.

satazor commented 3 years ago

@oliviertassinari in prod mode things get a bit faster, but I guess the differences stay there. You may click on deploy > vercel on code sandbox to quickly deploy with production build flags.

Looking at how makeStyles is implemented, I see how it is faster when you just use static styles:

  1. On every mount, attach gets called.
  2. If it's the first instance of that component type, stylesCreator.create gets called, which in turn calls fn specified in makeStyles(fn).
  3. On every other instance, stylesCreator.create is skipped because the ref count is > 0.

For rerenders:

  1. On every rerender, attach gets called.
  2. Afterwards, stylesCreator.create is skipped because ref count is > 0, so no work is done at all.

Please note that if dynamic styles are into play, the sheets would have been updated here, on every mount and rerender.

On the contrary, styled-components and emotion seem to run our styling functions on every component instance, which results in more CPU cycles and memory backpressure. I thought they could somehow do a multi memoize cache based on props combination, but this would assume that the styling functions are pure, which might not be the case, consider:

     const someContext = useContext(FooContext);

    return <div css={ { paddingTop: someContext.padding(1) } }>;

If my assumptions are right, it will be very hard to reach the performance level of makeStyles for static styles generation, as the way it works and how the API is designed allows for optimizations that styled or css API can't.

oliviertassinari commented 3 years ago

I see a couple of possible directions we could explore:

satazor commented 3 years ago

If somehow emotion exposed a useCss hook like it was requested in https://github.com/emotion-js/emotion/issues/1321 and https://github.com/emotion-js/emotion/issues/1853, we could retain makeStyles API and consequently, the benefits of "static CSS". However, we would keep using static CSS everywhere, to boost performance, which is not so "clean" but that's what we are already doing in v4.

satazor commented 3 years ago

Actually, with the ClassNames API, I think we could do a port of withStyles right now that would retain the benefits of the static CSS perf and the good performance of dynamic CSS that emotion has. If const css = useCss() existed, then it would be straightforward to create a useStyles API port as well.

The main advantages of keeping withStyles + makeStyles API which uses emotion under the hood are:

oliviertassinari commented 3 years ago

We have explored the performance issue further in https://twitter.com/olivtassinari/status/1309247827839680512. I think that we can move forward as is. For teams that care deeply about performance, they can opt-in into the unstyled components, they provide better performance. For the others, like most of the industry, emotion and styled-components are good enough.

https://codesandbox.io/s/slider-comparison-forked-jziv6?file=/src/App.js…

mifrej commented 3 years ago

Sorry that I bother you here so late in the discussion but I'm a bit surprised you are not looking closely at styled-jsx

Their list exactly met your requirements:

I would pay attention to the fact that it is almost a shadow CSS standard API. So by removing jsx attribute you are good to transition to web components in the future which are working already in the normal browsers BTW.

Yes I know it might be not the most popular one. But I just want to point out that Flash was very popular some years ago. But it dried out at the end not being web standard compliant, and we have SVGs now. Just for the record SVG standard was present long when Flash was ruling the industry. I just look at historical events as good lessons and would try to spot the pattern that popularity is the last indicator of being bulletproof maintainable and future proof.

oliviertassinari commented 3 years ago

@mifrej I personally had a bad experience with it: https://github.com/vercel/styled-jsx/issues/142.

re-thc commented 3 years ago

We have explored the performance issue further in https://twitter.com/olivtassinari/status/1309247827839680512. I think that we can move forward as is. For teams that care deeply about performance, they can opt-in into the unstyled components, they provide better performance. For the others, like most of the industry, emotion and styled-components are good enough.

  • native: 7.71ms
  • v5 unstyled: 20.89ms
  • v4: 29.93ms
  • v5: 37.37ms
  • antd: 53.48ms
  • chakra: 64.67ms

https://codesandbox.io/s/slider-comparison-forked-jziv6?file=/src/App.js…

Did you try the babel plugins for emotion / styled-components? Did it make a difference to the timings?

ianschmitz commented 3 years ago

What would a change from JSS mean for the classes prop available on existing MUI components? How would a v5 migration look for existing users that leverage this prop extensively?

mnajdova commented 3 years ago

We envision people to use the following syntax instead: https://next.material-ui.com/components/slider-styled/#unstyled-slider the classes are basically replaced by the class selectors available for each slot in the component. You can take a look also on the customization example: https://next.material-ui.com/components/slider-styled/#customized-sliders

As an advantage you can just use the props and apply dynamic styles based on them.

ConAntonakos commented 3 years ago

We envision people to use the following syntax instead: https://next.material-ui.com/components/slider-styled/#unstyled-slider the classes are basically replaced by the class selectors available for each slot in the component. You can take a look also on the customization example: https://next.material-ui.com/components/slider-styled/#customized-sliders

As an advantage you can just use the props and apply dynamic styles based on them.

I love this API! Very much a welcome change, and it seems to play really nicely with the style engines.

Before v5, if I recall correctly, the JSS compiler would mangle those class names and you couldn't reliably target them? But now it seems they'll be exposed for targeting purposes? 🙌 Also, there was the issue with CSS precedence. And those concerns are resolved with this new approach? Thank you for working on this refactor!

mnajdova commented 3 years ago

@ConAntonakos exactly these classes can be targeted for both the unstyled and styled versions of the components. The order of which styled is invoked ensures that the new styles will win, of course if the specificity is the same :)

eps1lon commented 3 years ago

Before v5, if I recall correctly, the JSS compiler would mangle those class names and you couldn't reliably target them?

You could already target them in v4.

the classes are basically replaced by the class selectors available for each slot in the component.

Are they "basically" replaced or actually replaced? I thought we decided to keep some of the old API to reduce the number of breaking changes.

mnajdova commented 3 years ago

I thought we decided to keep some of the old API to reduce the number of breaking changes.

We decided to keep the same API for the theme.components.overrides - overrides defined in the theme work in the same manner.

For the instance overrides, we have the styled approach now with the class selectors, that is why the classes prop is not supported anymore. Developers can do that same in more flexible manner now.

eps1lon commented 3 years ago

Developers can do that same in more flexible manner now.

That sounds like it's such a minor issue because there's an alternative but the migration cost for that is huge. How does the migration plan look like?

mnajdova commented 3 years ago

Developers can still use the theme overrides, if they just move the instance overrides to a nested ThemeProvider they wouldn't need to change the styles defined at all, as the structure between those two is the same (it's not perfect, but if they want incremental upgrade it is a way to go)

On the other hand, we can still support the classes prop easily, but in that case we cannot guarantee if the combination of classes + styled is used on the same component which one should win. Should we backport the support of classes at least on the styled version of the components?

ianschmitz commented 3 years ago

I may have missed this along the way through this thread - another related question to my classes question. What sort of "correctness" guarantees might there be?

For example i've edited the slider example:

const StyledSlider = styled(SliderUnstyled)`
  & .MuiSlider-raill {
    display: block;
    position: absolute;
    width: 100%;
    height: 2px;
    border-radius: 1px;
    background-color: currentColor;
    opacity: 0.38;
  }
)

You'll notice i accidentally misspelled MuiSlider-rail. Previously with classes there would be something like classes.rail, and if i misspelled the property, i would get a runtime warning that classes.raill doesn't exist in the stylesheet. I believe the Theme had the same behavior?

oliviertassinari commented 3 years ago

The advantages of the classes API I can think of:

  1. Edge against the global CSS selectors that leaks between children: e.g. In .css-e7ylz8 .MuiTreeItem-group. You have no guarantees that the component doesn't apply the style on a child component (not the effect you were expecting, a surprise!). Probably OK-ish for our components as we will be careful.
  2. Make handling portals a no-brainer: https://material-ui.com/guides/interoperability/#portals. For styling the tooltip, we will need to style the popper component, not the root as we did with the Slider.
  3. The $styleRule syntax warns if the style rule is undefined.
  4. Allow customizing the class names used. The current solution is to use the componentsProps prop. We merge the class names.

There is an alternative world where we would:

a. Allow to solve 1. and 3. with styled component selectors. b. Expose the classes API for backward compatibility. c. In order to get a. and b. working, we would need to flatten how the styles of the component are written internally. x styled component instead of 1 styled root. But ⚠️ with the perf implication.

Do we really need to do that?

I have looked at the history of jQuery UI's classes prop. I could find two issues: https://bugs.jqueryui.com/ticket/7053, https://bugs.jqueryui.com/ticket/8928 and the initial commit: https://github.com/jquery/jquery-ui/commit/c192d4086d9bbaf09d5f870857af30c60a427e22.

romainquellec commented 3 years ago

We envision people to use the following syntax instead: https://next.material-ui.com/components/slider-styled/#unstyled-slider the classes are basically replaced by the class selectors available for each slot in the component. You can take a look also on the customization example: https://next.material-ui.com/components/slider-styled/#customized-sliders

As an advantage you can just use the props and apply dynamic styles based on them.

Wow, this is the best thing ever. Unstyled or Headless components is going to be the best thing for customization (one of the critics about mui, I think). This is not a little thing to fix imo, but a hude plus for MUI.

PS : I remember some hard time customizing some components in the past PS2 : Did you take a look at https://github.com/modulz/stitches ?

mnajdova commented 3 years ago

You'll notice i accidentally misspelled MuiSlider-rail. Previously with classes there would be something like classes.rail, and if i misspelled the property, i would get a runtime warning that classes.raill doesn't exist in the stylesheet. I believe the Theme had the same behavior?

@ianschmitz in addition to the option @oliviertassinari suggested for using the styled components selectors, we have another option, which is to expose constants for the classes that we expose. Maybe something like:

import { sliderClasses } from '@material-ui/core/Slider';

const StyledSlider = styled(SliderUnstyled)`
  & .${sliderClasses.rail} {
    display: block;
    position: absolute;
    width: 100%;
    height: 2px;
    border-radius: 1px;
    background-color: currentColor;
    opacity: 0.38;
  }
)

It's not the same as the classes.rail runtime warning, but should help against misspelling the classes.

oliviertassinari commented 3 years ago

@mnajdova We could also consider an eslint plugin.

mnajdova commented 3 years ago

Regarding the support on the classes prop - in order for us to be able to reliably do this, we will have to create components for each part (slot) of the core component. For example, for the Slider, we would need to create components for the rail, track, mark, mark label, thumb and the value label. This will allow us to specify the styles directly on those components, instead of using classes for increasing the specificity. These changes can be find on this PR - https://github.com/mui-org/material-ui/pull/22893

With these changes, we've created a codesandbox, that can compare the perf of this new updated Slider component compared to the v4, native styled, unstyled, as well as two other competitive libraries - https://codesandbox.io/s/slider-comparison-with-multiple-components-2tytc?file=/package.json

If we compare these perf with the perf of having only one component and using the classes selectors for the parts of it - https://codesandbox.io/s/slider-comparison-forked-jziv6?file=/src/App.js we will notice that adding components per slot has an 20% perf degradation 😢

Production versions:

To be honest I don't know at this point whether it's better to add this backward compatibility or not.

Are these any real use-cases for the the support of the classes or is it just for easing up the upgrade? Are we ok with having 20% perf degradation only for easing the migration path? Is there some alternative that we cannot see, that would help us to do both of these without paying the perf cost?

@ianschmitz @eps1lon @oliviertassinari and others :) are there any thoughts on this?

MathiasKandelborg commented 3 years ago

As long as we can define and use themes & styles with TypeScript, I wouldn't mind spending time migrating compared to losing 20% perf

ConAntonakos commented 3 years ago

I'm generally curious, and forgive me if I don't understand the underlying design, but classes was an API for JSS? If the API design is switching from JSS to styled components, does it make sense to keep supporting classes?

ianschmitz commented 3 years ago

Are these any real use-cases for the the support of the classes or is it just for easing up the upgrade? Are we ok with having 20% perf degradation only for easing the migration path? Is there some alternative that we cannot see, that would help us to do both of these without paying the perf cost?

Apologies if i've missed anything with the proposed API, but IMO the use case I most often see within our org is the abstraction of the underlying styling system used by MUI. Yes i guess classes is sort of an "API for JSS" as @ConAntonakos mentioned, but to the consumer it didn't matter. As a consumer I could use the CSS tech of preference (are there any limitations today with classes?). We have a number of products using a variety of:

depending on the needs and preferences of those teams.

Exporting the class names helps if you're using some flavor of CSS-in-JS, but what are the thoughts for those that aren't?

RE. 20% perf, i agree that is likely not an acceptable trade-off. At the end of the day you guys should do what's best for the majority of the community. Just offering my thoughts 😄

mschipperheyn commented 3 years ago

One wish I never got was that material-ui would be react-native compatible. Many projects are cross platform these days and having a unified styling solution offers a lot of advantages for cross platform components. We ended up rolling our own using material-ui on the web side and react-native-paper on the native side and standardizing on the material-ui API. I'd be interested to understand if this new styling solution will make the use of (some/all) Material UI components on react-native a possibility? Any window references in components would obviously be a blocker, but the styling itself supports native as well doesn't it?

oliviertassinari commented 3 years ago

@mschipperheyn react-native is a no-goal so far. It's +5% of usage potential (1 month of growth) and +50% more effort (a guess). The opportunity cost is really high with no obvious direction on how to monetize it (outside Ionic's model). Also, mind that flutter seems to have captured a large chunk of the audience that react-native targets.

zzzev commented 3 years ago

Now that v5 is in alpha release, is there a resolution to this issue? In particular, is the styling solution still based on JSS? We've seen substantial performance issues related to JSS, so have been eagerly anticipating a new solution.

MathiasKandelborg commented 3 years ago

@zzzev This isn't an issue per se. It's an RFC (Request for Comments) thread.

I'm inquisitive about which substantial performance issues you're talking about and how going from JSS would improve that. Because the way I see it is that if you have performance problems, it's probably not the styles but how they're implemented, i.e., the same result could be achieved by writing the styles in another way, using less processing power.

At the very least do I fail to relate to how going from JSS to Emotion (which is shown in this thread to most likely have some performance degradation) would improve anything.

zzzev commented 3 years ago

My understanding is that emotion will cause a slight hit to static styles performance, and much better dynamic styles perf -- but maybe that's not quite right? There are a lot of different numbers in this thread and it's hard to reconcile into one picture of performance (and obviously it depends a lot on an individual's situation).

When you say we should write the styles in a different way, do you mean avoiding dynamic styles? Or are there other best practices we should be considering?

mbrookes commented 3 years ago

@zzzev Correct on the first part, not quite on the second (unless you count going from unsupported to supported as an infinite performance gain 🙂).

Emotion enables support for dynamic styles, at the cost of moderately slower performance for static.

zzzev commented 3 years ago

I'm confused; aren't there dynamic styles in the current/v4 makeStyles? e.g. this pattern

kelly-tock commented 3 years ago

I'm confused; aren't there dynamic styles in the current/v4 makeStyles? e.g. this pattern

There are but there's a big known perf issue. My team stays far away ATM

soroushm commented 3 years ago

I just hate component-styled jss is good but there are a few problems in debugging, performance and still some bug still not resolved such as nested like&:after

it's my packages build-up for react-native and react-native-web https://www.npmjs.com/package/@material-native-ui/theme-provider

I`d like something like this (it's build-up on top of RN and RNW)

mwskwong commented 3 years ago

So is there a conclusion on the recommended style solution to use with Material UI v5? It seems that there is an intention of moving away from JSS which @material-ui/styles is currently built upon. Or @material-ui/styles will be refactored to build upon other styling solutions like styled-components instead?

mnajdova commented 3 years ago

So is there a conclusion on the recommended style solution to use with Material UI v5? It seems that there is an intention of moving away from JSS which @material-ui/styles is currently built upon. Or @material-ui/styles will be refactored to build upon other styling solutions like styled-components instead?

@matthewkwong2 we wouldn’t adopt the @material-ui/styles package to the new styled engine, it will keep supporting jss. In v5 it will be isolated to the rest of the codebase and we plan to remove it in v6, meant to help transition to the new styling engine.

For v5, we will have new beat practices regarding the styling solution, like the sx prop and the styled utility, we still work on the documentation around this.

Also the theme overrides and variants are still going to be supported in v5.

mwskwong commented 3 years ago

For v5, we will have new beat practices regarding the styling solution, like the sx prop and the styled utility, we still work on the documentation around this. Also the theme overrides and variants are still going to be supported in v5.

So if I understand correctly, for the styling of individual components, it is encouraged to use sx or styled instead of makeStyles.

But what about the theme (i.e. createMuiTheme)? I believe that part is also built upon JSS right? What would be the recommended way to create a theme (i.e. with the main purpose of defining global styles) now?

mnajdova commented 3 years ago

We still keep the same structure for creating the theme, we just expect that the values for the components overrides & variants follow the syntax for emotion/styled-components. There are no changes in how the theme is created, the API is exactly the same, the same theme context is reused for the new styled engine too. Does this makes sense @matthewkwong2 ?

dantman commented 3 years ago

@mschipperheyn react-native is a no-goal so far. It's +5% of usage potential (1 month of growth) and +50% more effort (a guess). The opportunity cost is really high. Also, mind that flutter seems to have captured a large chunk of the audience that react-native targets.

I don't want to take us on too big of a tangent, but I would like to push back on some of these rationales.

mbrookes commented 3 years ago

the switch to emotion has likely eliminated 2/3 of the difficulty in making MUI work in RN

We look forward to seeing your POC 😄

dantman commented 3 years ago

We look forward to seeing your POC 😄

I'd love to play around with it if I have a chance. But people generally don't bother making POCs for things that maintainers show a disinterest for. No point building something when the current atmosphere is that it'll probably just end up abandoned. Hence why I want to push away from dismissing the value of RN or the value of RN as a possibility for the future.

CrocoDillon commented 3 years ago

Two questions:

  1. Can you use the new styling solution without HOCs? I for one love the React hook API that MUI currently has... not sure if the styled syntax is also a HOC under the hood but that would beat efforts (pretty much everywhere in React libraries) in moving away from HOCs.
  2. Will the classes prop most components have still be supported (it would give full flexibility in what styling solution users can go with)?
smmoosavi commented 3 years ago

the fast-refresh is enabled by default in create-react-app v4. should we add fast-refresh support as a new requirement?