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

Rename Performance Opportunity "Defer Unused CSS" #6588

Closed
ScottTravisHartley opened this issue Nov 16, 2018 · 9 comments · Fixed by #7235
Closed

Rename Performance Opportunity "Defer Unused CSS" #6588

ScottTravisHartley opened this issue Nov 16, 2018 · 9 comments · Fixed by #7235
Assignees

Comments

@ScottTravisHartley
Copy link

I find that this opportunity needs to be renamed as it's unclear in the way it's formatted. Defer Unused CSS sounds like you're suggesting for the developer/user to remove render blocking CSS files using loadCSS or another method. Which is why I was driving myself crazy as to why this rule kept coming up even though it was implemented.

However, me not paying attention I went ahead and clicked it to see the following
"Remove unused rules from stylesheets to reduce unnecessary bytes consumed by network activity). Learn More.

That's an entirely different suggestion from the language used to describe the opportunity. Deferring the CSS would be as I suggested, loading it asynchronously to not interrupt the rendering of the webpage. This rule should be renamed to something more obvious like.
"Remove Unused CSS Rules"
"Remove Unused CSS"

Using the word Defer though, in this case, doesn't make sense in my opinion thoughts?

@ScottTravisHartley
Copy link
Author

I also want to add that Eliminate Render-blocking resources is its own opportunity which gives the following suggestion.

Resources are blocking the first paint of your page. Consider delivering critical JS/CSS inline and deferring all non-critical JS/styles.

So the verbiage appears in this rule meaning don't get rid of anything but push off the loading so it doesn't interfere with the painting (asynchronous loading).

This is also like the Defer offscreen images rule, which again isn't to remove unused images but to defer the images below the initial viewport by lazy loading them. The consistency would help clear up confusion among novice users (or those who haven't had enough coffee to read the label like me (oops)).

@patrickhulce
Copy link
Collaborator

For context, it's Defer because we're giving you the benefit of the doubt that the CSS will be used eventually when the user interacts with the page. It's quite likely that the CSS is never used in any set of user interactions, but we don't know for sure.

@ScottTravisHartley
Copy link
Author

Then the context of the defer unused CSS rule should be clarified because recommending that unused CSS should be removed (albeit the tool can't be 100% positive of what's being used vs what is not), but then saying to defer it gives a mixed recommendation.

For something to be deferred its included on the page but loaded after a certain point (on scroll, after the onload event, etc) but to recommend it to be removed (or in this case part of it) isn't the same thing.

The only reason I bring this up is that the eliminate render-blocking resources rule seems to cover the defer aspect quite nicely but when someone sees defer unused CSS and wonder's why that's coming up then clicks it to be told to remove unused CSS rules it's confusing.

This is very confusing as a lot of nontechnical users will come across the recommendation on PSI and likely get mixed signals.

But I do see the reason why it was initially chosen to describe the opportunity

@ScottTravisHartley
Copy link
Author

oops hit the wrong button, was trying to close my tab >.>

@pocketjoso
Copy link

pocketjoso commented Jan 13, 2019

Hi all,
First of all thanks for Lighthouse and all good things you do for the web. 👏

Confusing title

I agree with the issue author; I also see many people who read this label (ever since Google Pagespeed Insights started using Lighthouse) as that they still have render blocking CSS, when they don't (since this warning shows for any css, async or not, that is not used on the page). It gets worse because the page you link to for further info talks more about render-blocking and critical css than it does about "removing unused CSS". It also uses "Defer" in a the more typical sense (load async, not remove from page).

Hence if you mean to have a warning that identifies unused css (as opposed to (unused) render-blocking css) using "Defer" in title is unfortunate. Something more explicit would be better, such as "Remove potentially unused CSS".

Undocumented and generally unknown optimization

It's also confusing for users that this type of optimisation is not really as far as I've seen discussed much at all anywhere on the web. This results in people being confused on how to "fix it". Split CSS per page(?) How? Do they need h/2 to benefit? etc etc. I have seen people put a lot of effort into working on this warning, without really having a good understanding of what impact it will have, sometimes thinking it will release clean critical path perf benefits (that they might have already gained).. Again, I believe this is in part because of the help page (https://developers.google.com/web/tools/lighthouse/audits/unused-css) which isn't really clear about the difference in benefits between using critical css (removing render blocking resources), and removing already async unused css.

Misguiding not mentioning that JS (likely) has bigger impact

In addition to clarifying what you mean with "removing unused CSS", and how it can be done, I think another thing that's missing in Lighthouse is guidance of how important it is to "remove unused CSS" compared to doing the same for JS. Even if there's currently no lighthouse audit for "unused JS", we know that JS is more expensive per byte (taking into account parsing and execution), and that there is usually more JS per site than CSS. In addition, there's much better tooling support for splitting JS than CSS. I think it would be helpful to the community if Lighthouse and PSI guided its users to put their efforts into trimming the JS they load before doing the same for CSS...
I should say, indeed Lighthouse does have some other warnings that relate to how much JS is loaded on the page; not whether it's used or not, but whether it's blocking the thread etc. It's just that these are obvious high priority issues.. here for CSS an issue that has much less impact is highlighted. To properly help users prioritise it, I still think it's misleading talking about "removing unused CSS" without doing the same for JS.

Now what do you think? Let me know if you want me to open a separate issue for what goes beyond the title issue itself. 👍

@patrickhulce
Copy link
Collaborator

Thanks for the very thoughtful comments @pocketjoso! We've had an audit for unused javascript for some time actually, we've just never been able to turn it on by default due to performance concerns, but we're trying! (see #6728 😃).

The help doc is indeed misaligned with what the audit is surfacing. Thanks for pointing this out!

All things considered, I think I'm in favor of renaming this to "Remove Unused CSS" and leaving the bit about "defering if it's used on interaction" to the help text.

@cagross
Copy link

cagross commented Mar 7, 2019

I've come here to agree with OP--I think the verbiage is a bit misleading. I recently brought it up in a separate Stack Overflow question (see here), and was directed here.

I see that the title was changed in a pull request. That is good to hear.

@patrickhulce patrickhulce reopened this Mar 8, 2019
@patrickhulce
Copy link
Collaborator

Re-opening to track the progress of updating the docs in WebFundamentals to reflect this more accurately. @cagross summarized the three types nicely over in #7235 (comment)

@brendankenny
Copy link
Member

brendankenny commented Jun 26, 2019

Re-opening to track the progress of updating the docs in WebFundamentals to reflect this more accurately

docs fixed in GoogleChrome/web.dev#741 (where they live now)

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

Successfully merging a pull request may close this issue.

7 participants