Repository navigation
feat: Add support for Ability Deprecation - #13852
justlevine wants to merge 9 commits into
Conversation
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
- Support `true` as a shorthand for the `deprecated` meta filter in `wp_get_abilities()`, so REST API and PHP callers can request only deprecated abilities. - Validate that the deprecation `replacement` detail is a namespaced ability name. - Handle the `deprecated_ability_run` action while serving REST API requests. The notice becomes an `X-WP-DeprecatedAbility` header instead of corrupting the JSON response when `WP_DEBUG` is enabled. - Document the `deprecated` collection parameter details in the REST API schema. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- `meta.deprecated` now takes `null|array` instead of `false|array` - `wp_get_abilities()` hide deprecated abilities by default, a new `allow_deprecated` (or explicit meta filtering) includes them.
e706554 to
575c8f0
Compare
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| $category = isset( $args['category'] ) && is_string( $args['category'] ) ? $args['category'] : ''; | ||
| $namespace = isset( $args['namespace'] ) && is_string( $args['namespace'] ) ? rtrim( $args['namespace'], '/' ) . '/' : ''; | ||
| $meta = isset( $args['meta'] ) && is_array( $args['meta'] ) ? $args['meta'] : array(); | ||
| $include_deprecated = ! empty( $args['include_deprecated'] ) || array_key_exists( 'deprecated', $meta ); |
There was a problem hiding this comment.
Could we support true as a shorthand for any deprecation details here? From the REST API there is currently no way to get only the deprecated abilities without knowing the exact since value, e.g: meta[deprecated]=true returns an empty list. I guess we can do something like:
if ( true === $meta['deprecated'] ) {
$meta['deprecated'] = array();
}| $ability = $this->registered_abilities[ $name ]; | ||
|
|
||
| // Check if the ability is deprecated and handle it accordingly. | ||
| $ability->_handle_ability_deprecation(); |
There was a problem hiding this comment.
The run controller calls wp_get_ability three times per request (permission check, input sanitization and execute), so the deprecated_ability_run action and the REST header handler run three times for a single POST to the run endpoint. Should we only trigger the notice on execute, or also apply the "once per lifecycle" logic to the action and not just to the error?
There was a problem hiding this comment.
(My gut reaction without having the source code open in front of me is to fix the run controller in a separate PR to reuse the same instance. Will look at this with fresh eyes in the morning)
There was a problem hiding this comment.
Could we keep _handle_ability_deprecation() as a private method on the registry instead? The ability instance already exposes all the metadata needed, so the registry can perform the same steps without adding a public helper to WP_Ability. For execution, the registry could wrap execute_callback during registration when the ability is deprecated. This would keep the deprecation handling internal without expanding the ability’s public API.
|
|
||
| foreach ( array( 'since', 'replacement', 'message' ) as $key ) { | ||
| if ( ! array_key_exists( $key, $args['meta']['deprecated'] ) ) { | ||
| if ( 'since' === $key ) { |
There was a problem hiding this comment.
Why is since required? The previous PR accepted an array with just a message, and @wordpress/deprecated also has since as optional. I think a plugin may want to deprecate an ability without committing to a version.
There was a problem hiding this comment.
I don't feel too strongly here, but fwiw the rationale was that in core on the PHP side, _deprecated_*() functions usually require a $version (starting https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/functions.php#L5711 ).
Perhaps you know the backstory why that Gutenberg library deviated from core here? (My gut would have been backcompat for js nullability quirks. I couldn't find anything via trac/github mcp )
I think a plugin may want to deprecate an ability without committing to a version.
This should still be possible, with a 'since' => '', // I don't wanna say for some reason. in your array.
There was a problem hiding this comment.
I also don't feel strongly here it was just something I noticed, feel free to keep since not a blocker form my side.
| } | ||
|
|
||
| foreach ( array( 'since', 'replacement', 'message' ) as $key ) { | ||
| if ( ! array_key_exists( $key, $args['meta']['deprecated'] ) ) { |
There was a problem hiding this comment.
Not a blocker but 'replacement' => null throws here while 'deprecated' => null means not deprecated. Should we use isset so a null detail is treated as not provided?
There was a problem hiding this comment.
Hmm 🤔 I always waffle when it comes to unset vs null
I don't know why in my mind meta.deprecated should be more forgiving than meta.deprecated.replacement 😅
Will update
| * @return mixed|WP_Error The result of the ability execution, or WP_Error on failure. | ||
| */ | ||
| public function execute( $input = null ) { | ||
| // Triggers a deprecation notice if the ability is marked as as deprecated and hasn't been triggered earlier. |
There was a problem hiding this comment.
nitpick: "marked as as deprecated" -> "marked as deprecated"
| * | ||
| * 1. Declarative filters (`category`, `namespace`, `meta`) — per-item, AND logic between | ||
| * arg types. | ||
| * 1. Declarative filters ( `category`, `include_deprecated`, `namespace`, `meta`) - per-item, AND logic between arg |
There was a problem hiding this comment.
nitpick: there is an extra space after the opening parenthesis, and this line uses a hyphen while the other steps keep the em dash.
Trac ticket: https://core.trac.wordpress.org/ticket/64209
Adds a standard way to deprecate an ability without removing it.
Supersedes #10507 (relevant diff)
More specifically:
Adds a
meta.deprecatedproperty (defaultnull). Set it to an array to mark an ability as deprecated:Deprecated abilities are hidden from
wp_get_abilities()by default, but can be included with theincluded_deprecatedarg, or by explicitly filtering fordeprecatedmetadata.Retrieving or executing a deprecated ability will output a deprecation notice. The deprecated ability only outputs one deprecation notice per lifecycle. Deprecation notices on retrieval (only) are suppressed when
include_deprecatedormeta.deprecatedis explicitly filtered by inwp_get_abilities()Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5.5
Used for: Updating the tests after changing from #10507
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.