Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Rename: Defer Unused CSS #733

Closed
addyosmani opened this issue Mar 17, 2019 · 12 comments
Closed

Rename: Defer Unused CSS #733

addyosmani opened this issue Mar 17, 2019 · 12 comments
Labels
P2 A normal priority task. This is the default for most issues.

Comments

@addyosmani
Copy link
Member

cc @demianrenzulli re: https://web.dev/fast/defer-unused-css

Over on Lighthouse there's been some back and forth around renaming the 'Defer Unused CSS' audit to 'Remove Unused CSS'.

In the above case, Lighthouse opted to get more aggressive with pushing folks to remove any styles that were non-critical rather than just deferring them. I agree that makes sense for dead code. Now in our current guide, we're talking about a related topic - deferring non-critical styles (which may be needed in future pages). Should we rename this guide to 'Defer non-critical CSS'? Does this end up mis-aligning with Lighthouse?

The guide text seems fine for conveying the critical-path CSS optimization technique that relies on tools like Critical and https://github.com/filamentgroup/loadCSS.

cc'ing @patrickhulce for his two cents

@patrickhulce
Copy link
Contributor

Through discussion in the threads @addyosmani linked, we identified 3 main sets of CSS we're talking about here and different actions for each.

Critical CSS: CSS rules used on the page that affect above-the-fold content, which, if removed, could result in noticeable differences in layout/style.

Inline this type of CSS.

Non-Critical CSS: CSS rules used on the page that affect below-the-fold content, or content not viewable immediately on page load.

Defer this type of CSS.

Unused CSS: CSS rules that have no chance at all of being used on the page.

Remove this type of CSS.

The discussion was specifically around our audit that detects "unused CSS" which can technically fall into either of the latter two buckets due to the way Lighthouse measures "unused" but is frequently in the last bucket. The name change was also motivated by the fact that our render-blocking resources audit already detects CSS resources that should be deferred/inlined and the difference between the two was unclear to our users.

@demianrenzulli
Copy link
Contributor

Hi guys, thanks for all the comments.

On the Glitch demo, we are actually using the three kind of styles mentioned by @patrickhulce, and suggesting to inline the critical, and defer the rest (including non-critical and unused).

Based on your comments, it seems like it would make sense to do two changes to the article:

  1. Renaming the guide to 'Defer non-critical CSS', as @addyosmani suggested.
  2. Edit the guide, and suggest developers to eliminate those classes that are not used at all, by removing them from the CSS, instead leaving them there, and deferring the load of the file, based on comments by @patrickhulce.

Note: in the CSS of the demo, the only "unused" class is "h2". I would only leave it on the demo, as long as you think the explanation on this point might be of value for developers. Otherwise, I can just remove it completely.

@robdodson
Copy link
Contributor

I think changing things is fine so long as we make sure that the Lighthouse audits in our measure tool point to the correct resources.

The current guide is pointed to by the following audits:

- unused-css-rules
- render-blocking-resources

After the proposed edits would this guide still be the right resource to point both of those audits to? It seems like if it's talking about all three buckets the answer might be "yes" but it may need to clearly state in the guide which section fixes which audit.

@robdodson robdodson added content P2 A normal priority task. This is the default for most issues. labels Mar 18, 2019
@demianrenzulli
Copy link
Contributor

Adding more context to @robdodson comment, in case it helps:

The article starts with an unoptimized page, which requests the CSS, containing critical, non-critical and unused classes, as mentioned before.

Running Lighthouse on that page (see report), shows "Defer unused CSS" as "Passed", because the file might not be large enough to be flagged as an optimization by the tool.

For that reason, we focused on showing the improvements on FCP, and solving the "Eliminate render-blocking suggestion", to demonstrate the impact of this optimization.
(In any case, the technique explained on the article addresses both LH suggestions, as mentioned by Rob).

Maybe, as part of the changes to make on this article, would make sense to a note, explaining under which conditions the "Defer unused CSS" suggestion might appear, and why it's not showing up in this case.

@robdodson
Copy link
Contributor

I think that sounds good.

@demianrenzulli
Copy link
Contributor

Thanks @robdodson!

@patrickhulce / @addyosmani: Could you suggest what would be a good way to communicate how LH flags the "Defer CSS" suggestion, so that developers know that, for a given CSS file size they'll receive that?

For example: in the case of Minify CSS, this is calculated by defining a byte/percentage threshold.

Will be awaiting your confirmation on the changes to be made, before start working on the PR as well.

Thanks!

@patrickhulce
Copy link
Contributor

As @robdodson mentioned, Lighthouse has two different audits that deal with CSS in the "Defer CSS" bucket.

  1. unused-css-rules, the title of this audit is "Remove Unused CSS" in latest Lighthouse versions. We flag all stylesheets (whether inline or as separate files) that have at least 2048 bytes of CSS rules that weren't used while rendering the above-the-fold content.
  2. render-blocking-resources, the title of this audit is Eliminate render-blocking resources in the latest Lighthouse versions. We flag all media-matching, non-disabled CSS files that were discovered by the HTML parser and not loaded with an async loading method (CSS files added via script and the onload="this.rel" hack are fine). There is no file size threshold, but if the time savings on first contentful paint by inlining the assets is less than ~100ms then we will hide the results by default.

Hope this helps!

@demianrenzulli
Copy link
Contributor

Sounds good @patrickhulce! In that case, I can add a note explaining that the "unused-css-rules" suggestion is not being raised, because the savings are less than 2048 bytes.

@demianrenzulli
Copy link
Contributor

Sorry guys, I have one more question about the renaming:

From @patrickhulce prev. comment, LH will now suggest to remove stylesheets that weren't used for the ATF content.
Now, if you "remove" instead of "defer" those styles: what happens with the styles applied to BTF content?

Asking because @addyosmani mentioned that deferring would be useful for future pages, and it seems like, in this case, deferring would also be useful for that part of the page... or maybe I'm missing something else? (I'd need to explain this distinction clearly on the guide).

@patrickhulce
Copy link
Contributor

LH will now suggest to remove stylesheets that weren't used for the ATF content.
Now, if you "remove" instead of "defer" those styles: what happens with the styles applied to BTF content?

To clarify, we aren't intending everyone to completely remove all the CSS, that's the title of the audit now because folks were confused how it was different from "Eliminate render-blocking resources", and the reality is way more of it is unused than simply applying to unseen content (it's frequently for other pages, large parts of a framework like bootstrap that are unused, etc).

In practice, rules for BTF content will typically be "used" while rendering, and it's only the rules for interaction (hover rules, menu rules, etc) that are typically included in this set unnecessarily. Complex apps can certainly reach >2KB of such rules per stylesheet but the reported savings after getting rid of all the truly unused rules is typically so small I'm comfortable framing the guide mostly around the removal aspect for this audit and just mentioning that some false positives may occur if you have lots of complex interaction-based CSS.

@demianrenzulli
Copy link
Contributor

Thanks for the clarification Patrick,

I'll move ahead with the proposed changes, and will let you guys know, so you can take a look.

@addyosmani
Copy link
Member Author

Linking to GoogleChrome/lighthouse#7235 as a cross-reference here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 A normal priority task. This is the default for most issues.
Projects
None yet
Development

No branches or pull requests

4 participants