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

Backport: Caching of global styles for blocks from core #66349

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

aaronrobertshaw
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw commented Oct 23, 2024

Backports: WordPress/wordpress-develop#6879

What?

Brings core changes adding transient caching to global styles for blocks to Gutenberg.

Why?

Performance and parity.

How?

Update gutenberg_add_global_styles_for_blocks in line with core's wp_add_global_styles_for_blocks.

Unit tests or lack thereof

The core tests currently rely on the wp_get_development_mode function's ability to fudge the WP_DEVELOPMENT_MODE based on WP_RUN_CORE_TESTS and a global variable that is set. Gutenberg doesn't have this capability which impedes efforts to backport all of core's tests in this area. It likely explains why Gutenberg doesn't have its own test coverage for gutenberg_add_global_styles_for_blocks.

After some digging, it doesn't appear that the time investment required to make these tests all work in Gutenberg is worth it. At best, it could be a follow-up if desired.

Testing Instructions

  1. Confirm updates match Update transient cache for wp_add_global_styles_for_blocks wordpress-develop#6879
  2. Add the snippet below to your theme's functions.php file
Debugging snippet
if ( WP_DEBUG ) {
	add_filter(
		'pre_set_transient_wp_styles_for_blocks',
		function ( $value ) {
			error_log( "Setting 'wp_styles_for_blocks' transient - Hash: " . $value['hash'] );
			return $value;
		}
	);
	add_filter(
		'transient_wp_styles_for_blocks',
		function ( $value ) {
			error_log( "Fetching 'wp_styles_for_blocks' transient - Hash: " . $value['hash'] );
			return $value;
		}
	);
}
  1. Make sure WP_DEBUG and WP_DEBUG_LOG are enabled in your env
  2. Monitor your debug.log file (usually in wp-content/debug.log by default): tail -f wp-content/debug.log
    • If using wp-env, you can set the debug.log location using something like the following in .wp-env.json
     ...
    "config": {
    	"WP_DEBUG_LOG": "/var/www/html/wp-content/debug.log"
    },
    
  3. Visit the frontend of your site. If this is the first time loading the frontend the log should contain a call to fetch and set the transient cache for wp_styles_for_blocks.
  4. Continuing to reload the page should only display a log for fetching the transient
  5. Visit the site editor and adjust a style within global styles.
  6. Reload the frontend again and you should see two logs, one fetching the current transient value and the other setting the updated styles in the cache
  7. Reset global styles in the site editor, refresh the frontend, and confirm correct logs
  8. Smoke test tweaking various global styles e.g. root level, element, block type, block style variations, switching theme variations etc. All should function as expected.
  9. Set WP_DEVELOPMENT_MODE to theme e.g. wp config set WP_DEVELOPMENT_MODE "theme" --allow-root
  10. Repeat the process above. There should be no caching logs, showing the transient isn't used while in theme development mode
Screen.Recording.2024-10-23.at.3.57.32.pm.mp4

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement A suggestion for improvement. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core labels Oct 23, 2024
@aaronrobertshaw aaronrobertshaw self-assigned this Oct 23, 2024
Copy link

github-actions bot commented Oct 23, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: ramonjd <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@aaronrobertshaw aaronrobertshaw changed the title `Backport: Caching of global styles for blocks from core Backport: Caching of global styles for blocks from core Oct 23, 2024
@aaronrobertshaw
Copy link
Contributor Author

I've discussed the difficulties in backporting the unit tests from WordPress/wordpress-develop#6879 with @ramonjd and @andrewserong.

The current thinking is that the time investment in setting up Gutenberg to be able to properly test this functionality isn't worth it with the coverage already in core. If others have better ideas or want to take a run at these tests as a follow-up, that's definitely an option.

For the time being though, the main priority is to get Gutenberg and Core's code in sync. This PR does that so I propose to move forward with it as is.

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

✅ Code matches Core patch
✅ Cache is used when set and get_styles_for_block is not hit
✅ Cache is updated when block styles change
✅ Cache isn't used when WP_DEVELOPMENT_MODE === theme

@aaronrobertshaw aaronrobertshaw merged commit 67fca2e into trunk Oct 24, 2024
78 checks passed
@aaronrobertshaw aaronrobertshaw deleted the backport/caching-of-global-styles-for-blocks branch October 24, 2024 03:53
@github-actions github-actions bot added this to the Gutenberg 19.6 milestone Oct 24, 2024
karthick-murugan pushed a commit to karthick-murugan/gutenberg that referenced this pull request Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants