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

Introduce function to control whether to use persistent cache for certain theme file-based logic #46791

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

felixarntz
Copy link
Member

@felixarntz felixarntz commented Dec 26, 2022

What?

Addresses #45912

Why?

See #45912.

How?

  • Introduces new function to control whether a persistent cache should be used for certain theme file-based logic.
  • This is necessary because those caches cannot be reliably invalidated when direct file changes to the theme are made, which is typically the case during development of the theme. WordPress can cater for file changes made through WP Admin (e.g. through a theme update, or via the theme file editor), but not through other ways.
  • Since Make theme.json object caches non persistent #46150, no persistent caching is used for these functions entirely, however reintroducing persistent caching in a reliable way should still be worked on in the near future.
  • In order to maintain the current behavior, at this point the default is simply false. As noted in the code documentation, neither WP_DEBUG nor the environment type are reliable ways to know whether a theme is currently being developed, so those aren't reliable defaults anyway. The filter introduced by this function is the main benefit here; however as of Make theme.json object caches non persistent #46150 for now enabling persistent cache is discouraged except for experimental nature, as we keep working towards reliable cache invalidation and decoupling hooks from the cached data.

@felixarntz
Copy link
Member Author

@oandregal @spacedmonkey @mmtr Would like to get your review on this.

The primary need for this function is due to the lack of any existing way to provide dedicated control over this. Neither WP_DEBUG nor wp_get_environment_type() reliably indicate whether a theme is currently being developed (and thus whether persistent cache for theme file-based logic should be disabled). We can potentially use one of theme as a somewhat reasonable default, but neither of them can be the only way to do it. The filter that the function comes with is therefore the main benefit added by the PR.

It is less important to add since #46150, however it still provides notable value, particularly as the existing WP_DEBUG checks around the cache usage are inaccurate. Plus they are furthermore incorrect at this point anyway, as we're no longer caching the logic persistently.

@felixarntz felixarntz requested review from mmtr and oandregal December 26, 2022 20:49
@felixarntz
Copy link
Member Author

@oandregal Circling back on this: Since the latest changes in Gutenberg (also for core e.g. WordPress/wordpress-develop#3556) is changing these caches to be non-persistent, the whole idea of having the WP_DEBUG check is not really useful anymore, given that the caches do not last beyond a single request. In other words, theme developers will not be impacted by a non-persistent WP core cache.

What this PR will eventually be useful for is once we can bring back persistent caching (by fixing all the quirks that previously led us to choose the non-persistent cache approach). Once we can use persistent caching without problems, we will need such a way to disable the cache again for theme developers, and only at that point this PR would become relevant really.

Can you please give this a look already (more importantly this comment than the concrete code in here)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant