Update faq.md #346

Merged
merged 2 commits into from Jan 4, 2017

Projects

None yet

5 participants

@SachaG
Contributor
SachaG commented Jan 2, 2017 edited

Adding a section to the FAQ to clarify the fact that styled-components will generate duplicate styles.

@SachaG SachaG Update faq.md
ede33a4
@k15a
k15a approved these changes Jan 2, 2017 View changes

LGTM 👍

docs/faq.md
+}
+```
+
+You may decided that this is an acceptable drawback since increasing the size of a CSS file rarely has any noticeable impact on performance compared to adding a single image to a site, but if this is a concern you can use the `css` helper to factor our the common styles into their own declaration:
@vdanchenkov
vdanchenkov Jan 2, 2017 edited Member

Usage of css helper has no effect on generated css rules. Second example will generate exactly the same rules in style tag.

@SachaG
SachaG Jan 2, 2017 Contributor

Oh, I misunderstood what @mxstbr told me then. Is there any way to achieve what I'm talking about?

@vdanchenkov
vdanchenkov Jan 2, 2017 Member

What is your concern? Regular usage or server-side rendering.

If it's about regular usage - number of rules in generated classes doesn't matter in practice.

If it's about SSR I'm not sure what to recommend, by SSR is not supported yet anyway.

@SachaG
SachaG Jan 2, 2017 Contributor

Regular usage. I'm not sure if I agree that the number of rules doesn't matter. Or at least, I think it matters enough to some people to be worth addressing somewhere in the docs.

@vdanchenkov
vdanchenkov Jan 2, 2017 Member

Sorry, you are right that it worth mentioning. I meant that I'm not aware of any performance issues related to it. If you have some measurements indicating that extraction of common rules improves performance I'd love to discuss it.

@SachaG
SachaG Jan 3, 2017 Contributor

Wouldn't extracting common rules make the file smaller, and thus improve performance? The improvement would then strictly depend on the number of rules extracted. So I'm not sure what there is to measure?

Again I know in most cases this is not a major worry, but I just think people should understand how styled-components works in order to make the decision themselves.

@mxstbr
mxstbr Jan 3, 2017 Member

Wouldn't extracting common rules make the file smaller, and thus improve performance?

No, since you're only sending down the JS file the only difference that would make is that there would be less CSS injected into the HEAD of the DOM.

We've tested this quite thoroughly (/cc @geelen) and not noticed any performance impact related to that. This will become relevant when we support SSR (#214), in which case you will send down slightly more CSS.

I'm inclined to include this note?

@SachaG
SachaG Jan 3, 2017 Contributor

Oh right, I was thinking about SSR. In a way, I think the fact that I got pretty much everything wrong about this is in itself a good reason to include a note about it in the docs :)

@k15a
k15a Jan 3, 2017 Member

Why is this a problem with SSR? You are probably not changing the styles on the server when rendering because you don't have interactions from the user so no new styles have to be applied?
I am missing something important?

@vdanchenkov
vdanchenkov Jan 3, 2017 Member

There might be several instances of one component in different states. But I think that gzip will cover it anyway.

@mxstbr
mxstbr Jan 3, 2017 Member

Actually, you're right! gzip will cover that as far as I'm aware, so it shouldn't be an issue?

@SachaG
SachaG Jan 3, 2017 Contributor

You can only gzip individual CSS files though, or does that also help with CSS injected directly into the HTML file?

@k15a
k15a Jan 4, 2017 Member

Yes you can and should gzip all your files. So HTML with CSS injected should also be gzipped.

@SachaG SachaG Update FAQ to explain why this isn't a problem
Update FAQ to explain why this isn't a problem
a5c8503
@SachaG
Contributor
SachaG commented Jan 3, 2017

OK, let me know if this is better :)

@geelen
geelen approved these changes Jan 4, 2017 View changes
@mxstbr mxstbr merged commit 22997d7 into styled-components:master Jan 4, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mxstbr
Member
mxstbr commented Jan 4, 2017

Awesome, thanks so much @SachaG!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment