peterbe / minimalcss

Extract the minimal CSS used in a set of URLs with puppeteer
https://minimalcss.app/
MIT License
350 stars 35 forks source link

Above the fold? #286

Open peterbe opened 5 years ago

peterbe commented 5 years ago

Worth seeing if it's worth it. I certainly know I have some examples where the CSS below the fold adds up a fair portion of the total minimal CSS.

The implementation can be something like...

document.querySelector(selector).getBoundingClientRect().top <= MAX_TOP
muralikg commented 5 years ago

Another way to do this would be to use IntersectionObserver and find everything in the viewport

var observer = new IntersectionObserver(function(entries, observer) {
    entries.forEach(entry=>{
        if (entry.isIntersecting) {
            console.log(entry.target.classList)
        }
    })
});

document.querySelectorAll('*').forEach(element=>{
    observer.observe(element)
})
muralikg commented 5 years ago

My bad I just realised that selectors can be more complex than just a class name.

EDIT Each element has to be then matched with css selectors

entry.target.matches(cssSelector)
leeoniya commented 5 years ago

check this project out: https://github.com/ABVanton200/critical-css-parser

peterbe commented 5 years ago

@leeoniya What does that have to do with "Above the fold?"? I know you built DropCSS and you believe it's better than minimalcss but please don't drop random plugs in random places.

leeoniya commented 5 years ago

What does that have to do with "Above the fold?"

wait, what? that's precisely what this project does: "Get critical (above-the-fold) and rest CSS using Puppeteer". i'm not sure how this is plugging DropCSS. i discovered critical-css-parser via Github's new "Used By" button a few days ago and thought it was interesting. it works by removing elements with a bounding box below the fold. then running a css purger. yes, it happens to use DropCSS but that's besides the point, it could use minimalcss just the same. very confused by your comment here.

peterbe commented 5 years ago

Oh I see. I the above-the-folder of critical-css-parser is implicit, not a specific section. Sorry for jumping to conclusions. The relevant implementation is this: https://github.com/ABVanton200/critical-css-parser/blob/6945428a05a6f8749804814a71afc5a8ccc16b37/src/index.js#L137

akashdsouza commented 5 years ago

This is really a much needed feature. But, as it is right now, I'm not sure if this can be supported. minimalcss uses cheerio to check if a selector is present in the dom or not. From my understanding, cheerio does not yet have a method for getBoundingClientRect as it doesn't actually render the page.

One way could be to render the page again in puppeteer to check if it is in viewport. But, this doesn't seem efficient. Any thoughts on how to implement this better?

peterbe commented 5 years ago

This is really a much needed feature.

Interesting. What's your use case for that argument?

I did some very rough estimates of my own (months ago) and found it was not worth it. Of the remaining actual critical CSS, almost all of it was needed for the above-the-fold content as so much of that CSS wasn't exclusively for the above-the-fold but for structure and stuff that's needed by the whole page. From a 100KB thatbloatedcssframework down to 10KB for the critical, and of that maybe 1KB only applicable below the fold. So it's not 0 but the advantage for no bothering is that users can potentially scroll down faster than the page has a chance to load ALL css (e.g. with loadCSS). This way there's no rush to load all CSS which can have performance benefits.

akashdsouza commented 5 years ago

Ideally, the css for below the fold content may not contribute as much to the page. But, it is usual for styles in common sections as footers to be shared across multiple pages(which wouldn't need to be included). It is also not uncommon to have some css heavy animations down the page.

Despite inlining critical css, we still need to asynchronously load the required stylesheets. This implies all styles will be applied twice on each element.

Essentially, while not a common requirement, it would be better to have an option to only inline above the fold css to get the performance benefits when required in larger pages.

akashdsouza commented 5 years ago

One way could be to render the page again in puppeteer to check if it is in viewport.

If we leveraged puppeteer's coverage API to extract the selectors, then it would be possible to check if the selector is applied on an element within the view port while the page is open in puppeteer itself.

akashdsouza commented 5 years ago

I was checking out puppeteer's coverage API. While it is extracts out the used css, it also doesn't include keyframes and font-faces. Further, puppeteer's selector APIs are asynchronous making it difficult to use them with AST parsers like css-tree.

I think it might be better to do something like extracting out above the fold html similar to critical-css-parser as you mentioned here.