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

wp_get_global_stylesheet: substitute 1-minute transient by non-persistent object cache #3712

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 28 additions & 15 deletions src/wp-includes/global-styles-and-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,34 @@ function wp_get_global_styles( $path = array(), $context = array() ) {
* @return string Stylesheet.
*/
function wp_get_global_stylesheet( $types = array() ) {
// Return cached value if it can be used and exists.
// It's cached by theme to make sure that theme switching clears the cache.
$can_use_cached = (
( empty( $types ) ) &&
( ! defined( 'WP_DEBUG' ) || ! WP_DEBUG ) &&
( ! defined( 'SCRIPT_DEBUG' ) || ! SCRIPT_DEBUG ) &&
( ! defined( 'REST_REQUEST' ) || ! REST_REQUEST ) &&
! is_admin()
);
$transient_name = 'global_styles_' . get_stylesheet();
/*
* Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme
* developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
$can_use_cached = empty( $types ) && ! WP_DEBUG;
felixarntz marked this conversation as resolved.
Show resolved Hide resolved

/*
* By using the 'theme_json' group, this data is marked to be non-persistent across requests.
* @see `wp_cache_add_non_persistent_groups()`.
*
* The rationale for this is to make sure derived data from theme.json
* is always fresh from the potential modifications done via hooks
* that can use dynamic data (modify the stylesheet depending on some option,
* settings depending on user permissions, etc.).
* See some of the existing hooks to modify theme.json behavior:
* @see https://make.wordpress.org/core/2022/10/10/filters-for-theme-json-data/
*
* A different alternative considered was to invalidate the cache upon certain
* events such as options add/update/delete, user meta, etc.
* It was judged not enough, hence this approach.
* @see https://github.com/WordPress/gutenberg/pull/45372
*/
$cache_group = 'theme_json';
$cache_key = 'wp_get_global_stylesheet';
if ( $can_use_cached ) {
$cached = get_transient( $transient_name );
$cached = wp_cache_get( $cache_key, $cache_group );
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
if ( $cached ) {
return $cached;
}
Expand Down Expand Up @@ -150,11 +166,8 @@ function wp_get_global_stylesheet( $types = array() ) {
}

$stylesheet = $styles_variables . $styles_rest;

if ( $can_use_cached ) {
// Cache for a minute.
// This cache doesn't need to be any longer, we only want to avoid spikes on high-traffic sites.
set_transient( $transient_name, $stylesheet, MINUTE_IN_SECONDS );
wp_cache_set( $cache_key, $stylesheet, $cache_group );
}

return $stylesheet;
Expand Down
2 changes: 1 addition & 1 deletion src/wp-includes/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ function wp_start_object_cache() {
)
);

wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) );
wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) );
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
}

$first_init = false;
Expand Down
4 changes: 2 additions & 2 deletions src/wp-includes/ms-blogs.php
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ function switch_to_blog( $new_blog_id, $deprecated = null ) {
);
}

wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) );
wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) );
}
}

Expand Down Expand Up @@ -666,7 +666,7 @@ function restore_current_blog() {
);
}

wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) );
wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) );
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/phpunit/includes/abstract-testcase.php
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ public static function flush_cache() {
)
);

wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) );
wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) );
}

/**
Expand Down
14 changes: 14 additions & 0 deletions tests/phpunit/tests/theme/wpGetGlobalStylesheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -270,4 +270,18 @@ public function test_should_enqueue_stored_styles() {
'Registered styles with handle of "wp-style-engine-my-styles" do not match expected value from the Style Engine store.'
);
}
/**
* Tests that switching themes recalculates the stylesheet.
*
* @ticket 56970
*/
public function test_switching_themes_should_recalculate_stylesheet() {
$stylesheet_for_default_theme = wp_get_global_stylesheet();
switch_theme( 'block-theme' );
oandregal marked this conversation as resolved.
Show resolved Hide resolved
$stylesheet_for_block_theme = wp_get_global_stylesheet();
switch_theme( WP_DEFAULT_THEME );

Comment on lines +280 to +283
Copy link
Contributor

@peterwilsoncc peterwilsoncc Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
switch_theme( 'block-theme' );
$stylesheet_for_block_theme = wp_get_global_stylesheet();
switch_theme( WP_DEFAULT_THEME );
switch_theme( 'block-theme' );
$post_switch_cache = wp_cache_get( 'wp_get_global_stylesheet', 'theme_json' );
$stylesheet_for_block_theme = wp_get_global_stylesheet();
switch_theme( WP_DEFAULT_THEME );
// This needs to be tested after switching back to the default screen, thus the variable.
$this->assertFalse( $post_switch_cache );

$this->assertStringNotContainsString( '--wp--preset--font-size--custom: 100px;', $stylesheet_for_default_theme, 'custom font size (100px) not present for default theme' );
$this->assertStringContainsString( '--wp--preset--font-size--custom: 100px;', $stylesheet_for_block_theme, 'custom font size (100px) is present for block theme' );
}
}