-
Notifications
You must be signed in to change notification settings - Fork 12
Implement server-side registry for Abilities API #3
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
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5f74492
Add server-side registry
gziolo 326dea2
Update tests/unit/WPAbilitiesRegistryTest.php
gziolo 152e263
Update src/class-wp-abilities-registry.php
gziolo 04333ab
Use type shorthand for null consistently
gziolo 2d0fab5
Fix test hints for incorrect usage
gziolo 1975276
Backport changes for PHP 7.2 and coding standards compatibility with …
gziolo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# This file is for unifying the coding style for different editors and IDEs | ||
# editorconfig.org | ||
|
||
# WordPress Coding Standards | ||
# https://make.wordpress.org/core/handbook/coding-standards/ | ||
|
||
root = true | ||
|
||
[*] | ||
charset = utf-8 | ||
end_of_line = lf | ||
insert_final_newline = true | ||
trim_trailing_whitespace = true | ||
indent_style = tab | ||
|
||
[*.yml] | ||
indent_style = space | ||
indent_size = 2 | ||
|
||
[*.md] | ||
trim_trailing_whitespace = false | ||
|
||
[{*.txt,wp-config-sample.php}] | ||
end_of_line = crlf |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<phpunit | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/9.2/phpunit.xsd" | ||
bootstrap="tests/bootstrap.php" | ||
backupGlobals="false" | ||
colors="true" | ||
beStrictAboutTestsThatDoNotTestAnything="true" | ||
beStrictAboutOutputDuringTests="true" | ||
convertErrorsToExceptions="true" | ||
convertWarningsToExceptions="true" | ||
convertNoticesToExceptions="true" | ||
convertDeprecationsToExceptions="true" | ||
> | ||
<testsuites> | ||
<testsuite name="unit"> | ||
<directory suffix="Test.php">tests/unit</directory> | ||
</testsuite> | ||
</testsuites> | ||
</phpunit> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
<?php declare( strict_types = 1 ); | ||
|
||
/** | ||
* Abilities API | ||
* | ||
* Defines functions for managing abilities in WordPress. | ||
* | ||
* @package WordPress | ||
* @subpackage Abilities API | ||
* @since 0.1.0 | ||
*/ | ||
|
||
/** | ||
* Registers a new ability using Abilities API. | ||
* | ||
* Note: Do not use before the {@see 'abilities_api_init'} hook. | ||
* | ||
* @see WP_Abilities_Registry::register() | ||
* | ||
* @since 0.1.0 | ||
* | ||
* @param string|WP_Ability $name The name of the ability, or WP_Ability instance. 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 $properties Optional. An associative array of properties for the ability. This should | ||
* include `label`, `description`, `input_schema`, `output_schema`, | ||
* `execute_callback`, `permission_callback`, and `meta`. | ||
* @return ?WP_Ability An instance of registered ability on success, null on failure. | ||
*/ | ||
function wp_register_ability( $name, array $properties = array() ): ?WP_Ability { | ||
if ( ! did_action( 'abilities_api_init' ) ) { | ||
_doing_it_wrong( | ||
__FUNCTION__, | ||
sprintf( | ||
/* translators: 1: abilities_api_init, 2: string value of the ability name. */ | ||
esc_html__( 'Abilities must be registered on the %1$s action. The ability %2$s was not registered.' ), | ||
'<code>abilities_api_init</code>', | ||
'<code>' . esc_attr( $name ) . '</code>' | ||
), | ||
'0.1.0' | ||
); | ||
return null; | ||
} | ||
|
||
return WP_Abilities_Registry::get_instance()->register( $name, $properties ); | ||
} | ||
|
||
/** | ||
* Unregisters an ability using Abilities API. | ||
* | ||
* @see WP_Abilities_Registry::unregister() | ||
* | ||
* @since 0.1.0 | ||
* | ||
* @param string $name The name of the registered ability, with its namespace. | ||
* @return ?WP_Ability 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 ); | ||
} | ||
|
||
/** | ||
* Retrieves a registered ability using Abilities API. | ||
* | ||
* @see WP_Abilities_Registry::get_registered() | ||
* | ||
* @since 0.1.0 | ||
* | ||
* @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. | ||
*/ | ||
function wp_get_ability( string $name ): ?WP_Ability { | ||
return WP_Abilities_Registry::get_instance()->get_registered( $name ); | ||
} | ||
|
||
/** | ||
* Retrieves all registered abilities using Abilities API. | ||
* | ||
* @see WP_Abilities_Registry::get_all_registered() | ||
* | ||
* @since 0.1.0 | ||
* | ||
* @return WP_Ability[] The array of registered abilities. | ||
*/ | ||
function wp_get_abilities(): array { | ||
return WP_Abilities_Registry::get_instance()->get_all_registered(); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Would recommend using
inc/
instead ofsrc/
. I've got a feeling we're gonna need use some@wordpress/*
packages that need to be built down the road.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.
Initially, I wanted to replicate https://github.com/WordPress/php-ai-client and went with
src
for PHP, but I agree that it's hard to use one pattern because we expect both PHP and JS code in this repo. Gutenberg usespackages
andlib
. Here we also need to take into account a need for a composer package, and an npm package. It isn't so simple to pick right folder names 😅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.
Naming things is always the hardest part 😬
I don’t think GB (JS-first monorepo) or php-ai-client (standalone Composer lib) are quite the right references for a feature (or canonical) plugin.
FWIW, heres what other official WordPress/* projects are doing:
includes
for PHP and scatter JS (assets
, root-levelcss/js
folders,src
)HealthCheck
namespace, build-step assets insrc
.includes
for PHP andassets
for JS/CSS`.src
for PHP but it's legacy and has no js/css.Semantically it seems the general pattern is
includes
(orinc
but that might be more of an a8c thing, havn't seen it inWordPress/*
) = core PHP (no build step)src
are plugin assets that need a build step. OR sometimes they're co-locatedassets
. (OR sometimes non-buildable PHP)packages
reusable (and usually releasable) JS/TS libraries.lib
in the org.tl;dr I think
includes
orinc
is best for clarity (unlikesrc
we know PHP is expected to be there). Then, if/when we need buildable assets for the admin we can see whetherassets
vs buildablesrc
(or a tspackage
, but that seems unlikely in this repo) make sense without being boxed in by semantics.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.
Interesting find is that there are best practices for WP plugins at https://developer.wordpress.org/plugins/plugin-basics/best-practices/#folder-structure. I see
includes
there.src
for JavaScript code makes sense, too. The remaining question is where we locate the composer package and npm package that we will need here allowing early access to server-side registry, REST API controller, and the client-side package with the client-side registry that also integrates the server-side registry through REST API call.Uh oh!
There was an error while loading. Please reload this page.
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 implemented previously
wp-scripts plugin-zip
command and this is what I found there today:https://github.com/WordPress/gutenberg/blob/fc937dccee20e7b0e6dfa671f6afac03f4bf6ef5/packages/scripts/scripts/plugin-zip.js#L32-L46
Let's go with
includes
to have an easy way to integrate that. For JS output, we can use eitherbuild
orpublic
.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 figured out it's best to tackle it in #4 after we land the initial code. This way it's going to be much easier to handle every code change while the CI is running.