GoogleChrome / lighthouse

Automated auditing, performance metrics, and best practices for the web.
https://developer.chrome.com/docs/lighthouse/overview/
Apache License 2.0
28.2k stars 9.34k forks source link

[Audit] Avoid CSS @import #11863

Open andydavies opened 3 years ago

andydavies commented 3 years ago

Provide a basic description of the audit

Check whether stylesheets are being imported from within other stylesheets.

Aim is to identify the imported stylesheets through the request chain.

Drawback to this approach is that only identifies cases where the stylesheet is imported in the Lighthouse run. Searching stylesheets source code is an alternative approach that works for a wider set of scenarios e.g. @import enclosed within a media query. This second approach would still miss some stylesheets e.g those wrapped in IE's conditional comments.

How would the audit appear in the report?

This audit will come under performance.

When passing:

When failing:

How is this audit different from existing ones?

There's no existing audit.

Imported stylesheets are visible in the Avoid Chaining Critical Requests audit but they're not highlighted and there's no specific advice about avoiding them.

What % of developers/pages will this impact?

Querying https://publicwww.com for "@import url" filetype:css returns over 100,000 CSS files using @import.

How is the new audit making a better web for end users?

Styles imported into another stylesheet via @import delay initial rendering as they are discovered late ā€“ a browser must download and parse the outer stylesheet before they are discovered.

Gains will depend on the number and size of imported stylesheets, and the length of the import chain.

As example of the gains that might be made here's a comparison of snowpack.dev with and without the imported stylesheet on Moto G4 / 3G.

In the top test the @imported stylesheet has been blocked and the page starts rendering 0.5s earlier.

https://www.webpagetest.org/video/compare.php?tests=201221_DiWH_925d14973580957e917ef244fb6f322a%2C201221_Di7P_cec2262fdef423f979661401f762199f&thumbSize=200&ival=100&end=visual

If the imported stylesheet was directly in the document the gains should be roughly similar as the preloader will discover it and dispatch the request earlier (avoiding the latency penalty)

What is the resourcing situation?

@andydavies will implement and document the audit

Any other links or documentation that we should check out?

None

patrickhulce commented 3 years ago

Thanks for filing @andydavies I think this is a great idea! This could even be an opportunity with quantified savings estimates using our graphs.

Are there any situations you can think of where @import is really the only option and preloading it wouldn't solve the issue? I'm thinking there must be a third-party situation of this sort out there, but I can't think of one. If not, could apply to everything not just first-party resources šŸŽ‰

paulirish commented 3 years ago

@andydavies did you start prototyping out an implementation on this?

IMO, preventing imports is totally legit. We've just been ignoring this because we thought @import was not used heavily. I guess you've seen it enough times to warrant some action, eh?

andydavies commented 3 years ago

@paulirish Yes, I've got a prototype up and running just need to clean it up so I can make a PR for review but bee na bit busy with the day job ATM

I've gone for the approach of using the PageDendencyGraph rather than searching code for @import

Comment from the top of my audit says

* Identifies external stylesheets via the PageDependencyGraph and then iterates through them to identify
 * any stylesheets with dependents that are also external stylesheets.
 * 
 * Flattens chains, so in results 1.css > 2.css > 3.css is represented as:
 * 
 * 1.css
 *  - 2.css
 * 
 * 2.css
 *  - 3.css
 * 
 * Only identifies sheets that are actually requested so an @import wrapped in a media-query that doesn't
 * apply would be missed.
 * 
 * An alternative approach would be to search stylesheets for @import (allowing for those wrapped in 
 * comments etc)
andydavies commented 3 years ago

Still see CSS imports more frequently than I'd like even on 'modern sites' e.g. snowpack.dev had one until a few months ago https://www.webpagetest.org/result/201202_DiS7_f3b2bb222f6532ba74ad8fc22866c413/1/details/#waterfall_view_step1, https://stanford.edu has a few, and a search of publicwww turns up plenty.

paulirish commented 3 years ago

I've gone for the approach of using the PageDendencyGraph rather than searching code for @import

nice. yeah we think that's probably the smarter.. just gotta deal with some cases where there's <style> in the HTML. but ultimately seems doable.

Still see CSS imports more frequently than I'd like even on 'modern sites'ā€¦

Awesome. Appreciate the extra evidence.

so I can make a PR for review but bee na bit busy with the day job ATM

all good, not rushing you... just didnt want to step on your toes. Also you can always link us your WIP branch if you're okay with someone else taking it across the finish line.

looking forward to the blog posts on the 3p work you've been doing! :)

xiaochengh commented 3 years ago

Is this audit on all @import statements, or just @import statements used in external stylesheets?

@import used in inline <style> elements can be discovered as early as a <link rel=stylesheet>, and shouldn't affect loading performance. So there's no need to discourage it.


Context

I'm working on the CSS Cascade Layers feature, which currently relies on @import to assign external/3P style to a cascade layer. The syntax is @import url(...) layer(my-layer).

We are working on introducing a similar syntax to <link rel=stylesheet>, but right now <style>@import url(...) layer(my-layer)</style> is the only valid syntax to achieve the purpose.

andydavies commented 3 years ago

Just @import referenced from external stylesheets

On the @imports in HTML it's probably worth checking the Blink quoting issues mentioned here are / get fixed https://csswizardry.com/2018/11/css-and-network-performance/

On Tue, 17 Aug 2021 at 21:08, Xiaocheng Hu @.***> wrote:

Is this audit on all @import https://github.com/import statements, or just @import https://github.com/import statements used in external stylesheets?

@import https://github.com/import used in inline is the only valid syntax to achieve the purpose.

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/lighthouse/issues/11863#issuecomment-900596180, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADIILFD6R52FIGMA7TKEX3T5K6VVANCNFSM4VEKC26Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .