-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Plugin: Document the project structure and conventions used in the lib
folder for PHP code
#39603
Changes from all commits
476964f
abbfb8b
d43351b
be12599
a32abde
b3fe3f6
dcb2d7e
b2cdd80
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 |
---|---|---|
@@ -0,0 +1,144 @@ | ||
# Gutenberg PHP | ||
|
||
## File structure | ||
|
||
Gutenberg adds features to WordPress Core using PHP hooks and filters. Some | ||
features, once considered stable and useful, are merged into Core during a Core | ||
release. Some features remain in the plugin forever or are removed. | ||
|
||
To make it easier for contributors to know which features need to be merged to | ||
Core and which features can be deleted, Gutenberg uses the following file | ||
structure for its PHP code: | ||
|
||
- `lib/experimental` - Experimental features that exist only in the plugin. They | ||
are not ready to be merged to Core. | ||
- `lib/stable` - Stable features that exist only in the plugin. They could one | ||
day be merged to Core, but not yet. | ||
- `lib/compat/wordpress-X.Y` - Stable features that are intended to be merged to | ||
Core in the future X.Y release, or that were previously merged to Core in the | ||
X.Y release and remain in the plugin for backwards compatibility when running | ||
the plugin on older versions of WordPress. | ||
|
||
## Best practices | ||
|
||
### Prefer the `wp` prefix | ||
|
||
For features that may be merged to Core, it's best to use a `wp_` prefix for functions or a `WP_` prefix for classes. | ||
|
||
This applies to both experimental and stable features. | ||
|
||
Using the `wp_` prefix avoids us having to rename functions and classes from `gutenberg_` to `wp_` if the feature is merged to Core. | ||
|
||
Functions that are intended solely for the plugin, e.g., plugin infrastructure, should use the `gutenberg_` prefix. | ||
|
||
Comment on lines
+24
to
+33
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. Any thoughts on naming conventions for when we're updating a function from core? For example, this change from https://github.com/WordPress/gutenberg/pull/38681/files?w=1#diff-089523f9c993d53f6f02926d28f7a80394d93740e7d16e47e17244547b2b8f5e. The core action is removed and the plugin action is added to replace it. My intention with that change was that the 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 don't think we really have a good strategy for this kind of stuff. We have a similar nightmare with Using |
||
#### Feature that might be merged to Core | ||
|
||
```php | ||
function wp_get_navigation( $slug ) { ... } | ||
``` | ||
|
||
#### Plugin infrastructure that will never be merged to Core | ||
|
||
```php | ||
function gutenberg_get_navigation( $slug ) { ... } | ||
``` | ||
|
||
### Group PHP code by _feature_ | ||
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 actually think the The main goal is to simplify the work for the person back porting. 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. The downside of that though is that you can't move features in and out of "experimental" status e.g. using |
||
|
||
Developers should organize PHP into files or folders by _feature_, not by _component_. | ||
|
||
When defining a function that will be hooked, developers should call `add_action` and `add_filter` immediately after the function declaration. | ||
|
||
These two practices make it easier for PHP code to start in one folder (e.g., `lib/experimental`) and eventually move to another using a simple `git mv`. | ||
|
||
#### Good | ||
|
||
```php | ||
// lib/experimental/navigation.php | ||
|
||
function wp_get_navigation( $slug ) { ... } | ||
|
||
function wp_register_navigation_cpt() { ... } | ||
|
||
add_action( 'init', 'wp_register_navigation_cpt' ); | ||
``` | ||
|
||
#### Not so good | ||
|
||
```php | ||
// lib/experimental/functions.php | ||
|
||
function wp_get_navigation( $slug ) { ... } | ||
|
||
// lib/experimental/post-types.php | ||
|
||
function wp_register_navigation_cpt() { ... } | ||
|
||
// lib/experimental/init.php | ||
add_action( 'init', 'wp_register_navigation_cpt' ); | ||
``` | ||
|
||
### Wrap functions and classes with `! function_exists` and `! class_exists` | ||
|
||
Developers should take care to not define functions and classes that are already defined. | ||
|
||
When writing new functions and classes, it's good practice to use `! function_exists` and `! class_exists`. | ||
|
||
If Core has defined a symbol once and then Gutenberg defines it a second time, fatal errors will occur. | ||
|
||
Wrapping functions and classes avoids such errors if the feature is merged to Core. | ||
|
||
#### Good | ||
|
||
```php | ||
// lib/experimental/navigation/navigation.php | ||
|
||
if ( ! function_exists( 'wp_get_navigation' ) ) { | ||
function wp_get_navigation( $slug ) { ... } | ||
} | ||
|
||
// lib/experimental/navigation/class-wp-navigation.php | ||
|
||
if ( class_exists( 'WP_Navigation' ) ) { | ||
return; | ||
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. Nice, TIL you can call return at root level in a PHP file (i.e. not inside a function) to skip execution 👍 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. PHP is pretty lawless. |
||
} | ||
|
||
class WP_Navigation { ... } | ||
``` | ||
|
||
#### Not so good | ||
|
||
```php | ||
// lib/experimental/navigation/navigation.php | ||
|
||
function wp_get_navigation( $slug ) { ... } | ||
|
||
// lib/experimental/navigation/class-gutenberg-navigation.php | ||
|
||
class WP_Navigation { ... } | ||
``` | ||
|
||
Furthermore, a quick codebase search will also help you know if your new method is unique. | ||
|
||
### Note how your feature should look when merged to Core | ||
|
||
Developers should write a brief note about how their feature should be merged to Core, for example, which Core file or function should be patched. | ||
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. This will help so much with backports to Core ⭐ |
||
|
||
Notes can be included in the doc comment. | ||
|
||
This helps future developers know what to do when merging Gutenberg features into Core. | ||
|
||
#### Good | ||
|
||
```php | ||
/** | ||
* Returns a navigation object for the given slug. | ||
* | ||
* Should live in `wp-includes/navigation.php` when merged to Core. | ||
* | ||
* @param string $slug | ||
* | ||
* @return WP_Navigation | ||
*/ | ||
function wp_get_navigation( $slug ) { ... } | ||
``` |
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.
Not suggesting we do it right away, but do you think it'd be helpful for folks if expanded the compat topic at some stage? For example rationale, preferred naming conventions when extending classes, things to test, and so on.
There are also plenty of lessons to draw from PRs such as #38883 and #38671
Or maybe it's just too fuzzy an area to pin down. 🌪️
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 absolutely. This should be a living document, like all our docs. I think we're still figuring a lot of that out, so it's hard to pen down right now. If you have any suggestions on things we can add now then please go ahead!