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

Single style element vs. style per component class #45

Closed
kof opened this issue Apr 10, 2016 · 6 comments
Closed

Single style element vs. style per component class #45

kof opened this issue Apr 10, 2016 · 6 comments

Comments

@kof
Copy link

kof commented Apr 10, 2016

I wonder if you guys did any performance comparison of this 2 different approaches:

The one of aphrodite (as far as I understand):

  • one style element for all style sheets
  • rules converted to a CSS string when they are used by a component through css() function
  • subsequent css() calls are buffered and rendered by asap flush

vs.

The one of jss or better to say react-jss:

  • style element for each component class
  • rules converted to a CSS string on componentWillMount
  • style is attached to the dom when first component using this sheet is about to mount
  • same sheet instance used for all component instances
@kof kof changed the title Single style element vs. style per component Single style element vs. style per component class Apr 10, 2016
@kof
Copy link
Author

kof commented Apr 10, 2016

A problem I can see with aphrodite's approach is that writes to the style element will become slower with every added rule. If at some point, when all styles are rendered, lets say 10kb of styles, I add one more rule, it will create a CSS string with all previous styles + the new one and insert it into style element. Correct me if I am wrong.

The problem is not just parsing of that CSS string, but also that CSSRule instances are not going to be reused.

@xymostech
Copy link
Contributor

The main reason that we use a single <style> tag is that IE 9 doesn't support more than 31 <style> tags in the document: #6 There's a link to a jsperf in that issue

I think you are correct, the browser probably has to re-parse the whole CSS string. We add new TextNodes to our single <style> tag, which is maybe faster than setting innerText or something though. We also do some buffering internally so that we probably only write to the <style> tag once per render, not once per call to css().

As far as comparing to jss, Aphrodite's interface is pretty different. It lets you mix many styles from different sources, instead of having to declare up-front which ones you want merged. Also, aphrodite only generates CSS for the styles that you actually use, which is nice in general but especially a boon if you're doing server-side rendering (since it won't generate CSS for elements that you don't use).

Hopefully this answers your questions, feel free to re-open if I missed something.

@kof
Copy link
Author

kof commented Apr 11, 2016

Thank you for your answer.

Yes I have heard of a limit in IE9, that is bad. JSS needs a legacy rendering mode for IE9.

only generates CSS for the styles that you actually use

This is probably not much of profit when rendering at runtime. The moment user clicks a button and you render an element that uses style rule which is not rendered, you are doing the whole overhead of rerendering the entire style sheet just to add for e.g. one rule to the style sheet. This is post probably performance drawback than optimization.

However I agree that server-side rendering will benefit from this. As it renders just one state for each component. I am very curious how much bandwidth this can safe! Can you generate some stats on the code base you have?

I think you are correct, the browser probably has to re-parse the whole CSS string.

Of course they will if you overwrite the entire sheet.

I saw this jsperf bench: http://jsperf.com/append-to-style-vs-create-new-style

  1. I am wondering what happens if you use insertRule/addRule api. I can't add it because jsperf instantly classifies me as a robot. (which I probably am, but how the hell do they know ...)

    var style = document.createElement("style");
    style.setAttribute("data-test", "true");
    document.head.appendChild(style);
    var sheet = style.sheet || style.styleSheet;
    var cssRules = sheet.cssRules || sheet.rules;
    
    for (var i = 0; i < 500; i++) {
        var selector = ".test-" + i;
        var style = "{ color: red; font-size: 20px; }";
        var nextIndex = cssRules.length;
        if (sheet.insertRule) sheet.insertRule(selector + style, nextIndex);
        else sheet.addRule(selector, style, nextIndex);
    }
    elem.outerWidth;
  2. This bench is not quite correct for aphrodite because it does not consider the buffering. This means that there is no inserting of text nodes for each rule. When a text node inserts a bunch of rules at once, this might be a good thing.

@xymostech
Copy link
Contributor

I am very curious how much bandwidth this can safe! Can you generate some stats on the code base you have?

I don't have these offhand, but I'll look into it. Unfortunately, most of the things that we're using aphrodite styles for were written from scratch with them, so we don't have anything to compare that to.

I am wondering what happens if you use insertRule/addRule api.

That API looks really interesting! That could definitely help with performance, I'll check it out.

@kof
Copy link
Author

kof commented Apr 11, 2016

I don't have these offhand, but I'll look into it. Unfortunately, most of the things that we're using aphrodite styles for were written from scratch with them, so we don't have anything to compare that to.

You could log the rules used during rendering and then log entire styles objects used during the same rendering. Like turn on and off the cherry picking of rules and compare the sizes.

@kof
Copy link
Author

kof commented Apr 11, 2016

Also you might be interested in cssinjs/jss#204

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

No branches or pull requests

2 participants