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

Delete only theme-related caches when saving changes #685

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Jul 6, 2024

Related to #655, #654

This PR changes how the cache is deleted when you save changes to a theme:

  • Delete the database cache as well as the object cache
  • Delete only the theme cache, not all caches

Delete the database cache as well as the object cache

As I understand it, the wp_cache_flush() function deletes only the object cache. On the other hand, the pattern uses the database cache via the Transient API (Source).

Therefore, I think that wp_cache_flush() cannot completely solve the problem reported in #655.

In my testing in the local wp-env environment, the content did not appear in the editor canvas after saving a modified template unless I deleted the database cache or changed the value of the DEVELOPMENT_MODE constant to theme.

Delete only the theme cache, not all caches

wp_cache_flush() function deletes all object caches. Therefore, it may have unintended negative effects on sites that are trying to improve performance by using object caches.

WP_Theme (wp_get_theme())->cache_delete() method properly deletes the object cache for the theme and the database cache for the pattern (Source).

Testing Instructions

First of all, I would be grateful if you could confirm whether the problem reported in #655 still occurs in the wp-env environment and test whether this PR solves the problem.

If this PR is useful, we may evaluate the effectiveness of the wp_cache_flush() function used in other places individually and replace it with a different cache deletion method similar to this PR.

The video below shows that the issue reported in #655 still occurs in the wp-env environment.

a164276e19a6881b67d82c56cfc5bb8b.mp4

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

Thanks @t-hamano! Thanks also for your explanation of the wp_cache_flush() function, I didn't realise this may have unintended negative effects. wp_get_theme()->cache_delete() certainly sounds more appropriate in this case.

I followed the testing steps from #655 using a Pressable instance, and this PR works well. It fixes the issue and I didn't see any errors ✅

@t-hamano
Copy link
Contributor Author

t-hamano commented Aug 1, 2024

@mikachan Thanks for the review!

@t-hamano t-hamano merged commit a8b2e52 into trunk Aug 1, 2024
2 checks passed
@t-hamano t-hamano deleted the delete-theme-cache branch August 1, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants