-
Notifications
You must be signed in to change notification settings - Fork 47
Sync changes from WordPress core after 6.9 beta 1 #126
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
Changes from 8 commits
6b88d37
0c7f621
76f1fad
c03bbc7
550d0ef
5392975
0a511e0
b70eec3
aa88f6a
3f31cb5
a0355d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,47 +6,47 @@ | |
| * | ||
| * @package WordPress | ||
| * @subpackage Abilities_API | ||
| * @since 0.1.0 | ||
| * @since 6.9.0 | ||
| */ | ||
|
|
||
| declare( strict_types = 1 ); | ||
|
|
||
| /** | ||
| * Registers a new ability using Abilities API. | ||
| * | ||
| * Note: Do not use before the {@see 'abilities_api_init'} hook. | ||
| * Note: Should only be used on the {@see 'wp_abilities_api_init'} hook. | ||
| * | ||
| * @since 0.1.0 | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Registry::register() | ||
| * | ||
| * @param string $name The name of the ability. The name must be a string containing a namespace | ||
| * prefix, i.e. `my-plugin/my-ability`. It can only contain lowercase | ||
| * alphanumeric characters, dashes and the forward slash. | ||
| * @param array<string,mixed> $args An associative array of arguments for the ability. This should include | ||
| * `label`, `description`, `category`, `input_schema`, `output_schema`, `execute_callback`, | ||
| * `permission_callback`, `meta`, and `ability_class`. | ||
| * @return ?\WP_Ability An instance of registered ability on success, null on failure. | ||
| * | ||
| * @phpstan-param array{ | ||
| * label?: string, | ||
| * description?: string, | ||
| * category?: string, | ||
| * execute_callback?: callable( mixed $input= ): (mixed|\WP_Error), | ||
| * permission_callback?: callable( mixed $input= ): (bool|\WP_Error), | ||
| * input_schema?: array<string,mixed>, | ||
| * output_schema?: array<string,mixed>, | ||
| * meta?: array{ | ||
| * annotations?: array<string,(bool|string)>, | ||
| * show_in_rest?: bool, | ||
| * ...<string,mixed>, | ||
| * }, | ||
| * ability_class?: class-string<\WP_Ability>, | ||
| * ...<string, mixed> | ||
| * } $args | ||
| * @param string $name The name of the ability. The name must be a string containing a namespace | ||
| * prefix, i.e. `my-plugin/my-ability`. It can only contain lowercase | ||
| * alphanumeric characters, dashes and the forward slash. | ||
| * @param array<string, mixed> $args { | ||
| * An associative array of arguments for the ability. | ||
| * | ||
| * @type string $label The human-readable label for the ability. | ||
| * @type string $description A detailed description of what the ability does. | ||
| * @type string $category The ability category slug this ability belongs to. | ||
| * @type callable $execute_callback A callback function to execute when the ability is invoked. | ||
| * Receives optional mixed input and returns mixed result or WP_Error. | ||
| * @type callable $permission_callback A callback function to check permissions before execution. | ||
| * Receives optional mixed input and returns bool or WP_Error. | ||
| * @type array<string, mixed> $input_schema Optional. JSON Schema definition for the ability's input. | ||
| * @type array<string, mixed> $output_schema Optional. JSON Schema definition for the ability's output. | ||
| * @type array<string, mixed> $meta { | ||
| * Optional. Additional metadata for the ability. | ||
| * | ||
| * @type array<string, bool|null> $annotations Optional. Annotation metadata for the ability. | ||
| * @type bool $show_in_rest Optional. Whether to expose this ability in the REST API. Default false. | ||
| * } | ||
| * @type string $ability_class Optional. Custom class to instantiate instead of WP_Ability. | ||
| * } | ||
| * @return WP_Ability|null An instance of registered ability on success, null on failure. | ||
| */ | ||
| function wp_register_ability( string $name, array $args ): ?WP_Ability { | ||
| if ( ! did_action( 'abilities_api_init' ) ) { | ||
| if ( ! did_action( 'wp_abilities_api_init' ) ) { | ||
| _doing_it_wrong( | ||
| __FUNCTION__, | ||
| sprintf( | ||
|
|
@@ -55,116 +55,206 @@ function wp_register_ability( string $name, array $args ): ?WP_Ability { | |
| '<code>abilities_api_init</code>', | ||
gziolo marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| '<code>' . esc_html( $name ) . '</code>' | ||
| ), | ||
| '0.1.0' | ||
| '6.9.0' | ||
| ); | ||
| return null; | ||
| } | ||
|
|
||
| return WP_Abilities_Registry::get_instance()->register( $name, $args ); | ||
| $registry = WP_Abilities_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return null; | ||
| } | ||
|
|
||
| return $registry->register( $name, $args ); | ||
| } | ||
|
|
||
| /** | ||
| * Unregisters an ability using Abilities API. | ||
| * Unregisters an ability from the Abilities API. | ||
| * | ||
| * @since 0.1.0 | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Registry::unregister() | ||
| * | ||
| * @param string $name The name of the registered ability, with its namespace. | ||
| * @return ?\WP_Ability The unregistered ability instance on success, null on failure. | ||
| * @return WP_Ability|null The unregistered ability instance on success, null on failure. | ||
| */ | ||
| function wp_unregister_ability( string $name ): ?WP_Ability { | ||
| return WP_Abilities_Registry::get_instance()->unregister( $name ); | ||
| $registry = WP_Abilities_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return null; | ||
| } | ||
|
|
||
| return $registry->unregister( $name ); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an ability is registered. | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Registry::is_registered() | ||
| * | ||
| * @param string $name The name of the registered ability, with its namespace. | ||
| * @return bool True if the ability is registered, false otherwise. | ||
| */ | ||
| function wp_has_ability( string $name ): bool { | ||
| $registry = WP_Abilities_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return false; | ||
| } | ||
|
|
||
| return $registry->is_registered( $name ); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves a registered ability using Abilities API. | ||
| * | ||
| * @since 0.1.0 | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Registry::get_registered() | ||
| * | ||
| * @param string $name The name of the registered ability, with its namespace. | ||
| * @return ?\WP_Ability The registered ability instance, or null if it is not registered. | ||
| * @return WP_Ability|null The registered ability instance, or null if it is not registered. | ||
| */ | ||
| function wp_get_ability( string $name ): ?WP_Ability { | ||
| return WP_Abilities_Registry::get_instance()->get_registered( $name ); | ||
| $registry = WP_Abilities_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return null; | ||
| } | ||
|
|
||
| return $registry->get_registered( $name ); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves all registered abilities using Abilities API. | ||
| * | ||
| * @since 0.1.0 | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Registry::get_all_registered() | ||
| * | ||
| * @return \WP_Ability[] The array of registered abilities. | ||
| * @return WP_Ability[] The array of registered abilities. | ||
| */ | ||
| function wp_get_abilities(): array { | ||
| return WP_Abilities_Registry::get_instance()->get_all_registered(); | ||
| $registry = WP_Abilities_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return array(); | ||
| } | ||
|
|
||
| return $registry->get_all_registered(); | ||
| } | ||
|
|
||
| /** | ||
| * Registers a new ability category. | ||
| * | ||
| * @since 0.3.0 | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Category_Registry::register() | ||
| * @see WP_Ability_Categories_Registry::register() | ||
| * | ||
| * @param string $slug The unique slug for the category. Must contain only lowercase | ||
| * alphanumeric characters and dashes. | ||
| * @param array<string,mixed> $args An associative array of arguments for the category. This should | ||
| * include `label`, `description`, and optionally `meta`. | ||
| * @return ?\WP_Ability_Category The registered category instance on success, null on failure. | ||
| * @param string $slug The unique slug for the ability category. Must contain only lowercase | ||
| * alphanumeric characters and dashes. | ||
| * @param array<string, mixed> $args { | ||
| * An associative array of arguments for the ability category. | ||
| * | ||
| * @phpstan-param array{ | ||
| * label: string, | ||
| * description: string, | ||
| * meta?: array<string,mixed>, | ||
| * ...<string, mixed> | ||
| * } $args | ||
| * @type string $label The human-readable label for the ability category. | ||
| * @type string $description A description of the ability category. | ||
| * @type array<string, mixed> $meta Optional. Additional metadata for the ability category. | ||
| * } | ||
| * @return WP_Ability_Category|null The registered ability category instance on success, null on failure. | ||
| */ | ||
| function wp_register_ability_category( string $slug, array $args ): ?WP_Ability_Category { | ||
| return WP_Abilities_Category_Registry::get_instance()->register( $slug, $args ); | ||
| if ( ! did_action( 'wp_abilities_api_categories_init' ) ) { | ||
| _doing_it_wrong( | ||
| __METHOD__, | ||
| sprintf( | ||
| /* translators: 1: abilities_api_categories_init, 2: ability category slug. */ | ||
| __( 'Ability categories must be registered on the %1$s action. The ability category %2$s was not registered.' ), | ||
| '<code>wp_abilities_api_categories_init</code>', | ||
| '<code>' . esc_html( $slug ) . '</code>' | ||
| ), | ||
| '6.9.0' | ||
| ); | ||
| return null; | ||
| } | ||
|
|
||
| $registry = WP_Ability_Categories_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return null; | ||
| } | ||
|
|
||
| return $registry->register( $slug, $args ); | ||
| } | ||
|
|
||
| /** | ||
| * Unregisters an ability category. | ||
| * | ||
| * @since 0.3.0 | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Category_Registry::unregister() | ||
| * @see WP_Ability_Categories_Registry::unregister() | ||
| * | ||
| * @param string $slug The slug of the registered category. | ||
| * @return ?\WP_Ability_Category The unregistered category instance on success, null on failure. | ||
| * @param string $slug The slug of the registered ability category. | ||
| * @return WP_Ability_Category|null The unregistered ability category instance on success, null on failure. | ||
| */ | ||
| function wp_unregister_ability_category( string $slug ): ?WP_Ability_Category { | ||
| return WP_Abilities_Category_Registry::get_instance()->unregister( $slug ); | ||
| $registry = WP_Ability_Categories_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return null; | ||
| } | ||
|
|
||
| return $registry->unregister( $slug ); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an ability category is registered. | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Ability_Categories_Registry::is_registered() | ||
| * | ||
| * @param string $slug The slug of the ability category. | ||
| * @return bool True if the ability category is registered, false otherwise. | ||
| */ | ||
| function wp_has_ability_category( string $slug ): bool { | ||
| $registry = WP_Ability_Categories_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return false; | ||
| } | ||
|
|
||
| return $registry->is_registered( $slug ); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves a registered ability category. | ||
| * | ||
| * @since 0.3.0 | ||
| * @since 6.9.0 | ||
| * | ||
| * @see WP_Abilities_Category_Registry::get_registered() | ||
| * @see WP_Ability_Categories_Registry::get_registered() | ||
| * | ||
| * @param string $slug The slug of the registered category. | ||
| * @return ?\WP_Ability_Category The registered category instance, or null if it is not registered. | ||
| * @param string $slug The slug of the registered ability category. | ||
| * @return WP_Ability_Category|null The registered ability category instance, or null if it is not registered. | ||
| */ | ||
| function wp_get_ability_category( string $slug ): ?WP_Ability_Category { | ||
| return WP_Abilities_Category_Registry::get_instance()->get_registered( $slug ); | ||
| $registry = WP_Ability_Categories_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return null; | ||
| } | ||
|
|
||
| return $registry->get_registered( $slug ); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves all registered ability categories. | ||
| * | ||
| * @since 0.3.0 | ||
| * @since 6.9.0 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On since tags we are using 6.9.0, everywhere, should we be using the plugin version and not the core version?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would keep files the same as in WP core, as we it's a direct copy. In the future, WP core becomes the source of truth and we would only continue keeping this repo up to date so folks can polyfill Abilities API for WP 6.8 and older if they want to start using it but their support policy includes a range of WP major versions. It's been the case for Woo that is actively testing usage of this API. |
||
| * | ||
| * @see WP_Abilities_Category_Registry::get_all_registered() | ||
| * @see WP_Ability_Categories_Registry::get_all_registered() | ||
| * | ||
| * @return \WP_Ability_Category[] The array of registered categories. | ||
| * @return WP_Ability_Category[] The array of registered ability categories. | ||
| */ | ||
| function wp_get_ability_categories(): array { | ||
| return WP_Abilities_Category_Registry::get_instance()->get_all_registered(); | ||
| $registry = WP_Ability_Categories_Registry::get_instance(); | ||
| if ( null === $registry ) { | ||
| return array(); | ||
| } | ||
|
|
||
| return $registry->get_all_registered(); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why we're using
did_action()and notdoing_action? Is an action considered "done" immediately afterdo_action()is triggered? Or after the last hook function is fired?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, https://developer.wordpress.org/reference/functions/do_action/ does two things, increments the counter for how many times the action was fired, and sets the current action name. Then it runs all added actions.
did_actionensures that registration doesn’t happen too early and only when we know the registry is necessary for a given HTTP request, but the point was raised we should not prevent registering more abilities and categories later in the flow, by usingdoing_action.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, thanks for clearing that up. I really should have looked up the code myself. Good to know how that works!
Personally, I'm not convinced on the merits of allowing folks to register at any point after this time, as those are the sort of moments in my WP dev history that are maddening – "wait, why isn't that available? Oh, they hook into that hook later with a priority of 500? Why!?" Not being able to assume that all abilities and categories have been initialized during... initialization is weird. Feels like an established WP quirk that we're perpetuating. 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did provide similar feedback somewhere else (I believe on the Core PR) - I think allowing registration at any arbitrary point means we're being "forgiving" to developers doing it wrong, which can then lead to far more obscure problems down the line.
I think clearly defined APIs and, given WordPress's procedural nature, clearly defined registration timing are crucial to avoid more complex problems. Better tell the developer they should fix something rather than them running into a problem later that they have no idea on why it's occurring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m happy to apply further updates. I was a bit on the clock when landing the entire API in WP core. So while I agreed with reasoning, I missed this important nuance at the time.
It’s a tiny change for production logic, but needs some solid tinkering for unit tests so they are in proper state while registering abilities and ability categories. I’ll try to sort out something tomorrow, so it’s ready before beta 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR should be ready for review:
doing_actioncheck when registering wordpress-develop#10452Code changes as expected are trivial, but getting the unit tests passing was quite a journey.