Editor: Abstract block editor configuration

[gziolo]

We are approaching the day where new screens with the block editor get included in WordPress core. It looks like there are several WordPress hooks defined on the server that depends on $post object that isn’t present on the edit site, edit widgets, or edit navigation screens. It feels like we should deprecate existing filters and create replacements that are going to be context-aware.

Prior art in Gutenberg: ​https://github.com/WordPress/gutenberg/pull/28701.

Required for ​https://github.com/WordPress/gutenberg/pull/29969 - new REST API endpoint for the editor settings.

Changes included so far:

• new method get_default_block_categories were introduced to make it possible to share default block categories
• new method get_allowed_block_types was introduced to handle filters depending on the editor type
• most of the settings that are defined on the client in the block editor package in ​https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/store/defaults.js are already exposed from the new get_default_block_editor_settings method
• another method get_block_editor_settings got introduced to ensure that editor settings can be filtered depending on the editor type
• the logic that preloads common data used with the block editor by processing an array of REST API paths is now abstracted in block_editor_rest_api_preload method

All existing filters that depend on $post object get deprecated with apply_filters_deprecated as suggested by @jeremyfelt:

• allowed_block_types
• block_categories
• block_editor_preload_paths
• block_editor_settings

New filters are introduced that are context-aware:

• allowed_block_types_all
• block_categories_all
• block_editor_rest_api_preload_paths
• block_editor_settings_all

Trac ticket: https://core.trac.wordpress.org/ticket/52920

Ensures that wp-format-library assets are always loaded for the block editor.

[prbot]

​azaozz commented on ​PR #1118:

It feels like we should deprecate existing filters and create replacements that are going to be context-aware.

The changes so far look good imho. Thinking this can go a step further, perhaps, and clearly separate/define the (PHP) filters that run for the actual editor (settings, UI, etc.) and the filters that run for the things around the editor (tags, categories, etc.). Would make it clearer/easier to understand and eventually add/tweak/remove filters in the future.

[prbot]

​gziolo commented on ​PR #1125:

@youknowriad and @ellatrix – do you think it's safe to assume that we always need to load format library?

We do something similar for:

• block directory with wp_enqueue_editor_block_directory_assets
• block styles from WP_Block_Styles_Registry with enqueue_editor_block_styles_assets

[prbot]

​ellatrix commented on ​PR #1125:

You mean for WordPress? I guess so, otherwise it should just have no formats available without errors.

[prbot]

​gziolo commented on ​PR #1125:

You mean for WordPress? I guess so, otherwise it should just have no formats available without errors.

Yes, for WordPress. It's going to be loaded for all pages that render the block editor moving forward. Related commit: ​https://github.com/WordPress/wordpress-develop/commit/0df28171ed9c3c31ebc93add82b64df8f690f87c.

[prbot]

​gziolo commented on ​PR #1118:

I applied several iterations and added tests to cover some of the methods and filters that are marked as deprecated. It still needs more tests and one more abstraction as discussed in ​https://github.com/WordPress/wordpress-develop/pull/1118#discussion_r605516157 but I think it's close to what I would expect. Any thoughts on the current shape? I'd like to start committing parts of the codebase next week if there are no further concerns.

[SergeyBiryukov]

Just linking [50620] for reference, apparently it missed the ticket.

[SergeyBiryukov]

In 50629:

Editor: Consolidate enqueueing block editor assets in wp-includes/default-filters.php.

Follow-up to [44157], [46111], [48537], [50620].

See #52920.

[prbot]

​gziolo commented on ​PR #1118:

A note to myself.

The list of filters covered with unit tests:

• [x] allowed_block_types
• [x] allowed_block_types_{$editor_name}
• [x] block_categories
• [ ] block_editor_preload_paths
• [x] block_editor_settings
• [x] block_categories_{$editor_name}
• [ ] block_editor_preload_paths_{$editor_name}
• [x] block_editor_settings_{$editor_name}

The list of methods covered with unit tests

