Closed oliviertassinari closed 2 years ago
yordis
commented
4 years ago @oliviertassinari what is the exactly the tasks in hand?
Here is what I understand,
material-ui/stylesstyled-componentsIs this correct?
oliviertassinari
commented
4 years ago @yordis I think that the timing is going to be key in this transition. I would imagine the following steps:
ConAntonakos
commented
4 years ago @oliviertassinari I'm curious, and I apologize if this was repeated: why not keep @material-ui/styles as a wrapper or an abstraction for styled-components?
oliviertassinari
commented
4 years ago @ConAntonakos it's an option. It could help if we need to extend/normalize some of the features of styled-components.
yordis
commented
4 years ago @oliviertassinari do we have a list of what's left?
oliviertassinari
commented
4 years ago @yordis We haven't done many efforts in this direction yet. However, there is a probability that it will require breaking changes, v5 is planned for around Q4 2020. I think that we should explore a partial deployment strategy for developers that want to leverage it sooner. Now, this effort has to be balanced with the other priorities, like the support of new advanced components (free and paid) or the release of an unstyled version of the library to increase our audience reach (with different themes, e.g. iOS style). The good news is that we will likely grow the team in the coming months, enabling us to move faster.
I imagine you are already using styled-components on top of Material-UI today as many other developers do (and not @material-ui/styles). What are the top pain points you hope to address with this migration?
From a product perspective, our incentive is about: smaller bundle size, better performance, steaming support, fewer bugs, CSS template strings support, larger community, enables to use the system props in the core components, allow true dynamic themes and props support, better docs.
yordis
commented
4 years ago However, there is a high probability that it will require breaking changes,
Yeah, I tried to change some examples, but they require some integration with the theme provider, so we may need to inject material/style theme provider and styled-component theme provider I am assuming.
v5 is planned for around Q4 2020.
That is far enough, would it be better to pin different issues for visibility on what is a priority at the moment?
I imagine you are already using styled-components on top of Material-UI today as many other developers do (and not @material-ui/styles).
Actually, I am quite happy with @material-ui/styles and I am not using styled-components when I use Material UI (I would remove withStyles since I don't want to rely on programmer discipline 😉 , but I understand legacy software may no have hooks still )🤷♂️
What are the top pain points you hope to address with this migration?
I am trying to help with the migration, personally, no pain points. Unless you take into consideration that as an ecosystem, I have to maintain two ways to develop something, but meh, personally, it is okay for me.
Maybe styled-components fixes some interoperability with NextJS and Gatsby since the last time I tried was hard to make Mui work with those tools because of the SSR and styles (I am not sure if still painful) and most libraries using styled-components work out of the box.
Let me know how I could help, like I said, I was using the pinned issues to find the prioritization of the Org
oliviertassinari
commented
4 years ago That is far enough, would it be better to pin different issues for visibility on what is a priority at the moment?
It depends on the time horizon you are interested in. You can follow the issue with the label important, the roadmap page on the documentation and the monthly blog product updates.
I don't fully understand this point. Why not using styled-components when using Material-UI (today)? We had 1/3 of our users going down this path when we did our last survey, 6 months ago.
yordis
commented
4 years ago I don't fully understand this point. Why not using styled-components when using Material-UI (today)?
@material-ui/styles works like a champ so far, no reason to migrate everything 😄 so I am one of those out of 3 that don't use it together today.
Thank you for the info about prioritization.
oliviertassinari
commented
4 years ago @yordis btw, my answer was made under the assumption you were following up on the main styled-components issue. I haven't realized we were on the documentation one.
yordis
commented
4 years ago @oliviertassinari do you have any suggestions about the issue with theme context?
I tried to use props.theme inside an styled-components but didn't work, I am not sure if you have a suggestion for it, but I think we will require to add styled-components theme provider as well.
HiranmayaGundu
commented
4 years ago Hey @oliviertassinari! Are you looking for PR's that convert the existing customization demos to use styled-components as part of this issue?
nikandlv
commented
4 years ago I dont think styled-components is a good styling solution. current solution looks much much more readable, appealing, accessible and cleaner than styled. i dont see any good reason to migrate.
Jack-Works
commented
4 years ago I dont think
styled-componentsis a good styling solution. current solution looks much much more readable, appealing, accessible and cleaner than styled. i dont see any good reason to migrate.
And get almost fully typed. Don't see any reason to migrate +1
TheHolyWaffle
commented
4 years ago The link you've posted, that should show growing interest to styled-components, recently started showing a downward trend:
I feel that narrowing down a styling solution to a single library is going to give us the exact same problem as we have today.
What about integration with zero-runtime libraries such as linaria? This was suggested as being looked at in May 2019 Update.
eps1lon
commented
4 years ago So far it only recovered to pre-v5-hype. Let's see how the updated data point for January till now looks.
oliviertassinari
commented
4 years ago @TheHolyWaffle Don't trust rank2traffic.com too much, data hasn't been very reliable nor up-to-date, its main advantage was the overall trend over 10 years (before it was made paid). For a shorter time window, so 6 months, prefer https://www.similarweb.com/ as more reliable. Also take into account that once people know the API, they come back much frequently to the documentation.
ldiego08
commented
3 years ago I dont think
styled-componentsis a good styling solution. current solution looks much much more readable, appealing, accessible and cleaner than styled. i dont see any good reason to migrate.And get almost fully typed. Don't see any reason to migrate +1
+1 styles is the main reason we’re migrating to Material UI and moving away from styled components. It’s too much CSS and refactoring it has proven to be a huge burden for us.
maciej-gurban
commented
3 years ago Hi! First of all, thank you for providing a great component library, best one I've used so far. Our teams have written hundreds of thousands of lines of code using the components and methodology you created and once developers learn the basics of using classes, how to write the styles etc., it's a breeze to work with even on a massive enterprise scale type of codebase.
I'm not sure if that's the most common use of your library, but I suppose you are aware that many teams build their component libraries on top of Material UI, and so any components and decision you make also trickle down to those libraries. On our end we've been very happy with both performance and APIs so far.
I've been following recent development in the library and to be honest, I'm having trouble understanding some of the decisions and worried how that will affect our teams, and what's overall the benefit of this move would be, as opposed to for example forking MUI. Others have voiced their concern about move to styled components so I'll focus on the other one for me - the Box component.
I understand the utility of a Box component - to make it possible to use theme variables etc. in prop values, however the API and the consequences of using this component disqualify it from something I could recommend to our teams.
First, it has a cryptic API for no reason I can discern (unless saving a few bytes is that reason): Instead of writing <Box margin={3} />, it would be <Box m={3} />.
Abbreviations like that seem needless, make things potentially ambigious, and introdue a new learning curve to developers, a mapping of abbreviations to actual properties they now need to memorize: "Is applying color done using c or color?", "Is background a b, or bg, or background, or was b a border?" "Is background-size abbreviated as bs?"
Second, components abstract most commonly recurring UI patterns, and create APIs that provide means of customizing those patterns to the extent that the design system permits. This manifests in creating props like gutterBottom on Typography, or dense on ListItem - as opposed to accepting just about any padding or margin. I feel like introducing Box to large extent undermines this effort and introduces a tool that will make a mess out of any attempt to follow a design system unless we define some very nitpicky ways that Box component can be used for and spend effort in code reviews to make sure the all the values in used Box props aren't beyond the accepted list. And at that point, the way to "automate" this would be to create a component that restricts the options, and we're backt to square one. To give an example, why would using Box mb={2} to achieve margin under Typography be okay, but Box mb={6} not? This concern doesn't exist when we can rely on props to limit the options.
Third concern, or rather a question I have. Since the Box component is potentially a quick way to build certain functionality, it would be also used by libraries built on top of MUI. How would one override the styles of Box components used within another component? As an example. If we built ListItem using Box under the hood, would we need to introduce BoxProps={{ padding: 2 }}, or accept also dense prop based on which we write logic to apply a padding prop to a particular Box, or still something else?
oliviertassinari
commented
3 years ago I'm not sure if that's the most common use of your library, but I suppose you are aware that many teams build their component libraries on top of Material UI, and so any components and decision you make also trickle down to those libraries. On our end we've been very happy with both performance and APIs so far.
@maciej-gurban This use case is an important one for us. Our incentives are aligned to significantly improve it. This is one of our objectives with v5.
For instance, we are investigating the feasibility to provide a design tool that could be put in the hands of designers (paid) and would output ready to use customized Material-UI components (MIT), corresponding documentation, Sketch & Figma kit. Basically, all we have been going it for Material Design but scaling it 🚀. The end goal here would be to free the time of a couple of front-end developers in each company. Why have front-end developers build a custom design system when it can be done by your own designers + Material-UI at a fraction of the cost? + reduce risk and have your front-end developers spend time where they make the most differences: working on the product.
I'm having trouble understanding some of the decisions and worried how that will affect our teams
If you could list them, it would be awesome.
Others have voiced their concern about move to styled components
What's your concern with such a shift? Our objective on the styling side has a couple of layer:
classes API (first seen in jQuery-UI), provide default hardcoded values for each of the classes (global class names). The objective behind this shift answer to a couple of incentives. First, it's to dismiss a fear our users have, same to CRA eject mode, you won't use it but it feels safe to be present. Second, it's required to be able to build the paid design tool. Third, it's required for the next layerI'll focus on the other one for me - the Box component.
Yeah, I can hear you! I have been working with @gregberge in the past. At some point, we have considered hiding all the className props on @doctolib's design system. The interesting thought is that you can gain features (properties) in exchange of more constraints.
I wouldn't worry too much about this one. This can be resolved with a global option to limit the access to the "system"'s features or an eslint rule.
Jack-Works
commented
3 years ago The abbreviation on the Box component is confusing. Code is being read more than being written, so it's important to keep clear what the code is meaning. Last day I found "Box py={2}" in our codebase and I'm totally confused. After a search I figured out that means "paddingY". Those abbreviation should not be encouraged especially non-css properties (I can guess pt means padding-top but not for py)
maciej-gurban
commented
3 years ago @oliviertassinari Thanks for your patience
I'm having trouble understanding some of the decisions and worried how that will affect our teams
If you could list them, it would be awesome.
I wouldn't want this to turn into a litany of things I disagree with, because ultimately you're the guys who maintain this library and our view of what makes sense will be shaped by our own needs which might and likely don't always align, and that's fine. I'm not against introducing the means of using other styling solutions - in fact I think it's great as it will bring more users who were holding off because of their dislike for JSS, but there's also us - the already existing users of your library who are sold on your solution, and if possible, would like to avoid massive refactor.
Even if you decide that "we still provide support for JSS", refactoring all demos and your components to styled components will force the teams using JSS to migrate to styled components. "Why are we still using X, when MUI team moved to Y?" - will be one of the many questions in light of which it would be really hard not to agree with needing to make that migration sooner or later.
I can definitely empathize with wanting to reach a wider audience by using a styling solution that's more popular. However, I think it's worth considering that "popular" is not always "best" and that a move to a different tech needs onsideration not only of advantages and disadvantages of both solution, but the context in which we're making the decision.
Starting fresh, looking merely at advantages of X over Y makes sense, but working on an already existing system we also need to consider the cost of switching over and domino effects this bring on downstream teams. For this switch over to make sense the advantanges of the other solution need to outweight the cost of migrating over. It seems that for your team, that cost benefit analysis rules in favor of styled components, but from what I can tell looking at reactions at posts talking about styled components in your repo, your user base is far from the same conclusion.
Like you said, your aim is to open up MUI to more styling solutions. To provide good support for those solutions, there would need to be good documentation, examples and smooth integration layer - otherwise developers would find it hard to translate what they see in demos into what their styling solution of choice needs. I'm just not sure if you really need to migrate to styled components to achieve the goals. Seems like your solution is also perfectly capable, if not more so than others, to build the design tool you're after since you already use actual JS object for all the styles.
kelly-tock
commented
3 years ago One question I have, does this mean that the core of Mui would still use jss, and this allows for a better way to use styled components on top of that? Or would there be a way to say the core is also styled? I just think it would add a lot of bloat if you have two css in js solutions.
ldiego08
commented
3 years ago How would theming work if using styled-components? I used to use helpers in the CSS portion, and it was really messy. For me, the approach of applying theme props in a JS object is a lot cleaner than in a templated string.
thclark
commented
3 years ago For me (scientific backend programmer by origin), MUI styles bring beautiful, calming, predictable, parameterisable logic to the mental, crazy, bonkers-rules why-the-hell tearing-hair-out world of CSS.
The styles library (and its tight integration with the theme) is the main reason I mandate use of MUI over any other components library in the two organisations I oversee tech for - it takes all the css agony away! At first, all the developers I work with are like "what the hell's going on? Where's the css? Just give me css!!" and then they're like "Ohhhh. riiight. Got it. Magic."
In the longer term I think the current approach is going to become ever more powerful as the world moves to low/no code solutions (eg like sketch or figma, but outputting an actual routed app and set of components rather than a set of wireframes) - having styles expressed as an object unlocks the ability to introspect that - and create more new features in such an environment (like provision of a schema enabling direct use of MUI components within a CMS, or generation of 'theme from this' and hundreds I haven't thought of yet).
Moving back to the more raw-css kind of approach of styled-components doesn't preclude that - but it does make a lot of things (particularly parameterisation on the theme) a lot jankier and frustrating to achieve, and still requires much more in-depth knowledge of CSS's technicalities making it less accessible to new programmers and creative types alike.
On the evolution of the state-of-the-art, I think styled-components has become really popular because it's a great step in the right direction from where the world was (which is why it became popular). But the existing MUI styles system is the next step on from that; not an incorrect side-step. Once everyone's migrated to styled-components then the question will be "wait: why on earth are we doing this with these weird raw strings in our components...?"
Maybe I'm totally wrong, but for my $0.02, I'm begging you to stay the course for at least another major version :)
oliviertassinari
commented
3 years ago @thclark your main concern seems to be with the CSS template string syntax vs the JavaScript style object API. Is this accurate? It also seems to be the concern with most of the other comments of the thread.
Styled-components and emotions support both APIs. This wasn't the main purpose of the issue. The objective of this issue was to anticipate the migration to a different styling solution. However, we never move forward with "use styled-components in all the demos even if we didn't migrate the core engine". We have opted for keeping both synchronized. At this point, keeping the issue open doesn't add much value outside triggering discussions like this one :). We have to migrate the demos anyway for #22342.
I personally prefer the JavaScript API over the CSS string one because:
However, it's unclear what the community, at large, will enjoy using most. CSS template has its advantages too. It's easier to copy & paste code between the browser and the code. A lot more people (overall: designers, developers, etc.) are familiar with the approach.
To be noted that I think that we should use the style utilities as much as possible in the demos with v5.
@mnajdova any thoughts on the matter? Maybe it's worth asking the community on a poll.
thclark
commented
3 years ago @oliviertassinari succinctly put, as usual. Yes, my main concern is retaining the Javascript API. Honestly, I wasn't aware that styled-components retained that, as all of the examples I came across seem to be css templated.
That perhaps moots most of my points above. Nevertheless, glad you didn't move across - and thanks for your and the team's continued efforts here. I've built things I never could have without MUI existing.
oliviertassinari
commented
3 years ago This issue is being resolved in v5. We have migrated the documentation of the slider to the new approach: https://next.material-ui.com/components/slider/. We use the sx prop anytime simple CSS are necessary then use the styled API for more heavy customizations. I believe the previous concern raised have been handled, if not, please comment :).
vicasas
commented
3 years ago i will start taking this
vicasas
commented
3 years ago @oliviertassinari I am taking the documentation of the Button component but there is something I do not understand, when using the prop sx using margin: 1 or m: 1 this does not work, but if I change the 1 for another value it works. Any idea what is going on?
oliviertassinari
commented
3 years ago @vicasas Note that we aim to use the Stack component for adding spacing between items: #18158. However, here, we can't since it wrap.
I can't reproduce your issue. This works:
diff --git a/docs/src/pages/components/buttons/ContainedButtons.tsx b/docs/src/pages/components/buttons/ContainedButtons.tsx
index e8ba2d4372..c8e73b57f6 100644
--- a/docs/src/pages/components/buttons/ContainedButtons.tsx
+++ b/docs/src/pages/components/buttons/ContainedButtons.tsx
@@ -1,22 +1,16 @@
import * as React from 'react';
-import { makeStyles, createStyles, Theme } from '@material-ui/core/styles';
import Button from '@material-ui/core/Button';
-
-const useStyles = makeStyles((theme: Theme) =>
- createStyles({
- root: {
- '& > *': {
- margin: theme.spacing(1),
- },
- },
- }),
-);
+import Box from '@material-ui/core/Box';
export default function ContainedButtons() {
- const classes = useStyles();
-
return (
- <div className={classes.root}>
+ <Box
+ sx={{
+ '& > *': {
+ m: 1,
+ },
+ }}
+ >
<Button variant="contained">Primary</Button>
<Button variant="contained" color="secondary">
Secondary
@@ -27,6 +21,6 @@ export default function ContainedButtons() {
<Button variant="contained" href="#contained-buttons">
Link
</Button>
- </div>
+ </Box>
);
}
stunaz
commented
3 years ago @oliviertassinari where is the Stack Component?
oliviertassinari
commented
3 years ago @stunaz It's not implemented yet. @souporserious has started looking into it.
oliviertassinari
commented
3 years ago @vicasas I have figured it out. The issue is that this selector .foo > * + * or .foo > * have a specificity of 1. The same specificity as the margin: 0 sets by the Button. We have no predictable CSS injection order with emotion like we can have with JSS or styled-componnets. One workaround is to do: .foo > :not(style) to add a bit more specificity https://specificity.keegan.st/.
vicasas
commented
3 years ago @oliviertassinari I have been dealing with this problem for a long time, I thought I was doing something wrong. But using another way to enter the child html element using & > button works perfect. Thanks.
vicasas
commented
3 years ago There is some internal difference when writing the emotion styles as follows
1.
const Button = styled(MuiButton)(
({ theme }) => `
margin: ${theme.spacing(1)}
`,
);instead of doing this
2.
const Button = styled(MuiButton)(({ theme }) => ({
margin: theme.spacing(1)
}));?
oliviertassinari
commented
3 years ago @vicasas We only use the CSS template string syntax (1.) for the customization of the unstyled components. Anything else is using the JavaScript object syntax (2.). Now, in your example, none should be necessary. I would recommend moving slowly in the beginning, migrating a few demos at the time.
vicasas
commented
3 years ago Attached is a list of component demos that should be migrated to the new style api.
Layout
Inputs
Navigation
Surfaces
Feedback
Data Display
Utils
Lab
Should we add the demos in the menu system here?
cc: @oliviertassinari
oliviertassinari
commented
3 years ago @vicasas Thanks for putting this list together.
Should we add the demos in the menu system here?
All the system demos should have already been migrated. So I would hope it's not needed.
vicasas
commented
3 years ago All the system demos should have already been migrated. So I would hope it's not needed.
@oliviertassinari Yes that's right, I couldn't notice any that continue to use makeStyles or whitStyles. Except there are demos that use the style attribute instead of the sx prop. Is that expected?
On the other hand, in the customizations section there are demos that still use the old styles API. Do we add them?
And one last question, when using the styled api the rules names are no longer used? These would be obsolete of use?
oliviertassinari
commented
3 years ago Except there are demos that use the style attribute instead of the sx prop. Is that expected?
Not sure, it does sounds expected. I could confirm with a link.
in the customizations section there are demos that still use the old styles API. Do we add them?
Do you have a link as example?
And one last question, when using the styled api the rules names are no longer used? These would be obsolete of use?
Do you mean the $ruleName synthax? It's no longer available.
vicasas
commented
3 years ago Not sure, it does sounds expected. I could confirm with a link.
Some of them are:
Do you have a link as example?
Some of them are:
Do you mean the
$ruleNamesynthax? It's no longer available.
If that's exactly what I meant. Should they be removed from the component apis documentation?
oliviertassinari
commented
3 years ago For the inline style, it's seems OK. It draws a contrast between what we want to show and the set up for the demo to work correctly.
For the customization demo, it does look like something we need to update.
For the rule name, they are now legacy and specific to @material-ui/styles. The less we can depend on them, the better.
ocodista
commented
2 years ago Are you guys planning to make makeStyles obsolete?
oliviertassinari
commented
2 years ago @CAIOHOBORGHI Hopefully, this is the long-term outcome, yes.
thclark
commented
2 years ago I must admit, I utterly love makeStyles, find it far more intuitive than styled-components or anything else (it's actually the reason why I chose MUI in the first place, and adopted MUI throughout the both companies for which I run development).
Fudging strings together in styled components for css annotations looks ugly as all hell to me; so I will be extremely sad to see makeStyles go, and sadder still to refactor 5+ large webapps for the future version of MUI not supporting makeStyles. This is going to cost us thousands, difficult for small companies and non-profits to stomach.
However, @CAIOHOBORGHI it's worth noting that @oliviertassinari architected the whole lot in the first place - I've learned a huge amount from that and have come to trust his vision for what needs to happen in MUI for the benefit both of the wider react community and of mui users. So on balance I'm inclined to stomach this.
thclark
commented
2 years ago @CAIOHOBORGHI understood - probably more useful to weigh in on the RFC rather than here, since this issue is just about the docs.
cjnoname
commented
2 years ago Not a fan of styled component, makestyle is very handy to be honest
oliviertassinari
commented
2 years ago @thclark, @cjnoname Is the dislike about the styled() API the CSS template string syntax (JavaScript object works too)? Or the love about makeStyles in its capability to have multiple class names (classes object)?
We have talked about bringing makeStyles back powered by emotion, to further ease the migration (we already keep @material-ui/styles around).
olros
commented
2 years ago Is the dislike about the styled() API the CSS template string syntax (JavaScript object works too)? Or the love about makeStyles in its capability to have multiple class names (classes object)?
To me, the best thing about makeStyles is the capability to have muliple class names declared side by side. That allows us to have all the styling used in a component/file declared in a single place and resuse a lot of the styling. In my opinion it does also make much more sense to apply separate styling from the jsx. I think the new demos with the new styled-syntax looks really chaotic and it's difficult to find the code you're looking for when there's styling everywhere.
As a reference, this is an example of makeStyles-usage in a lot my projects:
import classnames from 'classnames';
import { makeStyles } from '@material-ui/styles';
const useStyles = makeStyles((theme) => ({
grid: {
display: 'grid',
gridGap: theme.spacing(2),
gridTemplateColumns: '1fr 1fr',
[theme.breakpoints.down('md')]: {
gridTemplateColumns: '1fr',
gridGap: theme.spacing(1),
},
},
column: {
gridTemplateColumns: '1fr',
},
margin: {
margin: theme.spacing(1),
},
}));
const Example = () => {
const classes = useStyles();
return (
<div className={classnames(classes.grid, classes.margin)}>
<div className={classnames(classes.grid, classes.column)}>{/* ... */}</div>
<div className={classnames(classes.grid, classes.column)}>{/* ... */}</div>
</div>
);
};
Following #6115, I think that we should migrate all our demos to use the Box component or styled-component. The goal is to hide
@material-ui/stylesas much as possible. styled-components keeps growing, a consolidation of this styling solution will be better, overall, for the React community.Regarding the timing, I think that we can start to use the Box component now. For the demos that we can't migrate, we probably want to wait for the first proof of concept with #6115.
Attached is a list of component demos that should be migrated to the new style API.
Docs infra
Misc
/docs/src/pages/getting-started/templates/#26604/test/regressions/, e.g. TestViewer.js #27010/docs/src/pages/premium-themes/#26658 #26671 #26707Layout
Inputs
Navigation
Surfaces
Feedback
Data Display
Utils
Lab