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

[Audit] Avoid CSS @import #11863

Open
andydavies opened this issue Dec 21, 2020 · 7 comments
Open

[Audit] Avoid CSS @import #11863

andydavies opened this issue Dec 21, 2020 · 7 comments

Comments

@andydavies
Copy link

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:

  • Title: Avoids css @import
  • Description: Using CSS @import delays the discovery of stylesheets and delays rendering (to be wordsmithed)

When failing:

  • Title: Avoids css @import
  • Description: Using CSS @import delays the discovery of stylesheets and delays rendering (to be wordsmithed)
  • Table: Containing stylesheet, @imported stylesheet

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
Copy link
Collaborator

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
Copy link
Member

@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
Copy link
Author

@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
Copy link
Author

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
Copy link
Member

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
Copy link

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
Copy link
Author

andydavies commented Aug 17, 2021 via email

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

No branches or pull requests

5 participants