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

Add Critical CSS as an NPM module. #3

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

ecairol
Copy link

@ecairol ecairol commented May 3, 2023

Changes proposed in this Pull Request

  • Adds critical as an NPM module. Critical CSS is used to boost performance:

If you have poor First Contentful Paint (FCP) and see “Eliminate render-blocking resource” opportunity in Lighthouse audits it’s a good idea to give critical CSS a go.
Source - https://web.dev/extract-critical-css/

Slack.

@ecairol ecairol requested a review from tommusrhodus May 3, 2023 15:04
@ahegyes
Copy link
Contributor

ahegyes commented May 12, 2023

Oh, I just happened to notice this while working on something else on the repo.

How does simply adding that NPM module work? I would expect it to need some configuration, at the very least ...

@tommusrhodus
Copy link
Contributor

How does simply adding that NPM module work? I would expect it to need some configuration, at the very least ...

@ahegyes We're discussing this in the performance task force, ideally we're going to be moving toward Jetpack Boost critical CSS and no pursuing an NPM module like this.

@jonesch
Copy link

jonesch commented May 19, 2023

@ahegyes

How does simply adding that NPM module work? I would expect it to need some configuration, at the very least.

It would need some configuration during the build process for sure. And my guess is we need some info on the syntax for how Contractor Devs work with it.

@tommusrhodus

ideally we're going to be moving toward Jetpack Boost critical CSS

Do you know if a user/dev has control over what Critical CSS JP Boost gives us?

@ahegyes
Copy link
Contributor

ahegyes commented May 19, 2023

My concerns/curiosity stemmed from the reason why critical CSS is good for page speed, and the problems I can think of when relying on the build process to generate it.

Just be clear on what I'm talking about: critical CSS speeds up the page load by having all the CSS rule for above-the-fold content embedded in the HTML very early on. It works because the browser can already render that layout without having to wait on external CSS files to load, and doesn't have to parse anything in them that is not required for above-the-fold content. This helps with First Contentful Paint, FCP, and with Largest Contentful Paint, LCP, metrics.

However, if the critical CSS is missing any rules required for scaffolding, it can actually make things worse. For example, it can cause higher Cumulative Layout Shift, CLS, values because the layout shifts after the external CSS is loaded.

Hence why I was curious about this PR and how it aimed to solve that problem. If we build a page with Gutenberg, the theme doesn't know about the styles applied by Gutenberg and thus can't generate any critical CSS for it. So if we use some Gutenberg columns and then critical CSS from the theme, we're likely making things worse CLS-wise.

Plugins like Jetpack Boost and WP Rocket build the CSS based on the actual result on the page and have access to the entire CSS outputted, be it from the theme, Gutenberg, or the plugins. So they should pretty much always work better than something baked only in the theme.

Do you know if a user/dev has control over what Critical CSS JP Boost gives us?

I know that WP Rocket has a "fallback CSS" option where one can add their own, and also a filter for this. I'd expect Jetpack Boost to have at least the filter, and if not, we can make a case for adding it.

@ecairol
Copy link
Author

ecairol commented May 19, 2023

@ahegyes thanks for your comments. What you describe is exactly the issue we're seeing when using Jetpack Boost, so that's all valuable insight!

This Critical CSS topic is an ongoing discussion, and now looking backward this should have been a Github Issue rather than a PR, I apologize if that caused any confusion. We don't have any plans to merge this until we have a clear how its use will be recommended (if we end up recommending Critical CSS at all)

To give you some context about this PR:

  • The Performance Metrics Task Force learned that using a Critical CSS technique reduces the FCP, and we thought it was a good idea to have new projects to use it.
  • We later realized that Jetpack Boost was an option, so now we're considering that as well.
  • The purpose of this PR is to give developers a standard NPM module to use if we decide that manual Critical CSS will be a recommendation to contractors.
  • We might end up closing the PR and not recommending Critical CSS at all. But if we do recommend it, we want the scaffold to have the same NPM module for all future projects.

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

Successfully merging this pull request may close these issues.

4 participants