Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using inline styles this way reduces drastically the UI/UX performance!! #1132

Closed
danielmeneses opened this issue Jul 12, 2015 · 32 comments
Closed
Labels

Comments

@danielmeneses
Copy link

Hi,

start off saying that i really respect the work done here. great job!
Now i've to say :) that the inline style approach used here might be something you should re-think or at least add a little tweak. Well, i say this because It slows down all the UI loading when you start to place elements all over the place that have be render a lot of times. And in my case, the amount of html to send via server to the browser (isomorphic approach) it's an overload of data not really necessary.

So my suggestion would be, adding a config param for whether the developer wants or not inline styles. This way, if i choose not to use i can simply add the materials assets files on the page.
I know that probably the assets files could be even bigger than the generated html, but we all know too that those are cached and we can live with that.
Well, hope you'll not respond me that their is a way to achieve this and i was to lazy too get that info ;).

Does this make sense to you?

thanks

@djmc
Copy link

djmc commented Jul 15, 2015

I must agree, I'm surprised that Material UI didn't use something like React Style that would allow developers who need it the ability to extract the styles into a CSS file. There are already a number of inline-style React tools, yet this library went its own way.

I have been developing a website for a while with Material UI using Isomorphic JS and when it switched to inline CSS, it really made me reconsider this option and start again with another framework. However, Material UI is the most complete/as-close-to-native React-based Material Design library, that anything else seemed like a downgrade.

@imouto1994
Copy link

I also agree that we should allow user to choose to use inline styles or extract all into an asset file

@danielmeneses
Copy link
Author

Yeah djmc, React Style approach seems a very cool one for Material UI. I agree this is the best react-based material design that "lives" right now, but got to say that i'll not use it in production envs while there isn't a solution for this. It just doesn't make any sense to me having the same styles repeat over and over and of course all the consequences that come with this approach.

But i can wait ;), not a big deal!

@conundrumer
Copy link

+1 React Style approach

because reasons

@rozzzly
Copy link

rozzzly commented Oct 21, 2015

I've actually started writing my own UI library because of this. I tried Semantic, but it isn't react-y enough, SSR doesn't really work as well either. So I've been using a lot of the theming from semantic (converted from LESS to SCSS) and mixing that with some of the logic I see in material-ui. It's slow going, but I like the result quite a lot! I will post the results once I have a little something to show.

@rickihastings
Copy link

Is this actually being taken into consideration? UI heavy apps are really sluggish because of inline styles, it should be at least possible to extract them out if the dev wishes. It also blocks us when using media queries and psuedo selectors. I may have to consider using react-toolbox.

@oliviertassinari
Copy link
Member

@rickihastings I would refer to this issue #1951.

it should be at least possible to extract them out if the dev wishes

I agree, this could reduce page size. Do you see any other advantage?

@rickihastings
Copy link

Other than the points already mentioned in #1951 and UI heavy apps being sluggish, not really.

A major issue of mine is I can't even load tables on iPad mini devices, I'm assuming it's because of the sheer amount of inline styles being processed. I'd rather not swap out material-ui for another library because I think it's the most complete, but the inline styles are giving me a huge headache.

@beata
Copy link

beata commented Nov 17, 2015

I really like the effect of material-ui, but I can't implement it on my work because of inline styles.
It slow downs page rendering, and is hard to customize in css files.

@Kagami
Copy link

Kagami commented Nov 17, 2015

reduces drastically the UI/UX performance
UI heavy apps are really sluggish because of inline styles
It slow downs page rendering

Shouldn't you provide benchmarks in order to prove your words? Benchmarks here claim the opposite. Just saying "it's slow" as well as "+1" is useless.

@oliviertassinari
Copy link
Member

@Kagami I couldn't agree more.
Adding just +1 is adding noise to the conversation, use the subscribe button if you want to be kept up to date.

@rickihastings
Copy link

@Kagami
http://jsperf.com/class-vs-inline-styles/2 according to this test, inline styles are 86% slower.
http://jsperf.com/classes-vs-inline-styles 22% slower here.
http://jsperf.com/inline-style-vs-css-class/9 Inline style color is 38% slower, although inline style object is roughly the same as CSS class.

The main point I think people are making is it forces people to develop and write CSS in a way that hasn't been done since, well.. ever. We have great features such as media queries, psuedo selectors, inheritance that we can't really use with inline styles. Not to mention as you're tinkering with components in dev tools, you can't update styles for all components, only one.

@oliviertassinari
Copy link
Member

@rickihastings Thanks for bringing up those tests. That's interesting.
http://jsperf.com/inline-style-vs-css-class/9 seems to make inline style faster.
Let's continue the conversation on this thread #1951.
We are investingating another way to style components.

