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

Deprecate suggesting VIP-equivalent cached functions: wpcom_vip_get_page_by_path(), wpcom_vip_term_exists() and wpcom_vip_get_page_by_title() #801

Closed
rebeccahum opened this issue Oct 27, 2023 · 4 comments
Milestone

Comments

@rebeccahum
Copy link
Contributor

rebeccahum commented Oct 27, 2023

What code should not be reported as a violation?

Since the introduction of WP 6.0+, some functions (namely wpcom_vip_get_page_by_path(), wpcom_vip_term_exists() and wpcom_vip_get_page_by_title()) we previously suggested using the VIP-equivalent are now cached:
https://docs.wpvip.com/technical-references/caching/uncached-functions/#h-previously-uncached-wordpress-core-functions.

We should deprecate suggesting the below since it's no longer accurate:

'get_page_by_path' => [
'type' => 'warning',
'message' => '%s() is highly discouraged due to not being cached; please use wpcom_vip_get_page_by_path() instead.',
'functions' => [
'get_page_by_path',
],

'get_page_by_title' => [
'type' => 'error',
'message' => '%s() is prohibited, please use wpcom_vip_get_page_by_title() instead.',
'functions' => [
'get_page_by_title',
],

'term_exists' => [
'type' => 'error',
'message' => '%s() is highly discouraged due to not being cached; please use wpcom_vip_term_exists() instead.',
'functions' => [
'term_exists',
],
],

@rebeccahum rebeccahum changed the title Deprecate suggesting VIP-equivalent cached functions Deprecate suggesting VIP-equivalent cached functions: wpcom_vip_get_page_by_path(), wpcom_vip_term_exists() and wpcom_vip_get_page_by_title() Oct 27, 2023
@GaryJones GaryJones added this to the 3.x milestone Feb 6, 2024
@GaryJones
Copy link
Contributor

term_exists() was handled in #812.

@GaryJones
Copy link
Contributor

GaryJones commented Feb 16, 2024

get_page_by_title() has some interesting changes.

In https://core.trac.wordpress.org/ticket/36905 it was switched to use WP_Query internally, and thus now included caching for WP 6.1.

However, as of https://core.trac.wordpress.org/ticket/57041 and WP 6.2, this change was reverted, and the function became deprecated in favour of WP_Query.

This function deprecation is included in WPCS.

So now, the question becomes - should we:

  • leave wpcom_vip_get_page_by_title() as the recommended solution
  • update the error message for get_page_by_title() to recommend WP_Query (some what duplicating WPCS)
  • remove all references to it and let folks use WPCS to be notified of it? If so, I've opened End restricting usage of get_page_by_title() #814.

GaryJones added a commit that referenced this issue Feb 16, 2024
In WP 6.1.0, `get_page_by_title()` was changed to use `WP_Query` internally`. See core.trac.wordpress.org/ticket/36905. This means that reporting this function as not being cached is not true anymore.

Note that since WP 6.2.0, the function was reverted from using `WP_Query` but was deprecated in favour of using `WP_Query` directly. WordPress Coding Standards highlights this deprecation already.

See #801.
GaryJones added a commit that referenced this issue Feb 16, 2024
In WP 6.1.0, `get_page_by_title()` was changed to use `WP_Query` internally`. See core.trac.wordpress.org/ticket/36905. This means that reporting this function as not being cached is not true anymore.

Note that since WP 6.2.0, the function was reverted from using `WP_Query` but was deprecated in favour of using `WP_Query` directly. WordPress Coding Standards highlights this deprecation already.

See #801.
@GaryJones
Copy link
Contributor

get_page_by_path() did have a reliance on WP_Query, but that was reverted and as this time https://core.trac.wordpress.org/ticket/56689 is still open.

@GaryJones
Copy link
Contributor

As the three functions have been addressed one way or another, this can be closed.

rebeccahum pushed a commit that referenced this issue May 2, 2024
In WP 6.1.0, `get_page_by_title()` was changed to use `WP_Query` internally`. See core.trac.wordpress.org/ticket/36905. This means that reporting this function as not being cached is not true anymore.

Note that since WP 6.2.0, the function was reverted from using `WP_Query` but was deprecated in favour of using `WP_Query` directly. WordPress Coding Standards highlights this deprecation already.

See #801.
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

No branches or pull requests

2 participants