Plugin: Seek ways to improve the process of backporting PHP code to WP core

[gziolo]

Context

WordPress 5.8 ‘Tatum’ Retrospective.

Gutenberg plugin historically offered the way to develop new features for WordPress core. For the PHP part, we can identify the following role of the code shipped in the plugin:

• experimental features in the active development that might or might not end up in WordPress core
• stable features that are ready to be included in the next WordPress major release
• compatibility layer for previous version of WordPress that the Gutenberg plugin still supports: shims for missing functions, classes, and hooks to bring the same experience in every supported WP version

The most popular strategy for the minimum version of WordPress core supported is the current and the previous version of WordPress. Now, it would be WordPress 5.8 and 5.7. In practice, we still keep logic that bridges WordPress 5.6, but it should change soon.

What problem does this address?

The process of backporting PHP code from the Gutenberg plugin to WordPress core is the most significant pain point at the moment. It needs substantial refinements. The current structure of the Gutenberg plugin makes it hard to locate the changes necessary to bring to WordPress core together with related JavaScript logic. Before anything else, we should make it more transparent in the plugin what's already in WordPress core, what's ready to be backported, and what's still an experiment.

In addition to the challenges with identifying PHP changes that should be moved to the WordPress core from the plugin, there were other related issues:

• the same functions/classes used in the plugin and WordPress core weren't guarded correctly in the plugin triggering redeclaration errors
• the name of the functions/methods were too general in the context of WP core – it's something that could be caught earlier in the PR review process

Relevant feedback (full comment) shared on the post by @youknowriad – the editor release lead for WordPress 5.8:

On the Gutenberg side, there’s a lot of work that is done before PRs get merged, reviews, discussions… with the objective of a feature being ready for Core once it’s made stable without big changes. That said, when back porting these PRs into Core, we sometimes receive late feedback about these features as a lot of people only care about Core commits and ignore any activity in Gutenberg itself (understandable as there’s a lot of activity there). That feedback is great but is share a bit late in the process. Often times, the person doing the backport don’t know anything about the backported changes creating blockers and a lot of unnecessary back and forth.

Some initial ideas

Relevant feedback (full comment) shared on the post by @youknowriad – the editor release lead for WordPress 5.8:

Ideally, the backporting should be automatic, meaning at that moment, everything is done and we should require more changes, this ideal means two things: Align practices more and more between core and Gutenberg and encouraging folks to monitor commits in Gutenberg the same way they monitor commits in Core and not wait until things get in Core.

In the WordPress 5.8 release cycle, we experimented with the lib/compat subfolder that contains code necessary to run features included in the 5.8 release in older versions of WordPress. The idea would be that we can safely delete the folder as soon as WordPress 5.8 becomes the minimum supported version in the Gutenberg plugin.

In the future, we could take it to another level:

• Move feature development to lib/compat/experimental to have one place to seek code that is still not ready to integrate with WordPress core. We could introduce a convention that all class names and function names would have to be prefixed with the gutenberg prefix to better distinguish their current role.
• Introduce lib/compat/stable to keep code that is considered ready to move to WP core, it would introduce a formal step where code would get moved from the experiments folder. One of the benefits would be a simple option to integrate with GitHub CODEOWNERS notification system. The review process could be optimized towards common pain points like:
  • ensuring that the names of functions/classes/hooks are updated to meet WP core guidelines – gutenberg prefix removed or replaced with something else
  • functions/classes would be conditionally declared to account for their future presence in WP core
• Move code backported to WordPress core to the subfolder for the currently planned major versions of WordPress. In the WP 5.9 release cycle, we would move stable features from lib/compat/stable to lib/compat/wordpress-5/9 as soon as the same code gets added to trunk in the core.

[azaozz]

All of the ideas sound great! 👍

I'd like to chime in with couple more things that would improve the "situation" in the future:

1. Add some "bootstrap" code that will prevent loading of the Gutenberg plugin in unsupported versions of WordPress. Currently there's nothing stopping a user to install any Gutenberg version on any Core version. In these cases it would be best UX to not load the plugin but instead show a warning with details about the incompatibility. That would prevent potential PHP fatals, js errors/WSODs, or broken content.
2. Turn plugin auto-updates on by default, perhaps only for new installs.
3. Add an "early warning" system to periodically remind the users to update the plugin when auto-updates are off and there's a new version. Make these warnings really persistent when Gutenberg runs in older WP, and really really annoying couple of weeks before the plugin stops loading (the version check in the bootstrap kicks in).

[tellthemachines]

Align practices more and more between core and Gutenberg and encouraging folks to monitor commits in Gutenberg the same way they monitor commits in Core and not wait until things get in Core.

I'm curious about what aligning practices might look like, and how we can make it easier to monitor commits in Gutenberg, given the volume of work in this repo is much higher than in Core.

An alternative would be to update Core more regularly with new features, as they become close to stable in Gutenberg. This will allow folks running trunk to try new stuff out and give us early feedback, and potentially means folks wouldn't need to keep such a close eye on Gutenberg in order to anticipate the next changes to Core. This may not always be easy to do for major features, as we're often working on stabilising them right up until feature freeze, but if we manage to at least do it for smaller changes, that will reduce the overhead in the lead up to a release. We could update Core with whatever is stable after each plugin release, for example.

Subdividing the lib folder so it's clearer what code is plugin-specific, what is for back-compat only, and what is in development for future inclusion in Core is a great idea! My question is, should it be lib/compat/experimental or lib/experimental, assuming that what's experimental is not (yet) being kept for compat reasons?

[gziolo]

An alternative would be to update Core more regularly with new features, as they become close to stable in Gutenberg.

The main reason we don't update more often is that it's very difficult to identify which PHP changes were added since the last WP core backporting round.

My question is, should it be lib/compat/experimental or lib/experimental, assuming that what's experimental is not (yet) being kept for compat reasons?

Good point. It should be enough to have lib/experimental 😄

1. Add some "bootstrap" code that will prevent loading of the Gutenberg plugin in unsupported versions of WordPress. Currently there's nothing stopping a user to install any Gutenberg version on any Core version. In these cases it would be best UX to not load the plugin but instead show a warning with details about the incompatibility. That would prevent potential PHP fatals, js errors/WSODs, or broken content.

We have the following check present in the codebase that bails out early when executed under no longer supported version of WordPress. It should be fully functional after some fixes added a fews ago:

gutenberg/gutenberg.php

Lines 64 to 70 in 115e8c2

	// Compare against major release versions (X.Y) rather than minor (X.Y.Z)
	// unless a minor release is the actual minimum requirement. WordPress reports
	// X.Y for its major releases.
	if ( version_compare( $version, '5.6', '<' ) ) {
	add_action( 'admin_notices', 'gutenberg_wordpress_version_notice' );
	return;
	}

2. Turn plugin auto-updates on by default, perhaps only for new installs.
3. Add an "early warning" system to periodically remind the users to update the plugin when auto-updates are off and there's a new version. Make these warnings really persistent when Gutenberg runs in older WP, and really really annoying couple of weeks before the plugin stops loading (the version check in the bootstrap kicks in).

Both ideas are great 👍🏻

[azaozz]

We have the following check...

Right, that prevents running on old WP versions. Just need to add the same for future WP versions. For example: old WP + old Gutenberg, then WP is updated to latest. It would be best to not load the old Gutenberg but show warning/notice it also needs updating. Perhaps also a bit better warnings. I can start on PRs for these three.

[tellthemachines]

The main reason we don't update more often is that it's very difficult to identify which PHP changes were added since the last WP core backporting round.

That's a bit of a self-fulfilling prophecy though; the less often we update, the harder it is to identify the changes because there are more of them 😅

I seem to recall a script that we could run on the Gutenberg repo to get all the PHP changes since the last release? I think it was @mcsf who pointed me to it. It doesn't do all the work, as not all PHP changes need to be ported to Core and some of them need editing, not to mention figuring out whereabouts in Core the code needs to go (I found that the hardest part when doing the backports for 5.6, because I wasn't super familiar with Core) but helps as a guideline. And if all the plugin-specific PHP is clearly set apart in a different folder, it'll make things easier.

[noisysocks]