• [x] get_default_block_categories (indirectly – it's a hardcoded list)
• [x] get_block_categories
• [x] get_allowed_block_types
• [x] get_default_block_editor_settings
• [x] get_block_editor_settings
• [ ] block_editor_preload_api_requests

[prbot]

​nosolosw commented on ​PR #1118:

This looks good and brings clarity about the different editors and settings. Nice work! Trying to understand how to juggle these changes with the plugin and, particularly, with the addition of ​global styles settings to the mix. I'm thinking out loud here, it looks like the next steps are:

• Port the get_default_block_editor_settings as a shim to the plugin, verbatim. You already have this at ​https://github.com/WordPress/gutenberg/pull/30245 This is a shim to be removed as soon as the minimum WordPress version for the plugin is 5.8.
• In the plugin, all new editors start using the new editor-specific filters.
• In the plugin, for the post editor, the plugin hooks into the block_editor_settings and runs the block_editor_settings_post_editor filter. All our code uses the editor-specific filter. When the minimum WordPress version for the plugin is 5.8 we switch to hook into the editor-specific filter instead.
• In subsequent cycles, the new settings to port to WordPress core will be added to either get_default_block_editor_settings (if they're common to all editors) or get_block_editor_settings (if they are only relevant to a specific editor).

Does this sound about right to you?

[prbot]

​gziolo commented on ​PR #1118:

Gutenberg some of the deprecated filters:

1. allowed_block_types
   1. E2E test: ​https://github.com/WordPress/gutenberg/blob/3da717b8d0ac7d7821fc6d0475695ccf3ae2829f/packages/e2e-tests/plugins/allowed-blocks.php#L17-L24
2. block_categories
   1. ​https://github.com/WordPress/gutenberg/blob/12c23261ab469b91f84568f8ed9e6493bff6f187/lib/blocks.php#L358-L377
3. block_editor_preload_paths
   1. ​https://github.com/WordPress/gutenberg/blob/7c134eb66f1e5ed67db646e19234d9edf6e3090e/lib/compat.php#L154-L159
4. block_editor_settings
   1. ​https://github.com/WordPress/gutenberg/blob/2c0ba0758d48a530596f4977c272d3a35906b215/lib/global-styles.php#L171
   2. ​https://github.com/WordPress/gutenberg/blob/7c134eb66f1e5ed67db646e19234d9edf6e3090e/lib/client-assets.php#L673
   3. ​https://github.com/WordPress/gutenberg/blob/7c134eb66f1e5ed67db646e19234d9edf6e3090e/lib/client-assets.php#L690
   4. and more ...

[prbot]

​gziolo commented on ​PR #1118:

@nosolosw, it's a tough problem. I'm still unsure how to handle the deprecation of filters and add more general versions that don't depend on the $post object not present in some contexts. I did some searching in the Gutenberg plugin to better understand how we use those filters today. For example, the usage of the block_categories filter makes me want to have a way to set this category for all screens at once rather than calling it for every screen individually.

[prbot]

​gziolo commented on ​PR #1118:

When WP_DEBUG is set to true, I see a few deprecation warnings when testing with the Gutenberg plugin enabled:

| <b>Deprecated</b>: block_categories is <strong>deprecated</strong> since version 5.8.0! Use block_categories_post-editor instead. in <b>/var/www/src/wp-includes/functions.php</b> on line <b>5242</b><br />

| <b>Deprecated</b>: block_categories is <strong>deprecated</strong> since version 5.8.0! Use block_categories_post-editor instead. in <b>/var/www/src/wp-includes/functions.php</b> on line <b>5242</b><br />

| <b>Deprecated</b>: block_editor_settings is <strong>deprecated</strong> since version 5.8.0! Use block_editor_settings_post-editor instead. in <b>/var/www/src/wp-includes/functions.php</b> on line <b>5242</b><br />

It feels like we should run those deprecated filters in the Gutenberg plugin only when new functions aren't declared because they mostly add the same functionalities.

[gziolo]

In 50776:

Editor: Abstract block editor configuration

There are several WordPress hooks defined on the server that depend on $post object that isn’t present on the new screens like edit site, edit widgets, or edit navigation. This patch deprecates existing filters and introduces replacements that are context-aware.

Props azaozz, andraganescu, jeremyfelt, nosolosw, youknowriad.
See #52920.

[prbot]

​gziolo commented on ​PR #1118:

I merged most of the changes included in this PR with ​https://github.com/WordPress/wordpress-develop/commit/a19f589a5f9d5c1c44b55971fa33ddf6fda653f3. I left out block_editor_preload_api_requests that requires more work.

[gziolo]

In 50777:

Editor: Shape block editor filters to work better with the Gutenberg plugin

This should allow to use new filters in the Gutenberg plugin and therefore it prevents deprecation warnings when in the debug mode.

See #52920.

[jeremyfelt]

From [50777]:

$block_categories = apply_filters( 'block_categories_all', $block_categories, $editor_name );

If the variable filter block_categories_{$editor_name} is not needed, and nothing is changing about the $block_categories parameter, then it may be that block_categories does not need to be deprecated. We can just add $editor_name as a new parameter. Otherwise it seems like the block_categories_all and block_categories filters are effectively the same thing.

[gziolo]

@jeremyfelt, that would be ideal to go with the approach you outlined. We are discussing the same issue in ​https://github.com/WordPress/gutenberg/pull/31027. Let me share the part that worries me the most if we go this route:

block_categories and other filters involved here, they have the $post object as a param which is not set outside of the edit post page. More importantly, we provide examples for this filter that depend on unguarded $post usage at ​https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/filters/block-filters.md#managing-block-categories:

<?php
// my-plugin.php

function my_plugin_block_categories( $categories, $post ) {
        if ( $post->post_type !== 'post' ) {
                return $categories;
        }
        return array_merge(
                $categories,
                array(
                        array(
                                'slug' => 'my-category',
                                'title' => __( 'My category', 'my-plugin' ),
                                'icon'  => 'wordpress',
                        ),
                )
        );
}
add_filter( 'block_categories', 'my_plugin_block_categories', 10, 2 );

If you have some ideas on how to address it safely then we should do it.

[gziolo]

In 50798:

Editor: Fix typo in image default size setting

Related change in Gutneberg: ​https://github.com/WordPress/gutenberg/pull/31324

Props mamaduka.
See #52920.

[chrisvanpatten]

A big concern I have with this change is that while it solves for making the filters more abstract, the lack of $post contextual awareness in the post-editor is a big change, in particular for post-type aware settings.

It's an incredibly common use-case to restrict the available blocks for a given post type in particular; an "events" post type you define may be dramatically different from an "article" type, for instance. If settings are ultimately abstracted and exposed through API endpoints without any way to access post context (as has been proposed in a few Gutenberg repo issues) that represents a massive functionality change and ultimately will lead to breaking a lot of complex needs that we have at the enterprise level.

[gziolo]

@chrisvanpatten, in the discussion you started in ​https://github.com/WordPress/wordpress-develop/pull/1118#discussion_r627396445, @TimothyBlynJacobs suggested we introduce a context class instance instead of the $post that would provide access to the $post. I'm curious what other thing about going that route and providing access to the post properties as if they were present in the context object through overloading. This way we could avoid all the dance with deprecations of existing filters.

[TimothyBlynJacobs]

I wouldn't do the overloading, because while the properties can be made available. It will still break any code that is actually expecting a WP_Post object, either causing a fatal error or unexpected behavior. So I'm not sure we can really get away with not deprecating the filters.

[gziolo]

In 50920:

Editor: Remove editor type specific filters for block editor configuration

Aligns with changes introduced in the Gutenberg plugin. The planned follow-up replace the editor name string with a context object to bring back an optional reference to the actual post.

Props youknowriad.
See #52920.

[prbot]

​gziolo commented on ​PR #1118:

@chrisvanpatten and @TimothyBJacobs – I refactored this code to work with WP_Block_Editor_Context class and covered everything with unit tests. If you agree that this is the path forward then I will apply the same changes with the context class to other functions that are already in trunk. That remaining part should be straightforward.

[prbot]

​chrisvanpatten commented on ​PR #1118:

@gziolo Looks like a good approach for my needs! Thanks for putting the work into this.

[prbot]

​azaozz commented on ​PR #1118:

Nitpick: the preload_paths part of block_editor_preload_paths_all doesn't sound good imho. What is being preloaded from these paths?

I see the new filter name follows the deprecated filter name, but perhaps it can be a bit more helpful/descriptive. Maybe something like block_editor_initial_data or block_editor_preload_data?

[prbot]

​azaozz commented on ​PR #1118:

Seems that names like:

• allowed_block_types_all,
• block_categories_all,
• block_editor_preload_paths_all
• block_editor_settings_all

should probably not be used. The _all part seems misleading.

Quick question: how does the block editor configuration handle multiple instances on the same page? Would filters ending in _all apply to a particular instance or all the instances?

[prbot]

​azaozz commented on ​PR #1118:

Not sure what the advantages are of using WP_Block_Editor_Context class vs. an associative array.

• Why is a class better? What benefits are there for core and for plugins if it is a class and not an associative array?
• Should it be final like WP_Post?
• Should it be restricted in some ways like perhaps make the core data (like $post) private?

I'm not sold on needing a class with all the extra built-in stuff just to pass some data around, about 50/50 for now. If there are benefits please add a sufficient explanation (in the docblock or as inline comments) what these benefits are and how this decision was reached.

[prbot]

​gziolo commented on ​PR #1118:

Nitpick: the preload_paths part of block_editor_preload_paths_all doesn't sound good imho. What is being preloaded from these paths?
I see the new filter name follows the deprecated filter name, but perhaps it can be a bit more helpful/descriptive. Maybe something like block_editor_initial_data or block_editor_preload_data?

It allows providing REST API paths that are going to be preloaded and passed to the client.

How about:

• block_editor_rest_api_preload for the function
• block_editor_rest_api_preload_paths for the filter

Applied in 22f6768faff35013d89feb8ba6804d7d65426992.

[prbot]

​gziolo commented on ​PR #1118:

Seems that names like:

• allowed_block_types_all,
• block_categories_all,
• block_editor_preload_paths_all
• block_editor_settings_all

should probably not be used. The _all part seems misleading.

Yes, it isn't perfect but there is no better alternative proposed so far. The agreement is that we need to deprecate old filters because $post passed as a param when used unguarded in checks will cause fatal errors when null passed for screens when there is no post selected like on the widgets screen.

Quick question: how does the block editor configuration handle multiple instances on the same page? Would filters ending in _all apply to a particular instance or all the instances?

There is no use case for multiple instances of the block editor on one page. It's hard to predict if there is going to be one. We focus on supporting different settings per screen, like the post editor vs widgets screen for now. In the future, there are going to be a navigation editor screen and maybe a site editor screen.

[prbot]

​gziolo commented on ​PR #1118:

Not sure what the advantages are of using WP_Block_Editor_Context class vs. an associative array.

• Why is a class better? What benefits are there for core and for plugins if it is a class and not an associative array?
• Should it be final like WP_Post?
• Should it be restricted in some ways like perhaps make the core data (like $post) private?

I'm not sold on needing a class with all the extra built-in stuff just to pass some data around, about 50/50 for now. If there are benefits please add a sufficient explanation (in the docblock or as inline comments) what these benefits are and how this decision was reached. Such documentation will be really helpful in the future.

I don't feel strongly about one or another. We discussed a class in ​https://github.com/WordPress/wordpress-develop/pull/1118#discussion_r627396445. We will need more data to be passed as context. The work on the widgets screen ​https://github.com/WordPress/wordpress-develop/pull/1284 might help decide what that is going to be. One thing that was raised by @chrisvanpatten is that filters should still offer simple access to the $post object when it's set so plugins/sites could keep their old logic build on top of that. I will include the final keyword. We can change that to an assocative array, too.

[gziolo]

In 50956:

Editor: Extract block_editor_rest_api_preload method for use with different editor screens

It is going to be used on the new widgets editor screen. This patch also introduced a new class WP_Block_Editor_Context that is going to be used with revised block editor filters to let extenders to keep their existing behavior. It should also allow to provide more settings through the context class as new screens get introduced like the navigation editor.

Props azaozz, chrisvanpatten, timothyblynjacobs, youknowriad.
Fixes #52920.

[gziolo]

In 50957:

Editor: Add missing class WP_Block_Editor_Context

Follow-up for [50956].
See #52920.

[prbot]

​gziolo commented on ​PR #1118:

In 1a4e6f2 I update the rest of the methods committed earlier to use the WP_Block_Editor_Context class. I need to apply the same changes in the Gutenberg plugin first.

[gziolo]

In 50963:

Editor: Remove unused param in get_default_block_editor_settings

Follow-up for [50776].

Props nosolosw.
See #52920.

[prbot]

​chrisvanpatten commented on ​PR #1118:

Thanks for the work on this @gziolo!

[gziolo]

In 50983:

Editor: Use the block editor context in filters that used the editor name

Follow-up for [50956].
Props azaozz, chrisvanpatten, timothyblynjacobs, youknowriad.
Fixes #52920.

[prbot]

​gziolo commented on ​PR #1118:

The last batch of changes committed in https://core.trac.wordpress.org/changeset/50983.

[jadpm]

Replying to gziolo:

@jeremyfelt, that would be ideal to go with the approach you outlined. We are discussing the same issue in ​https://github.com/WordPress/gutenberg/pull/31027. Let me share the part that worries me the most if we go this route:

block_categories and other filters involved here, they have the $post object as a param which is not set outside of the edit post page.
...
If you have some ideas on how to address it safely then we should do it.

My 2c on this topic although I know I am a bit late: why no just keep them?

As stated above, there are lost of wild usages of the now deprecated filters that rely precisely on the parameter that is not available on the broad, generic new filters. I would suggest keeping the previous, existing filters, and document that they are only available for the post-editor context. This is not a breaking change at all: right now, they are only available there anyway!

Otherwise, we will end up finding that developers are just repeating the logic now placed under deprecation, but manually: they will hook into block_categories_all, check that the editor name is post-editor, get the current post with get_post and evaluate whether they want to include their block categories on the post editor based on the post properties. I really wonder why we would want to force them to do so when the logic for that already exists.

[gziolo]

@jadpm, thank you for your feedback. Actually, it's very similar to what @chrisvanpatten shared on GitHub in the accompanying PR. When you check [50983], you can see that the current approach is slightly different. All new filters have access to the instance of WP_Block_Editor_Context class that has optional post property. It replaced the string that represented the block editor context that was limiting in its application.

In theory, we could keep old filters, too. My biggest concern is that in the long run, they would cause confusion because we now use the same post editor to edit not only the content of the post but also the template for the individual post. We kept in mind all that I mentioned when opting for the final solution that provides the flexibility that the WP_Block_Editor_Context class ebables.