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

CSS Concatenating: jetpack.css loads all styles, even if they've been manually dequeued #1258

Closed
cferdinandi opened this issue Nov 4, 2014 · 21 comments · Fixed by #39518
Closed
Assignees
Labels
[Focus] Performance General [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Comments

@cferdinandi
Copy link

As of this weekend, I'm now seeing jetpack.css loading on the front-end, even though the only feature I'm using in Markdown. This file contains CSS for all Jetpack features.

I'm currently overriding this behavior using this snippet in functions.php, but this feels like it may be a bug:

add_filter( 'jetpack_implode_frontend_css', '__return_false' );

More info: https://www.twirlingumbrellas.com/wordpress/remove-deregister-jetpack-contact-form-styles/

@jeherve
Copy link
Member

jeherve commented Nov 5, 2014

Yes, that's a known issue with Jetpack 3.2. Jetpack now concatenates its CSS into a jetpack.css file, overwriting any custom changes you may have made by manually dequeuing one of Jetpack's CSS files. We'll get this fixed.

You can follow our progress in #1235. I'll close this issue as it's a duplicate.

@jeherve jeherve closed this as completed Nov 5, 2014
@jeherve jeherve added the [Closed] Duplicate Duplicate of an existing issue. label Nov 5, 2014
@georgestephanis georgestephanis removed the [Closed] Duplicate Duplicate of an existing issue. label Nov 5, 2014
@cferdinandi
Copy link
Author

@jeherve - Thanks Jeremy. Didn't catch that when I opened the ticket. Cheers!

@jeherve jeherve added General [Type] Bug When a feature is broken and / or not performing as intended labels Nov 7, 2014
@jeherve
Copy link
Member

jeherve commented Nov 7, 2014

Reopening this since #1235 was closed when fixing the CSS specificity issue. This bug is regarding how we still include all stylesheets in jetpack.css, even when that stylesheet has been manually dequeued.

@jeherve jeherve reopened this Nov 7, 2014
@jeherve jeherve changed the title Jetpack loading all styles to front-end whether or not they're required CSS Concatenating: jetpack.css loads all styles, even if they've been manually dequeued Nov 7, 2014
@georgestephanis
Copy link
Member

Is there a core action we can track for when a stylesheet is dequeued?

@jeherve
Copy link
Member

jeherve commented Nov 12, 2014

Can we track all wp_dequeue_style that may be hooked into wp_enqueue_scripts?

@georgestephanis
Copy link
Member

Just checked, there isn't an action that does this.

@richardmtl richardmtl added this to the 3.6 milestone Apr 14, 2015
@dereksmart dereksmart modified the milestones: 3.6, 3.7 Jun 25, 2015
@samhotchkiss samhotchkiss modified the milestones: 3.7, Needs Triage Aug 28, 2015
@zinigor zinigor modified the milestones: 3.9, Needs Triage Dec 1, 2015
@zinigor zinigor self-assigned this Dec 1, 2015
@jeherve jeherve modified the milestones: 4.0, 3.9 Jan 15, 2016
@jeherve jeherve removed this from the 4.1 milestone Jun 17, 2016
@jeherve jeherve added the [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it label May 11, 2020
@stale stale bot removed the [Status] Stale label May 11, 2020
@jeherve jeherve removed this from the Not Currently Planned milestone May 11, 2020
@jeherve
Copy link
Member

jeherve commented Oct 23, 2020

Reopening this, as it is still a valid enhancement.

@jeherve jeherve reopened this Oct 23, 2020
@adaminfinitum
Copy link

I came across this thread after a google search led me to issue #15734, which is closed because it's basically a duplicate of this issue.

I'm glad this is still open because it is a valid enhancement, what I am working on now to address it is adding a filter to disable the concatenation and then using the "Asset CleanUp" plugin to disable the files I don't need and letting my caching plugin concatenate everything else into as few files as possible.

Hopefully others will find this approach helpful.

@github-actions
Copy link
Contributor

This issue has been marked as stale. This happened because:

  • It has been inactive in the past 6 months.
  • It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`, etc.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

@jeherve
Copy link
Member

jeherve commented Mar 7, 2023

This was also brought up in this post:

The only feature of Jetpack that I use in the frontend of my site is the Subscribe block that you can see in my site footer. There’s no way that simple text input and button needs 21.5 KB of CSS.

As we had discussed in #7656, we should try to find a way to not include everything in the concatenated file if it is not needed.

@jeherve jeherve moved this to 🔍 To investigate in [XWP] Jetpack performance Apr 13, 2023
@jeherve jeherve added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Apr 13, 2023
@jeherve
Copy link
Member

jeherve commented Apr 13, 2023

I'd like to come back to this issue and summarize the conversations above a bit:

We currently use this to enqueue a concatenated jetpack.css file on all frontend views when Jetpack is connected to WordPress.com (as long as you do not use the filter to disable the feature nor have SCRIPT_DEBUG set on your site).

That jetpack.css file is built off a list of stylesheets from individual modules that are not enqueued when the concatenated file is enqueued.

This has a few inconvenients:

  1. It doesn't take module status into account; you end up with a full jetpack.css even if you don't actually need any of the styles. In that case, not actually doing the whole concatenation would be better. With the advent of http/2, we may also ask ourselves if concatenation is even needed at all anymore.
  2. That list of stylesheets also doesn't take stylesheets added by blocks into account. Those are added individually, only when needed (when the block is present on the page), and with their potential dependencies. This may not have to be modified right now.

Also of note, this is only about CSS. We do not have anything similar for the different JavaScript files needed by different Jetpack modules.

I'll cc @Automattic/heart-of-gold on this conversation, since they may have some useful input on performance here.

@cferdinandi
Copy link
Author

With the advent of http/2, we may also ask ourselves if concatenation is even needed at all anymore.

This is pretty key, IMO. There's no benefit to end-users when HTTP/2 is enabled, and it can often be worse than just loading the CSS that's needed.

@thingalon
Copy link
Member

This is pretty key, IMO. There's no benefit to end-users when HTTP/2 is enabled, and it can often be worse than just loading the CSS that's needed.

It depends. Concatenating resources can still assist with improving compression, as compressing similar text patterns together is more efficient than separately. Based on a quick test, Brotli compression brings jetpack.css down from 99.7 kB to 17.1 kB.

Reviewing the contents of the uncompressed 99.7 kB, the big parts appear to be:

  • 18.3 kB for carousel.
  • 16.5 kB for sharedaddy.
  • 16.2 kB for contact forms.
  • 12.2 kB for swiper.
  • 6.0 kB for related posts.
  • The remaining ~30 kB is comprised of many files smaller than 5 kB each.

My gut feeling is that it's worth splitting out the bigger items that aren't always needed into their own files (e.g.: carousel), but it's better to keep the tiny files and the non-optional files combined. It's hard to estimate exactly how much of the compressed size would be saved by removing things like carousel, but we can expect at least a few kB savings there, which makes it worthwhile IMHO.

But we should probably test actual compressed sizes with different bundling to verify my gut feeling. Compression is a little hard to predict without testing. :)

@cferdinandi
Copy link
Author

Brotli compression brings jetpack.css down from 99.7 kB to 17.1 kB

17.1kb is not an insignificant amount of code, especially on slower connections and older devices.

it's better to keep the tiny files and the non-optional files combined

How would you define "non-optional"? I don't use Jetpack for a single front-end feature.

@p3ob7o
Copy link
Member

p3ob7o commented Oct 15, 2023

One specific case that we could handle is when no feature using jetpack.css on the front end is activated. Then we should not load it at all.

jeherve added a commit that referenced this issue Nov 14, 2023
See #1258

We currently enqueue a `jetpack.css` concatenated CSS file as soon as your site is connected to WordPress.com.
That jetpack.css file is built off a list of stylesheets from individual modules that are not enqueued when the concatenated file is enqueued.

In this commit, we add a new condition: for the file to be enqueued, there has to be at least 2 modules that need it. This will avoid loading the file when it wouldn't even be needed at all.
This requires a change in the implementation: we now check for active modules before we dequeue individual assets / enqueue jetpack.css. This is an additional call to get an option (( new Modules() )->get_active()).
This also means that the new $modules_with_concatenated_css variable will need to be kept updated.

Note that this is only a partial improvement. We still need to address the larger questions around the implementation in today's world, as discussed here:
#1258 (comment)
@github-project-automation github-project-automation bot moved this from 🔍 To investigate to ✅ Done in [XWP] Jetpack performance Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Performance General [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.