This was definitely a pain point for me during 5.8 and I love the proposal @gziolo.

How can we proceed? Bump the minimum supported version to 5.7 and refactor the plugin PHP into lib/compat/5.8 and lib/compat/experimental?

[gziolo]

How can we proceed? Bump the minimum supported version to 5.7 and refactor the plugin PHP into lib/compat/5.8 and lib/compat/experimental?

Yes, we need to start with bumping the minimum WP version to 5.7 and removing all code that exists only to polyfill WP 5.7 and lower. It might require some detective work in places where we don't have good comments in the code.

The next step would be code reorganization so we end up with the following folders:

• lib/compat/5.8 – code that polyfills functionality added to WP 5.8 to make the plugin work when executed with WP 5.7
• lib/compat/5.9 – code that polyfills functionality backported to WP 5.9 to make the plugin work when executed with WP 5.7 or 5.8
• lib/stable – code that is ready to be backported to WP 5.9
• lib/experimental – everything else that is in development

[adamziel]

Great post @gziolo! I like all the ideas you listed. I tried to poke a hole in that and I may have found a problem: We can polyfill new functions and classes, but it's harder to polyfill patches to existing ones, especially when there are no hooks to rely on. We also can't polyfill backwards incompatible changes. We've had quite a few of the former, and there's always a possibility of running into a new one that won't be polyfill-able. I'm not sure how many we've had of the latter.

It could be a show-stopper, but I may be missing something here. If it is, then I think we're left with binding Gutenberg trunk compatibility to only the latest core version (5.8). I'm curious about your thoughts.

[gziolo]

It could be a show-stopper, but I may be missing something here. If it is, then I think we're left with binding Gutenberg trunk compatibility to only the latest core version (5.8). I'm curious about your thoughts.

@adamziel, WordPress 5.8 is out for almost a month so most of the websites that use the latest Gutenberg should already be updated. It's hard to tell how much time it takes until site owners or hosting companies update to the latest major version so we try to be flexible as much as possible so we don't lose the huge group of beta testers by increasing the minimum version of WP too early.

On the technical level, if it's hard to change the functionality in the Gutenberg plugin that is included already in WP core then it's also an issue for extenders. We should do our best to provide all essential hooks at different levels of granularity when designing the functionality for shipping in WordPress core. I'm a bit worried by focusing on the minimum version of WordPress core supported we are making it easier to maintain the Gutenberg plugin but the problem that exists for 3rd party plugins remains but it's less visible.

There is also another issue, when the next release cycle starts, then the Gutenberg plugin has to work with the latest WordPress trunk for the next major version and the current major version of WordPress. It's still two versions where the same issues can be present with polyfilling.

I would be also curious to learn the use cases you struggle with because I'm not aware of any functionality that couldn't be in any way polyfilled in the past.

[adamziel]

@gziolo

we try to be flexible as much as possible so we don't lose the huge group of beta testers by increasing the minimum version of WP too early.

Great point!

I'm a bit worried by focusing on the minimum version of WordPress core supported we are making it easier to maintain the Gutenberg plugin but the problem that exists for 3rd party plugins remains but it's less visible.

I do agree that we would be experiencing some of the same issues as 3rd party plugins creators. At the same time, Gutenberg is special in that we routinely move chunks of PHP core over to WordPress trunk. I am not aware of any 3rd party plugin that shares these specific struggles.

There is also another issue, when the next release cycle starts, then the Gutenberg plugin has to work with the latest WordPress trunk for the next major version and the current major version of WordPress. It's still two versions where the same issues can be present with polyfilling.

Right, this is a big one. Perhaps we could bump the minimum major WordPress requirement to WP trunk right when the release cycle starts – not great, but not terrible. Or maybe a separate release channel (e.g. gutenberg-next) would help. Unfortunately I don't have any better ideas for now.

I would be also curious to learn the use cases you struggle with because I'm not aware of any functionality that couldn't be in any way polyfilled in the past.

Yeah I think you are right – I realized I couldn't come up with a good argument here. Anytime we introduce some new PHP code in the plugin, it polyfills the core by definition and so we only have to retain it:

• A new API endpoint could be polyfilled with the endpoint class we initially used in the plugin.
• A bugfix in existing API endpoint could be polyfilled by replacing the request handler. We could also accept that the older WP version will have a bug.
• The same goes for a CSS change in a theme – we could replace the CSS or just accept that older versions will have more issues.

Anytime we rely on a core patch, it may be hard to polyfill from the plugin. This isn't something we run into frequently. There may be some difficult cases of that, but nothing unsolvable. For example, this wp_set_sidebars_widgets patch is important because of an issue with batch requests to /v2/widgets endpoint, but we could either accept a problem in older versions or "polyfill" that fix by replacing the entire endpoint class with one that uses a workaround.

[gziolo]

For example, this wp_set_sidebars_widgets patch is important because of an issue with batch requests to /v2/widgets endpoint, but we could either accept a problem in older versions or "polyfill" that fix by replacing the entire endpoint class with one that uses a workaround.

It isn't ideal but we have been there in the past. I think we had to polyfill render_block hook (and 2 or 3 classes that is used internally) at some point because we didn't have lower-level hooks at that time. Registering blocks is another example – to keep the development flexibility we unregister them completely before we register them again with the latest version from the plugin.

[noisysocks]

I really like this proposal. I started refactoring the plugin PHP along these lines in a branch called update/refactor-php but it's a big job and I don't have the bandwidth right now to finish it. Feel free to check out that branch and continue working on it, if you'd like.

Something I noticed while working on this is that, for this proposal to be successful, I think we need to switch to grouping PHP code by feature instead of by what part of WordPress is modified. For example, instead of having PHP grouped into init.php, functions.php, blocks.php, etc., it would be better to have full-site-editing.php, widgets.php, template-editor.php, etc. This way, when a feature progresses from experimental to stable, we can simply move the PHP file without making significant code changes. To some extent we already do this but there are a few parts that ideally should be straightened out. (init.php, especially.)

I also wonder how we "enforce" this proposal going forward. Could we set up lint rules? I suspect it's not practical and that we just have to rely on communicating best practices e.g. in lib/README.md. For example: group PHP code by feature, call add_action() and add_filter() near to the function definition, wrap functions with if ( ! function_exists( ... ) ) { ... }, return early in class declarations with if ( class_exists( ... ) ) { return; }.

[gziolo]

@noisysocks, thank you for starting the branch. Do you think we can split the work into multiple PRs? This way it could be less error-prone and we could speed up the review process. One of the tasks could be removing all code that supports WordPress below WordPress 5.7.

The idea of grouping by feature sounds great. It's usually how we promote code from the experimental stage. It could make the process as simple as moving it to a different location and polishing 💯

I don't know how far PHP linting can go but if we could enforce some of the rules you mentioned then it would be awesome.

[adamziel]

@gziolo I think we will have to do gradual migration across many PRs anyway, considering how we keep adding new PHP code every week. We just need a minimal structure to support new changes. Once in place, little bits of migration could opportunistically piggy-back on existing tickets and PRs.

[adamziel]

@gziolo actually, let's talk about API endpoints again. In particular, this case here: WordPress/wordpress-develop#1508 (comment)

tl;dr: we need a way to render a widget preview via the API. We could either introduce a new endpoint, or update one that already exists. I lean towards the latter. How would you do it with polyfills?

I had two ideas, but both failed:

1. There are no hooks to change just that one particular behavior of the endpoint.
2. We could copy that code over to Gutenberg and override an entire rest route via register_rest_route. Now we have a reverse backporting problem: any patch you commit to core you also need to merge to Gutenberg repo. This will be a problem in all instances of "replace a building block from core with one from the plugin". Theoretically we could ignore new patches from core, but then porting updates from Gutenberg to core will require putting a lot of effort into solving conflicts.

Also, that new code relies on WP_REST_Widget_Types_Controller::get_item_permissions_check which means I need to also include that function and its dependency graph in the polyfill. Ideally I could somehow lean on the older polyfill of the entire WP_REST_Widget_Types_Controller endpoint 🤔 .

See my PR attempt: #34230

[gziolo]

Closing after @noisysocks landed #39603 that describes the current revised folder structure and code recommendations.