Skip to content

Commit

Permalink
Performance: remove imploding CSS code (#39518)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kraftbj
kraftbj authored Oct 2, 2024
1 parent 2b8fce1 commit 738e909
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 296 deletions.
3 changes: 1 addition & 2 deletions projects/plugins/jetpack/.phan/baseline.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
// PhanPluginInvalidPregRegex : 1 occurrence
// PhanPluginUseReturnValueInternalKnown : 1 occurrence
// PhanTypeConversionFromArray : 1 occurrence
// PhanTypeVoidArgument : 1 occurrence
// PhanUndeclaredConstant : 1 occurrence
// PhanUndeclaredExtendedClass : 1 occurrence
// PhanUndeclaredTypeReturnType : 1 occurrence
Expand Down Expand Up @@ -509,7 +508,7 @@
'tests/php/general/test_class.jetpack-client-server.php' => ['PhanDeprecatedFunction', 'PhanTypeMismatchArgumentProbablyReal'],
'tests/php/general/test_class.jetpack-user-agent.php' => ['PhanDeprecatedFunction'],
'tests/php/general/test_class.jetpack-xmlrpc-server.php' => ['PhanDeprecatedFunction', 'PhanPluginSimplifyExpressionBool', 'PhanRedundantCondition'],
'tests/php/general/test_class.jetpack.php' => ['PhanPluginDuplicateAdjacentStatement', 'PhanTypeMismatchPropertyDefault', 'PhanTypeVoidArgument'],
'tests/php/general/test_class.jetpack.php' => ['PhanPluginDuplicateAdjacentStatement', 'PhanTypeMismatchPropertyDefault'],
'tests/php/json-api/test-class.json-api-jetpack-endpoints.php' => ['PhanTypeMismatchArgumentProbablyReal'],
'tests/php/json-api/test-class.json-api-jetpack-site-endpoints.php' => ['PhanImpossibleConditionInLoop'],
'tests/php/json-api/test-class.json-api-post-jetpack.php' => ['PhanTypeMismatchArgument'],
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/remove-imploding-css-code
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: enhancement

Removed the option to use a combined CSS file for Jetpack features. Previous to Jetpack 13.9, this was the default behavior. In Jetpack 13.9, it was made optional to rely on individual CSS files for the features used on each page to improve page loading.
201 changes: 2 additions & 199 deletions projects/plugins/jetpack/class.jetpack.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,73 +75,6 @@ class Jetpack {
*/
public $xmlrpc_server = null;

/**
* List of Jetpack modules that have CSS that gets concatenated into jetpack.css.
*
* See $concatenated_style_handles for the list of handles,
* and the implode_frontend_css method for more details.
*
* When updating this list, make sure to update $concatenated_style_handles as well.
*
* @var array List of Jetpack modules.
*/
public $modules_with_concatenated_css = array(
'carousel',
'contact-form',
'infinite-scroll',
'likes',
'related-posts',
'sharedaddy',
'shortcodes',
'subscriptions',
'tiled-gallery',
'widgets',
);

/**
* The handles of styles that are concatenated into jetpack.css.
*
* When making changes to that list,
* you must also update concat_list in tools/webpack.config.css.js,
* and to $modules_with_concatenated_css if necessary.
*
* @var array The handles of styles that are concatenated into jetpack.css.
*/
public $concatenated_style_handles = array(
'jetpack-carousel-swiper-css',
'jetpack-carousel',
'grunion.css',
'the-neverending-homepage',
'jetpack_likes',
'jetpack_related-posts',
'sharedaddy',
'jetpack-slideshow',
'presentations',
'quiz',
'jetpack-subscriptions',
'jetpack-responsive-videos',
'jetpack-social-menu',
'tiled-gallery',
'jetpack_display_posts_widget',
'gravatar-profile-widget',
'goodreads-widget',
'jetpack_social_media_icons_widget',
'jetpack-top-posts-widget',
'jetpack_image_widget',
'jetpack-my-community-widget',
'jetpack-authors-widget',
'wordads',
'eu-cookie-law-style',
'flickr-widget-style',
'jetpack-search-widget',
'jetpack-simple-payments-widget-style',
'jetpack-widget-social-icons-styles',
'wpcom_instagram_widget',
'milestone-widget',
'subscribe-modal-css',
'subscribe-overlay-css',
);

/**
* Contains all assets that have had their URL rewritten to minified versions.
*
Expand Down Expand Up @@ -763,17 +696,6 @@ function () {
// Update the site's Jetpack plan and products from API on heartbeats.
add_action( 'jetpack_heartbeat', array( Jetpack_Plan::class, 'refresh_from_wpcom' ) );

/**
* This is the hack to concatenate all css files into one.
* For description and reasoning see the implode_frontend_css method.
*
* Super late priority so we catch all the registered styles.
*/
if ( ! is_admin() ) {
add_action( 'wp_print_styles', array( $this, 'implode_frontend_css' ), -1 ); // Run first.
add_action( 'wp_print_footer_scripts', array( $this, 'implode_frontend_css' ), -1 ); // Run first to trigger before `print_late_styles`.
}

// Actually push the stats on shutdown.
if ( ! has_action( 'shutdown', array( $this, 'push_stats' ) ) ) {
add_action( 'shutdown', array( $this, 'push_stats' ) );
Expand Down Expand Up @@ -5499,6 +5421,8 @@ public function deprecated_hooks() {
'replacement' => null,
'version' => 'jetpack-13.4.0',
),
// jetpack_implode_frontend_css has been removed, but is not listed here. The updated behavior is exactly the only use of the filter.
// We can reassess formally deprecating it here later; for now, it would be noise with no functional difference.
);

foreach ( $filter_deprecated_list as $tag => $args ) {
Expand Down Expand Up @@ -5630,127 +5554,6 @@ public static function absolutize_css_urls( $css, $css_file_url ) {
return $css;
}

/**
* This methods removes all of the registered css files on the front end
* from Jetpack in favor of using a single file. In effect "imploding"
* all the files into one file.
*
* Pros:
* - Uses only ONE css asset connection instead of 15
* - Saves a minimum of 56k
* - Reduces server load
* - Reduces time to first painted byte
*
* Cons:
* - Loads css for ALL modules. However all selectors are prefixed so it
* should not cause any issues with themes.
* - Plugins/themes dequeuing styles no longer do anything. See
* jetpack_implode_frontend_css filter for a workaround
*
* For some situations developers may wish to disable css imploding and
* instead operate in legacy mode where each file loads seperately and
* can be edited individually or dequeued. This can be accomplished with
* the following line:
*
* add_filter( 'jetpack_implode_frontend_css', '__return_false' );
*
* @param bool $travis_test Is this a test run.
*
* @since 3.2
* @since 13.9 Default to not imploding. Requires a filter to enable. This may be temporary before dropping completely.
*/
public function implode_frontend_css( $travis_test = false ) {
$do_implode = false;
if ( defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedIf
// phpcs:ignore Squiz.PHP.CommentedOutCode.Found
// $do_implode = false;
}

// Do not implode CSS when the page loads via the AMP plugin.
if ( class_exists( Jetpack_AMP_Support::class ) && Jetpack_AMP_Support::is_amp_request() ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedIf
// phpcs:ignore Squiz.PHP.CommentedOutCode.Found
// $do_implode = false;
}

/*
* Only proceed if at least 2 modules with concatenated CSS are active.
* There is no point in serving a big concatenated CSS file
* if there are no features (or only one) that actually need some CSS loaded.
*/
$active_modules = self::get_active_modules();
$modules_with_concatenated_css = $this->modules_with_concatenated_css;
$active_module_with_css_count = count( array_intersect( $active_modules, $modules_with_concatenated_css ) );
if ( $active_module_with_css_count < 2 ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedIf
// phpcs:ignore Squiz.PHP.CommentedOutCode.Found
// $do_implode = false;
}

/**
* Allow CSS to be concatenated into a single jetpack.css file.
*
* @since 3.2.0
*
* @param bool $do_implode Should CSS be concatenated? Default to true.
*/
$do_implode = apply_filters( 'jetpack_implode_frontend_css', $do_implode );

// Do not use the imploded file when default behavior was altered through the filter.
if ( ! $do_implode ) {
return;
}

// We do not want to use the imploded file in dev mode, or if not connected.
if ( ( new Status() )->is_offline_mode() || ! self::is_connection_ready() ) {
if ( ! $travis_test ) {
return;
}
}

// Do not use the imploded file if sharing css was dequeued via the sharing settings screen.
if ( get_option( 'sharedaddy_disable_resources' ) ) {
return;
}

/*
* Now we assume Jetpack is connected and able to serve the single
* file.
*
* In the future there will be a check here to serve the file locally
* or potentially from the Jetpack CDN
*
* For now:
* - Enqueue a single imploded css file
* - Zero out the style_loader_tag for the bundled ones
* - Be happy, drink scotch
*/

add_filter( 'style_loader_tag', array( $this, 'concat_remove_style_loader_tag' ), 10, 2 );

$version = self::is_development_version() ? filemtime( JETPACK__PLUGIN_DIR . 'css/jetpack.css' ) : JETPACK__VERSION;

wp_enqueue_style( 'jetpack_css', plugins_url( 'css/jetpack.css', __FILE__ ), array(), $version );
wp_style_add_data( 'jetpack_css', 'rtl', 'replace' );
}

/**
* Removes styles that are part of concatenated group.
*
* @param string $tag Style tag.
* @param string $handle Style handle.
*
* @return string
*/
public function concat_remove_style_loader_tag( $tag, $handle ) {
if ( in_array( $handle, $this->concatenated_style_handles, true ) ) {
$tag = '';
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
$tag = '<!-- `' . esc_html( $handle ) . "` is included in the concatenated jetpack.css -->\r\n";
}
}

return $tag;
}

/**
* Check the heartbeat data
*
Expand Down
58 changes: 0 additions & 58 deletions projects/plugins/jetpack/tests/php/general/test_class.jetpack.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,64 +200,6 @@ public function test_absolutize_css_urls_properly_handles_use_cases() {
$this->assertEquals( $expected, $result );
}

/**
* @author tonykova
*/
public function test_implode_frontend_css_enqueues_bundle_file_handle() {
global $wp_styles;
$wp_styles = new WP_Styles();

add_filter( 'jetpack_implode_frontend_css', '__return_true' );

if ( ! file_exists( plugins_url( 'jetpack-carousel.css', __FILE__ ) ) ) {
$this->markTestSkipped( 'Required CSS file not found.' );
}

// Enqueue some script on the $to_dequeue list
$style_handle = 'jetpack-carousel';
wp_enqueue_style( 'jetpack-carousel', plugins_url( 'jetpack-carousel.css', __FILE__ ) ); // phpcs:ignore WordPress.WP.EnqueuedResourceParameters.MissingVersion

Jetpack::init()->implode_frontend_css( true );

$seen_bundle = false;
foreach ( $wp_styles->registered as $handle => $handle_obj ) {
if ( $style_handle === $handle ) {
$expected = ( defined( 'WP_DEBUG' ) && WP_DEBUG ) ? "<!-- `{$style_handle}` is included in the concatenated jetpack.css -->\r\n" : '';
$this->assertEquals( $expected, get_echo( array( $wp_styles, 'do_item' ), array( $handle ) ) );
} elseif ( 'jetpack_css' === $handle ) {
$seen_bundle = true;
}
}

$this->assertTrue( $seen_bundle );
}

/**
* @author tonykova
* @since 3.2.0
*/
public function test_implode_frontend_css_does_not_enqueue_bundle_when_disabled_through_filter() {
global $wp_styles;
$wp_styles = new WP_Styles();

add_filter( 'jetpack_implode_frontend_css', '__return_false' );

// Enqueue some script on the $to_dequeue list
wp_enqueue_style( 'jetpack-carousel', plugins_url( 'jetpack-carousel.css', __FILE__ ) ); // phpcs:ignore WordPress.WP.EnqueuedResourceParameters.MissingVersion

Jetpack::init()->implode_frontend_css();

$seen_orig = false;
foreach ( $wp_styles->registered as $handle => $handle_obj ) {
$this->assertNotEquals( 'jetpack_css', $handle );
if ( 'jetpack-carousel' === $handle ) {
$seen_orig = true;
}
}

$this->assertTrue( $seen_orig );
}

public function test_activating_deactivating_modules_fires_actions() {
self::reset_tracking_of_module_activation();

Expand Down
38 changes: 1 addition & 37 deletions projects/plugins/jetpack/tools/webpack.config.css.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,43 +118,7 @@ const entries = {
};

// CSS that needs to have the rtl files renamed using the above RenamerPlugin.
const weirdRtlEntries = {
'css/jetpack': [
// When making changes to that list, you must also update $concatenated_style_handles in class.jetpack.php.
'modules/carousel/swiper-bundle.css',
'modules/carousel/jetpack-carousel.css',
'jetpack_vendor/automattic/jetpack-forms/src/contact-form/css/grunion.css',
'modules/infinite-scroll/infinity.css',
'modules/likes/style.css',
'modules/related-posts/related-posts.css',
'modules/sharedaddy/sharing.css',
'modules/shortcodes/css/slideshow-shortcode.css',
'modules/shortcodes/css/style.css', // TODO: Should be renamed to shortcode-presentations
'modules/shortcodes/css/quiz.css',
'modules/subscriptions/subscriptions.css',
'modules/theme-tools/responsive-videos/responsive-videos.css',
'modules/theme-tools/social-menu/social-menu.css',
'modules/tiled-gallery/tiled-gallery/tiled-gallery.css',
'modules/widgets/wordpress-post-widget/style.css',
'modules/widgets/gravatar-profile.css',
'modules/widgets/goodreads/css/goodreads.css',
'modules/widgets/social-media-icons/style.css',
'modules/widgets/top-posts/style.css',
'modules/widgets/image-widget/style.css',
'modules/widgets/my-community/style.css',
'modules/widgets/authors/style.css',
'modules/wordads/css/style.css',
'modules/widgets/eu-cookie-law/style.css',
'modules/widgets/flickr/style.css',
'modules/widgets/instagram/instagram.css',
'jetpack_vendor/automattic/jetpack-search/src/widgets/css/search-widget-frontend.css',
'modules/widgets/simple-payments/style.css',
'modules/widgets/social-icons/social-icons.css',
'modules/widgets/milestone/milestone-widget.css',
'modules/subscriptions/subscribe-modal/subscribe-modal.css',
'modules/subscriptions/subscribe-overlay/subscribe-overlay.css',
].map( n => path.join( __dirname, '..', n ) ),
};
const weirdRtlEntries = {};

// Non-minified CSS, that also needs to have the rtl files renamed using the above RenamerPlugin.
const weirdRtlNominEntries = {};
Expand Down

0 comments on commit 738e909

Please sign in to comment.