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 caching to WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type() #36584

Merged
merged 6 commits into from
Nov 24, 2021

Conversation

xknown
Copy link
Member

@xknown xknown commented Nov 17, 2021

Refactor
WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type
to add caching to the wp_global_styles post type lookup, and also to
remove the duplicate code added in
gutenberg_add_active_global_styles_link.

Fixes #36574

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @xknown! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 17, 2021
@Mamaduka Mamaduka added [Type] Performance Related to performance efforts Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Nov 18, 2021
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, @xknown.

The code looks good to me. Let's rebase on the current trunk and merge this once all checks are green.

…ustom_post_type

Refactor
`WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type`
to add caching to the `wp_global_styles` post type lookup, and also to
remove the duplicate code added in
`gutenberg_add_active_global_styles_link`.

Fixes #36574
@xknown
Copy link
Member Author

xknown commented Nov 24, 2021

Thanks for working on this, @xknown.

The code looks good to me. Let's rebase on the current trunk and merge this once all checks are green.

Thanks for the review/feedback. I rebased the PR branch.

@Mamaduka Mamaduka merged commit 975a3b9 into WordPress:trunk Nov 24, 2021
@Mamaduka
Copy link
Member

Thank you, @xknown.

@github-actions
Copy link

Congratulations on your first merged pull request, @xknown! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 24, 2021
@youknowriad
Copy link
Contributor

Is this something that need to be backported in core for 5.9? cc @oandregal @jorgefilipecosta

noahtallen pushed a commit that referenced this pull request Nov 24, 2021
…ustom_post_type() (#36584)

* Add caching to WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type

Refactor
`WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type`
to add caching to the `wp_global_styles` post type lookup, and also to
remove the duplicate code added in
`gutenberg_add_active_global_styles_link`.

Fixes #36574

* Add unit tests

* Use the correct query filter

* Use wp_cache_set instead of wp_cache_add.

* Remove the filter when finished.

* Simplify the if blocks
@noahtallen
Copy link
Member

A heads up that I cherry-picked this into the gutenberg 12.0.1 point release.

@noisysocks
Copy link
Member

If we don't backport it then it will be deleted when Gutenberg requires WP 5.9 at minimum, no?

@youknowriad
Copy link
Contributor

The changes in lib/compat/wordpress-5.9/ yes. We should be more strict with changes made to lib/compat if we do want to make changes there without back porting, we need to move the changes to lib/compat/wordpress-6.0

@oandregal
Copy link
Member

@youknowriad Without having looked deeply into the code, I'd think it's best to do it, yes: it changes some public method we're merging in 5.9 for the first time. While it is from a "private/internal" class, I'd rather play it safe.

@oandregal oandregal added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 26, 2021
@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 28, 2021
noisysocks pushed a commit that referenced this pull request Nov 29, 2021
…ustom_post_type() (#36584)

* Add caching to WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type

Refactor
`WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type`
to add caching to the `wp_global_styles` post type lookup, and also to
remove the duplicate code added in
`gutenberg_add_active_global_styles_link`.

Fixes #36574

* Add unit tests

* Use the correct query filter

* Use wp_cache_set instead of wp_cache_add.

* Remove the filter when finished.

* Simplify the if blocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_custom_post_type uses uncached SQL queries
6 participants