Block supports: refactor how the block to render is tracked to improve core/plugin compatibility #26356
Conversation
oandregal
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
|
Size Change: 0 B Total Size: 1.2 MB ℹ️ View Unchanged
|
ZebulanStanphill
added
the
[Type] Code Quality
|
Looks good to me, wdyt @mcsf |
oandregal
force-pushed
the
update/block-to-render-mechanism
branch
from
71d53f0 to
7dffcdd
Compare
| add_action( 'init', array( 'WP_Block_Supports', 'init' ), 22 ); | ||
| add_filter( 'register_block_type_args', 'wp_block_supports_track_block_to_render' ); |
There was a problem hiding this comment.
Do you think that filter is better where it was in compat.php because it's specifically about that right? compatibility with WP versions that don't support that variable yet.
There was a problem hiding this comment.
Yup:
- if the plugin is active on a WP version that doesn't have this class => this will be loaded.
- if the plugin is active on a WP version that does have the class => this won't be loaded.
There was a problem hiding this comment.
oh, but do we need to signal anywhere that this class shouldn't be loaded when the plugin requires WP 5.6? I'm not sure how that works.
There was a problem hiding this comment.
one thing we could do would be to move this code from load.php:
if ( ! class_exists( 'WP_Block_Supports' ) ) {
require_once dirname( __FILE__ ) . '/class-wp-block-supports.php';
}to compat.php (and add the comment about WP 5.6).
What do you think?
There was a problem hiding this comment.
One of the goals is to only set up the register_block_type_args filter when the WP_Block_Supports class loaded in the runtime is the one from the plugin and not core, correct? If so, what if the plugin's version of the class provided its own wp_block_supports_track_block_to_render as a static method? That way, we would have:
- Conditional class loading in
load.php, as is the case for all other classes - In
compat.php, something like:
if ( class_exists( 'WP_Block_Supports' ) && method_exists( 'WP_Block_Supports', 'track_block_to_render' ) ) {
add_filter( 'register_block_type_args', array( 'WP_Block_Support', 'track_block_to_render' ) );
}Or am I missing something?
The advantages are that we preserve normal loading, and avoid any arbitrary signals to instead use duck-typing (i.e. merely requiring the method to exist in order to register it as a filter).
There was a problem hiding this comment.
One of the goals is to only set up the register_block_type_args filter when the WP_Block_Supports class loaded in the runtime is the one from the plugin and not core, correct?
I would think this PR should also be ported to core for 5.6 as it is. Following that rationale my answer would be no.
Sure, there's the edge case of WP 5.6 beta 1, which already has a class different to this one. I'd argue that's a short-lived version for testing purposes, though. Anyway, the issue with inner blocks will be fixed anyway (because it's already fixed in core beta 1) and we don't need the current code in compat.php.
There was a problem hiding this comment.
Should we go ahead and merge this, then? I've already ported the changes to WordPress/wordpress-develop#640 Would benefit from some orientation as to how to land that one.
|
I see a couple of e2e test that are failing, but I see (or can repro) the same failures on master. I'm going to go ahead and merge this one. |
oandregal
changed the title
|
I started getting PHP errors today, and after tracking things down with The PHP error I'm getting with an FSE theme now: To replicate:
EDIT: Issue gets fixed after copying the |
|
@aristath Thanks for reporting! I prepared WordPress/wordpress-develop#640 to fix this. Not sure how to move the related core patch forward, but it's ready for review/commit. |
|
@nosolosw This PR seems to have introduced the following failing e2e test: It looks like the Latest Posts block no longer renders with the |
|
Dan, there's WordPress/wordpress-develop#640 that will fix the issue. Riad is now merging into core so it can be part of Beta 2. |
* Fix drop zone indicators for non blocks (#25986) * Remove isDraggingBlocks check * Clean up drag and drop data if the user presses escape when dragging * Add comment to ensure avoidance of dragend * Switch to using dragend * Fix archives block render function (#26309) * Code block: preserve indentation on paste (#26347) * Turn off autocomplete for token input. (#26427) * Fix parent post selector: ensure initial value available, search performed, all results shown. (#26397) Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com> * [RNMobile] Fix CI issues during `npm ci` (#26455) * Run mobile tests on master * Debug CI * Revert "Debug CI" This reverts commit dec1ebe. * Update react-native-screens ref, pin @react-navigation/native version * Pin @react-navigation/core,router versions * Fix composer test failures due to invalid lock (#26472) * Fix gallery block undo issue (#26377) * Mark change in attributes when gallery mounts as not persistant * Fix typo * Fix package lock changes * Fix some Twenty Twenty One related e2e test failures (#26341) * Account for unknown number of controls in block inspector * Ensure selection is on-screen by using a group block instead of cover, which takes up less space on screen * Fix typewriter test * Use correct name for patterns test * Revert change to .wp-env.json * Fix multi-entity side editor saving test * Remove blank line * insert paragraph instead of tempalte part (#26371) * Fix autosave e2e tests (#26416) * Update how block to render is tracked (#26356) Co-authored-by: Daniel Richards <daniel.richards@automattic.com> Co-authored-by: Riad Benguella <benguella@gmail.com> Co-authored-by: Ella van Durpe <wp@iseulde.com> Co-authored-by: Adam Silverstein <adamsilverstein@earthboundhosting.com> Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com> Co-authored-by: Ceyhun Ozugur <ceyhunozugur@gmail.com> Co-authored-by: Marcus Kazmierczak <marcus@mkaz.com> Co-authored-by: Addison Stavlo <Stavz01@gmail.com> Co-authored-by: O André <nosolosw@users.noreply.github.com>
See WordPress/wordpress-develop#640
This PR removes the global
$current_block_parsed(that was used to track which block is in to be rendered) in favor of making it a static property of the WP_Block_Supports class.