Skip to content
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

Block Patterns: Automatically load headered files from a theme's /patterns directory #36751

Merged
merged 19 commits into from
Mar 17, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions lib/compat/wordpress-6.0/block-patterns.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,145 @@ function( $current_screen ) {
}
}
);

/**
* Register any patterns that the active theme may provide under its
* `./patterns/` directory. Each pattern is defined as a PHP file and defines
* its metadata using plugin-style headers. The minimum required definition is:
*
* /**
* * Title: My Pattern
* * Slug: my-theme/my-pattern
* *
*
* The output of the PHP source corresponds to the content of the pattern, e.g.:
*
* <main><p><?php echo "Hello"; ?></p></main>
*
* If applicable, this will collect from both parent and child theme.
*
* Other settable fields include:
*
* - Description
* - Viewport Width
* - Categories (comma-separated values)
* - Keywords (comma-separated values)
* - Block Types (comma-separated values)
* - Inserter (yes/no)
*
* @since 6.0.0
* @access private
* @internal
*/
function gutenberg_register_theme_block_patterns() {
$default_headers = array(
'title' => 'Title',
'slug' => 'Slug',
'description' => 'Description',
'viewportWidth' => 'Viewport Width',
'categories' => 'Categories',
'keywords' => 'Keywords',
'blockTypes' => 'Block Types',
'inserter' => 'Inserter',
);

// Register patterns for the active theme. If the theme is a child theme,
// let it override any patterns from the parent theme that shares the same
// slug.
foreach ( wp_get_active_and_valid_themes() as $theme ) {
gziolo marked this conversation as resolved.
Show resolved Hide resolved
$dirpath = $theme . '/patterns/';
if ( file_exists( $dirpath ) ) {
$files = glob( $dirpath . '*.php' );
if ( $files ) {
foreach ( $files as $file ) {
$pattern_data = get_file_data( $file, $default_headers );

if ( empty( $pattern_data['slug'] ) ) {
trigger_error(
sprintf(
/* translators: %s: file name. */
__( 'Could not register file "%s" as a block pattern ("Slug" field missing)', 'gutenberg' ),
$file
)
);
continue;
}

if ( ! preg_match( '/^[A-z0-9\/_-]+$/', $pattern_data['slug'] ) ) {
trigger_error(
sprintf(
/* translators: %1s: file name; %2s: slug value found. */
__( 'Could not register file "%1$s" as a block pattern (invalid slug "%2$s")', 'gutenberg' ),
$file,
$pattern_data['slug']
)
);
}

if ( WP_Block_Patterns_Registry::get_instance()->is_registered( $pattern_data['slug'] ) ) {
continue;
}

// Title is a required property.
if ( ! $pattern_data['title'] ) {
trigger_error(
sprintf(
/* translators: %1s: file name; %2s: slug value found. */
__( 'Could not register file "%s" as a block pattern ("Title" field missing)', 'gutenberg' ),
$file
)
);
continue;
}

// For properties of type array, parse data as comma-separated.
foreach ( array( 'categories', 'keywords', 'blockTypes' ) as $property ) {
if ( ! empty( $pattern_data[ $property ] ) ) {
$pattern_data[ $property ] = array_filter(
preg_split(
'/[\s,]+/',
(string) $pattern_data[ $property ]
)
);
} else {
unset( $pattern_data[ $property ] );
}
}

// Parse properties of type int.
foreach ( array( 'viewportWidth' ) as $property ) {
if ( ! empty( $pattern_data[ $property ] ) ) {
$pattern_data[ $property ] = (int) $pattern_data[ $property ];
} else {
unset( $pattern_data[ $property ] );
}
}

// Parse properties of type bool.
foreach ( array( 'inserter' ) as $property ) {
if ( ! empty( $pattern_data[ $property ] ) ) {
$pattern_data[ $property ] = in_array(
strtolower( $pattern_data[ $property ] ),
array( 'yes', 'true' ),
true
);
} else {
unset( $pattern_data[ $property ] );
}
}

// The actual pattern content is the output of the file.
ob_start();
include $file;
$pattern_data['content'] = ob_get_clean();
if ( ! $pattern_data['content'] ) {
continue;
}

register_block_pattern( $pattern_data['slug'], $pattern_data );
}
}
}
}
}
add_action( 'init', 'gutenberg_register_theme_block_patterns' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registering the patterns on init might mean we're registering them on too many pages, even if we don't need them. Only the block editor UI page (post or site) needs block patterns, is that right? But init runs on all pages, like the site frontend and many other.

Other pattern registration functions run on current_screen and check if $current_screen->is_block_editor() is true. See #39493 where a similar fix was done recently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been tried to be solved above but apparently we need the patters also on frontend, so checking current screen doesn't work here #36751 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, a template can contain a <!-- wp:pattern {"slug":"foo"} /--> markup and then the server will render the pattern's content. But for which patterns does this work reliably? If the pattern is bundled with Gutenberg itself (like core/query-standard-posts) or with the theme, then yes, the pattern block will be rendered.

But what about remote patterns? A theme.json can declare a patterns field that contains merely slugs, not the full patterns. These need to be downloaded from api.wordpress.org. Then there are "core" and "featured" patterns that the block editor UI will dynamically download and register for you.

When the slug refers to such a remote pattern, the wp:pattern block will either be not rendered at all, or the frontend page rendering will be blocked on an api.wordpress.org REST request that the server needs to perform before it can proceed with rendering. And that sounds like something very undesired, doesn't it?

Seems we'll need to draw some sharp line between bundled and remote patterns. One can work with <!-- wp:pattern /--> markup, the other is exclusively a block editor UI thing. Remote patterns are ephemeral and references to them should not be persisted anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the pattern is bundled with Gutenberg itself (like core/query-standard-posts) or with the theme, then yes, the pattern block will be rendered

This is meant for themes essentially as a way to allow i18n for block templates. So the main use-cases is for the patterns registered by themes (like the current PR).

This is a potential decent solution for the problem mentioned here #36751 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a potential decent solution for the problem mentioned here #36751 (comment)

I agree that there's a strong need for the register_block_patterns action. It makes it obvious how to register a pattern. But @mcsf's comment doesn't distinguish between bundled an remote patterns. Registering bundled patterns means executing non-controversial PHP code, but registering remote patterns means remote network requests and blocking until they finish. May be need two register actions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are three kind of remote patterns:

  • The default ones that we want to always load (could potentially be bundled in the plugin itself but they are downloaded form the directory as convenience)
  • The theme ones: themes can also refer to patterns from the directory to mark them as patterns we want to always load.
  • Extra: All the remaining patterns in the directory. For these, we'd need a dedicated UI to search... directly from the directory and these don't need to be registered at all.

I think for 1 and 2, I'm not sure if there should be any difference in behavior with bundled patterns. The directory there just serves as a shortcut to avoid building these in the theme or core itself.

For 3. We don't need any action or any specific treatment, we just load these on demand from the editor.

So I think in the end, only one action should be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally agree:

  • Theme-provided patterns should be available in the front end, both those defined by the theme and those invoked in theme.json.
  • General directory patterns should not be available.
  • Core patterns sourced from the directory: I would say that yes, they should be present, but we should look into caching them properly if that's not the case already. By the looks of gutenberg_register_remote_theme_patterns and WP_REST_Request, it doesn't look like there's any caching, but patterns is the sort of thing we don't need to pull more than once daily, IMO. This caching should also apply to theme.json-invoked patterns.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for 1 and 2, I'm not sure if there should be any difference in behavior with bundled patterns.

Remote patterns, however, are quite different in terms of performance and reliability. When serving a request to render, say, a site homepage, rendering a wp:pattern block may involve a request to api.wordpress.org which may take a long time (seconds) to execute, and can fail. What if api.wordpress.org is down? Currently the pattern block will be silently not rendered at all, and the incorrect result will likely get cached.

When rendering a frontend page, a reasonable expectation is that the server needs to access only reliable "local" resources, like filesystem or the MySQL database?

By the looks of gutenberg_register_remote_theme_patterns and WP_REST_Request, it doesn't look like there's any caching

The pattern directory responses are cached for one hour with set_site_transient. That means we refresh the patterns once an hour, while serving a (random) request. Unless set_site_transient is broken, like it was on WP.com as @tyxla discovered 🙃, then there's no caching.

Ideally, there should be some autoupdate job that would download and sync the patterns to local FS or database, decoupled from serving frontend requests. Then the frontend request processing always works with local resources, and is more consistent and reliable.