@ethan-deng
Copy link

As an open source project, I have to say inline style is bad. First, it is hard to customize the style. There are always situations that something is deep inside and hard to customize. Second, it hurts performance when I use a table with a few hundred rows. Each row has tons of inline styles. For this reason, I have to go with React-bootstrap table instead.

For my personal project, I do find inline style is convenient and it reduces the effort to maintain CSS files.

@inspiraller
Copy link

It's about time some sense to this madness of inline styling is being addressed. We don't need tests to prove inline styling is slower, because any kind of duplication is going to be slower. The key point is how much slower and is it worth it? It's good that someone has proven with tests how much slower it is above. I didn't need those tests to know it would be slower.

I am annoyed that Material UI is considered the most complete material type react component library and that its so popular. What it proves is that there are a lot of people out there that are inexperienced that they have picked it off the shelf like candy and used it. Now they have got a monolith of ugly, slow running code, all because of a bigger problem in the web industry - Experienced developers aren't securing important positions in top companies to encourage the right decisions.

Facebook direction to encourage inline styling was misplaced in their developers lack of experience. I can't even believe it ever took such popularity. Who have they been hiring? and what other bad decisions in programming has arrived as a result of it? This is a fundamental mistake on reacts part and has put a lot of developers I know off because of this faux pas, giving more power to Angular which itself has it's own demons in heavily opinionated vocabulary.

Frameworks are like religions. They encourage laziness and lack of growth of understanding the end points by providing an 'easier to use' interface/language, and can cap peoples ability to be as efficient as possible in understanding and achieving their primary goals. As a result of trying to achieve a panacea for a general set of tasks they will always multiply language to achieve single unit tasks, and thus always be slower than the alternative of a native language. As they grow in popularity, giving power to people who don't understand how the world/web works, that can lead to further faux pas's that wouldn't have been allowed to happen. Ultimately, the framework empowers the inexperienced person, and disempowers the experienced person, leading to trouble - like inline styling.

@Kagami
Copy link

Kagami commented Aug 5, 2016

Read this instead of spreading ad-hominem highly opinionated reasonings. CSS has a lot of issues and inline styles is one of the ways to solve them. Maybe it's not perfect but it might be good enough compromise for some tasks. There are tons of discussions regarding React+CSS on the web and there are some better options, see e.g. here.

I didn't need those tests to know it would be slower

Huh? That's not how technology works. That is religion.

@inspiraller
Copy link

inspiraller commented Aug 5, 2016

ad-hominem? I have read that, thanks for the link. I can go through it bit by bit if you want and prove why all the reasoning used to prove inline styling is better is actually flawed?
First point being - class="button" class="buttondepressed" is already flawed. I wouldn't use that approach. I would use this approach, which is clearer about all the possible states it can have, and it distinguishes the difference between the classname and the datastates it can have:

button data-state="on" data-states="on,default" class="button"

You are right, Religion is believing something without testing it first. The point I mean to clarify, is that I didn't need someone else's tests to prove what I have already tested over years of development.

@danielmeneses
Copy link
Author

danielmeneses commented Aug 5, 2016

@Kagami, sorry i just saw your response today :). You said about one of comments:

"reduces drastically the UI/UX performance
UI heavy apps are really sluggish because of inline styles
It slow downs page rendering"
Shouldn't you provide benchmarks in order to prove your words? Benchmarks here claim the opposite. Just saying "it's slow" as well as "+1" is useless.

Really man, do you believe that? lol.
As @inspiraller said, i've made my tests throughout the years of development, i'll not waist my time doing something that someone did before me and that and we, all experienced developers, know. But i let you with a "React like inline-styles vs css" benchmark since you like them :) https://ctheu.com/2015/08/17/react-inline-styles-vs-css-stupid-benchmark/

@Kagami
Copy link

Kagami commented Aug 6, 2016

@danielmeneses thanks for the link. According to that test, there is small overhead on rendering time/time to mount in Chrome but it has no more impact as soon as it’s rendered. It doesn't sound like "UI heavy apps are really sluggish because of inline styles", does it?
Btw, react-jss approach is now considered for the styling solution.

@apendua
Copy link

apendua commented Jan 19, 2017

Have you guys considered using styled-components approach? It looks like that's slowly becoming the new standard so it would be great to see it in Material UI as well.

@oliviertassinari
Copy link
Member

@apendua We have, that's not performant and flexible enough for our needs https://github.com/oliviertassinari/a-journey-toward-better-style.

@apendua
Copy link

apendua commented Jan 19, 2017

@oliviertassinari Thank you for a quick answer. That's a very interesting project you've been working on. Thanks for the link.

@davecyen
Copy link

davecyen commented Jan 27, 2017

+1 on using external CSS instead of inline styles.

I cannot speak towards performance, but in order to customize the default styles I am forced to use !important rules throughout my app. There's no other alternatives to override inline-styles without using !important, and the risks of using !important are well documented.

@wbecker
Copy link

wbecker commented Mar 4, 2017

In addition to this, inline styles means that you need to use 'unsafe-inline' as an option for your Content Security Policty directive. Having them local means that you can verify that that they come from the same origin.

@leefernandes
Copy link

leefernandes commented Apr 6, 2018

mui's style/theme implementation is not liked. I'd prefer scss or plain css over what's going on in material ui.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 6, 2018

@itsleeowen You are going to like: https://material-ui.com/guides/interoperability/.

@iilei
Copy link

iilei commented Jul 21, 2019

Regarding the CSP issue mentioned by @wbecker – Is there a way to render the injected styles when webpack takes care of extracting css for example using mini-css-extract-plugin?

@oliviertassinari
Copy link
Member

@iilei We have a section on the documentation for the styles and the CSP: https://material-ui.com/styles/advanced/#content-security-policy-csp. There is no css extraction support possible as we are speaking.

@kelvin2200
Copy link

Man... i was insanely excited about using MUI in my next project, because it's awesome, but this styling issue is bad. One has to take into consideration that his project may scale, component disposition may become erratic, and when dealing with dashboards and admin interfaces having dozens/hundreds of form or even small components, you end up trading the very thing you strive for (application size), for something that works out of the box and is good looking. A good library should include CORE functionality only, leaving it to the dev to sort out his logic. Even if there were NO performance hit, (if) at some point, you have to include the whole library, you find yourself with a min of +80kb to your main bundle, plus whatever the inline css takes up in kb (when SSR'ing). It's very unlikely we will see a CSS in JS solution, but it would have been the proper way to go. It would eliminate the jss dependency, a HUGE amount of stuff that is hardcoded into the components, and offer the dev, a small solution and scalable to his needs. It would also eliminate a lot of issues with SSR, ordering etc... I really don't get it, for such an amazing job, why the lousy approach???... it takes more time in documenting issues and potential fixes, than writing something that wouldn't need it;

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 30, 2019

@kelvin2200 I don't understand. What's the issue?

A couple of points. This is a v0.x issue. v4 doesn't use inline styles, at scale. JSS is a CSS-in-JS solution. We are exploring a migrating to styled-components in v5: #6115. We are exploring a variant of the components without any styles with: #6218. The overhead of one component varies between 1 kB and 30kB, never 80 kB gzipped.

@kelvin2200
Copy link

kelvin2200 commented Jul 30, 2019

Sorry for the lack in explanation

    1. it is NOT a v0.x issue, unless style tags are NOT being appended to the <head> anymore, and i missed it.
    1. when i say CSS in JS, i refer to importing css/sass/less files, which can be picked up by some css/sass/less (pre)processor and bundled into a separate file, because whatever the app is, it will never use only MUI and will definitely have a css of its own... so why not have the option of bundling everything together, since the server roundtrip will exist no matter what. In my opinion, the hardcoded styling rules will eventually become hard to maintain. Leaving everything aside, those are objects, passed through functions, which generate a piece of text, later to be injected in the <head> as a style. Be it heavy or not, that is javascript workload which shouldn't exist. Themes and visual styles should/could be changed on the fly either through some global, classname modification, or even better, by [data-<some_state>] attributes, which is even a better practice than altering classnames. You would eliminate most SSR issues, and significantly reduce the overall size of the library. Styled components [RFC] Migrate to styled-components #6115 might provide useful, but the issue will persist. I get the approach, because it's a library meant to be used with react-native, but i'm sure that even so, a cleaner solution exists.
    1. I don't really understand what you mean by "the overhead of a single component". I was talking about the case when you would need to bundle ALL the components with some app, which i've come to accept, it happens in most cases.
    1. I am not judging. Furthermore, MUI it is a wonderful thing. We all have different approaches when it comes to building a logic for something, pros and cons and all that stuff.
    1. Provide a version of the components without any styles #6218 is a very good ideea. A lot of refactoring, but it may be worth it in the end.

@iilei
Copy link

iilei commented Jul 31, 2019

@oliviertassinari rgd the above mentioned documentation section on how to implement nonces: If you use the approach without SSR, like described at https://scotthelme.co.uk/csp-nonce-support-in-nginx/ the styling is an obstacle for nonce-based CSP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests