sitespeedio / coach

Clear Eyes. Full Hearts. Can’t Lose.
MIT License
1.21k stars 64 forks source link

Coach counts Link Preload stylesheet as render blocking #296

Open mendaomn opened 5 years ago

mendaomn commented 5 years ago

According to my tests, the coach counts as render blocking a stylesheet loaded with Link Preload

To reproduce:

docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:7.4.0 https://www.westwingsolutions.com/ -b chrome -n1

On that page, the resource /static/styles/home.later.css is loaded as follows:

<link rel="preload" as="style" href="/static/styles/home.later.css" onload="this.rel='stylesheet'">

To my knowledge, this shouldn't count as a render blocking CSS resource

I think one could either call a different function here: fastRender.js:7

Or maybe add a .filter here: fastRender.js:44

Hope this helps!

soulgalore commented 5 years ago

Hi @mendaomn thanks for the feedback, yeah we should look into that, any change you can make a PR?

Best Peter

mendaomn commented 5 years ago

Taking a deeper look at a possibile fix, it does seem you already are checking only styles with rel="stylesheet" which should by definition exclude the ones with rel="preload"

However, in my site there's a <noscript><style rel="stylesheet" ...></style></noscript> fallback and that may be the one that gets found by util.getCSSFiles()

At this point I'm not sure whether sitespeed wants and needs to take into account stuff inside of the noscripts, and therefore whether and how to fix it

Best Alessandro

soulgalore commented 5 years ago

Hi @mendaomn I'm planning to rework the recommendations for the Coach late October or late November, let keep this issue open until then. I will concentrate the time now first to release the new version of sitespeed.io etc.

Best Peter

mendaomn commented 5 years ago

Hey @soulgalore

Congrats on releasing Coach 3.0!! It looks like it features a lot of cool stuff!

I was wondering whether this issue is still relevant, or is it now to be closed? :)

Best Alessandro

soulgalore commented 5 years ago

Hi @mendaomn no I haven't looked at it, sorry!

Best Peter