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.

Sign up for GitHub

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

Add viewScriptModule handling to block.json metadata #5860

Closed
wants to merge 27 commits into from
Closed

Add viewScriptModule handling to block.json metadata #5860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
6b1035a
Handle scriptModule, module, viewModule
sirreal Dec 27, 2023
ad91ee9
Remove editorModule, module support
sirreal Jan 11, 2024
f94fe16
Use module_id(s) instead of _handle(s)
sirreal Jan 11, 2024
51f8671
Update for changes to module function names
sirreal Jan 23, 2024
0be4744
Update function names and docblocks
sirreal Jan 23, 2024
0f7283d
Fix some "script" references
sirreal Jan 23, 2024
8e4f1a7
Clean up some dead code and lints
sirreal Jan 23, 2024
3e51c6f
Rename to `viewScriptModule`
sirreal Jan 23, 2024
fb36f1f
Add to view_script_module_ids to block type REST API
sirreal Jan 23, 2024
8df5bc0
Add tests
sirreal Jan 23, 2024
da148ad
Test fixes
sirreal Jan 24, 2024
84d8498
lints
sirreal Jan 24, 2024
9705feb
Add more tests
sirreal Jan 24, 2024
876a507
REST tests
sirreal Jan 24, 2024
3712584
Fix typo in REST
sirreal Jan 24, 2024
b9af0e9
Fix tests
sirreal Jan 24, 2024
29edd40
Core viewScriptModule should be suffixed with `-script-module`
sirreal Jan 24, 2024
9ea7f4e
Split and expand core block handle module tests
sirreal Jan 24, 2024
a960f65
Fix since docblock description
sirreal Jan 24, 2024
1a1f4e3
Add since docblock to REST response
sirreal Jan 24, 2024
00fbf39
Update REST property count in tests (again)
sirreal Feb 5, 2024
9da386d
Fix alignment
sirreal Feb 7, 2024
2d3a37f
Make module asset file optional
sirreal Feb 7, 2024
2112074
Fix typo in test
sirreal Feb 7, 2024
fc338e6
Update test for no-asset-file case
sirreal Feb 7, 2024
a225da1
Require module asset file only if it exists
sirreal Feb 8, 2024
d2a5c71
Update documentation for module registration
sirreal Feb 8, 2024
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
109 changes: 102 additions & 7 deletions src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function remove_block_asset_path_prefix( $asset_handle_or_path ) {
*
* @since 5.5.0
* @since 6.1.0 Added `$index` parameter.
* @since 6.5.0 Added support for `viewScriptModule` field.
*
* @param string $block_name Name of the block.
* @param string $field_name Name of the metadata field.
Expand All @@ -52,19 +53,23 @@ function generate_block_asset_handle( $block_name, $field_name, $index = 0 ) {
if ( str_starts_with( $field_name, 'view' ) ) {
$asset_handle .= '-view';
}
if ( str_ends_with( strtolower( $field_name ), 'scriptmodule' ) ) {
$asset_handle .= '-script-module';
}
if ( $index > 0 ) {
$asset_handle .= '-' . ( $index + 1 );
}
return $asset_handle;
}

$field_mappings = array(
'editorScript' => 'editor-script',
'script' => 'script',
'viewScript' => 'view-script',
'editorStyle' => 'editor-style',
'style' => 'style',
'viewStyle' => 'view-style',
'editorScript' => 'editor-script',
'editorStyle' => 'editor-style',
'script' => 'script',
'style' => 'style',
'viewScript' => 'view-script',
'viewScriptModule' => 'view-script-module',
sirreal marked this conversation as resolved.
Show resolved Hide resolved
'viewStyle' => 'view-style',
);
$asset_handle = str_replace( '/', '-', $block_name ) .
'-' . $field_mappings[ $field_name ];
Expand Down Expand Up @@ -122,6 +127,62 @@ function get_block_asset_url( $path ) {
return plugins_url( basename( $path ), $path );
}

/**
* Finds a script module ID for the selected block metadata field. It detects
* when a path to file was provided and optionally finds a corresponding asset
* file with details necessary to register the script module under with an
* automatically generated module ID. It returns unprocessed script module
* ID otherwise.
*
* @since 6.5.0
*
* @param array $metadata Block metadata.
* @param string $field_name Field name to pick from metadata.
* @param int $index Optional. Index of the script module ID to register when multiple
* items passed. Default 0.
* @return string|false Script module ID or false on failure.
*/
function register_block_script_module_id( $metadata, $field_name, $index = 0 ) {
if ( empty( $metadata[ $field_name ] ) ) {
return false;
}

$module_id = $metadata[ $field_name ];
if ( is_array( $module_id ) ) {
if ( empty( $module_id[ $index ] ) ) {
return false;
}
$module_id = $module_id[ $index ];
}

$module_path = remove_block_asset_path_prefix( $module_id );
if ( $module_id === $module_path ) {
return $module_id;
}

$path = dirname( $metadata['file'] );
$module_asset_raw_path = $path . '/' . substr_replace( $module_path, '.asset.php', - strlen( '.js' ) );
$module_id = generate_block_asset_handle( $metadata['name'], $field_name, $index );
$module_asset_path = wp_normalize_path(
realpath( $module_asset_raw_path )
);

$module_path_norm = wp_normalize_path( realpath( $path . '/' . $module_path ) );
$module_uri = get_block_asset_url( $module_path_norm );

$module_asset = ! empty( $module_asset_path ) ? require $module_asset_path : array();
$module_dependencies = isset( $module_asset['dependencies'] ) ? $module_asset['dependencies'] : array();

wp_register_script_module(
$module_id,
$module_uri,
$module_dependencies,
isset( $module_asset['version'] ) ? $module_asset['version'] : false
);

return $module_id;
}

/**
* Finds a script handle for the selected block metadata field. It detects
* when a path to file was provided and optionally finds a corresponding asset
Expand Down Expand Up @@ -314,7 +375,7 @@ function get_block_metadata_i18n_schema() {
* @since 6.1.0 Added support for `render` field.
* @since 6.3.0 Added `selectors` field.
* @since 6.4.0 Added support for `blockHooks` field.
* @since 6.5.0 Added support for `allowedBlocks` and `viewStyle` fields.
* @since 6.5.0 Added support for `allowedBlocks`, `viewScriptModule`, and `viewStyle` fields.
*
* @param string $file_or_folder Path to the JSON file with metadata definition for
* the block or path to the folder where the `block.json` file is located.
Expand Down Expand Up @@ -490,6 +551,40 @@ function register_block_type_from_metadata( $file_or_folder, $args = array() ) {
}
}

$module_fields = array(
'viewScriptModule' => 'view_script_module_ids',
);
foreach ( $module_fields as $metadata_field_name => $settings_field_name ) {
if ( ! empty( $settings[ $metadata_field_name ] ) ) {
$metadata[ $metadata_field_name ] = $settings[ $metadata_field_name ];
}
if ( ! empty( $metadata[ $metadata_field_name ] ) ) {
$modules = $metadata[ $metadata_field_name ];
$processed_modules = array();
if ( is_array( $modules ) ) {
for ( $index = 0; $index < count( $modules ); $index++ ) {
$result = register_block_script_module_id(
$metadata,
$metadata_field_name,
$index
);
if ( $result ) {
$processed_modules[] = $result;
}
}
} else {
$result = register_block_script_module_id(
$metadata,
$metadata_field_name
);
if ( $result ) {
$processed_modules[] = $result;
}
}
$settings[ $settings_field_name ] = $processed_modules;
}
}

$style_fields = array(
'editorStyle' => 'editor_style_handles',
'style' => 'style_handles',
Expand Down
8 changes: 8 additions & 0 deletions src/wp-includes/class-wp-block-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,14 @@ class WP_Block_Type {
*/
public $view_script_handles = array();

/**
* Block type front end only script module IDs.
*
* @since 6.5.0
* @var string[]
*/
public $view_script_module_ids = array();

/**
* Block type editor only style handles.
*
Expand Down
6 changes: 6 additions & 0 deletions src/wp-includes/class-wp-block.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,12 @@ public function render( $options = array() ) {
}
}

if ( ! empty( $this->block_type->view_script_module_ids ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering whether it would fit better after all filters get applied like render_block. This way, plugin authors still would have the opportunity to change the list of module IDs on the block.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean that authors could change the block or module dependencies in the render_block filter function?

I don't have strong feelings on whether or not it should be moved after the render filters. I do know that changing the module graph is error prone and not something that should be done in most cases.

Copy link
Member

Choose a reason for hiding this comment

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

Extenders can add/remove/replace what is inside $this->block_type->view_script_module_ids with filters like render_block, so maybe it's worth accounting for it. Anyway, the same issue exists today with view scripts, so maybe we need to live with it for conistency.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm happy to defer to your judgment. My instinct was to align with how scripts work but we do not strictly need to do if we can improve things.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can check if differs after filters and do a _doing_it_wrong() with the message of Changing the module graph is error prone

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong opinion about this but from my experience trying to change supports.interactivity dynamically, I don't think mutating the Block Type at render time should be encouraged. The way I see it now is that the Block Type is meant to represent the properties of all the blocks of that type, not just the one being rendered. Changing it on the fly to accommodate the block that is being rendered is confusing and leads to complexity. So, I think that the differences between the rendered blocks of the same type should be reflected by other means.

Copy link
Member

Choose a reason for hiding this comment

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

Do you anticipate introducing any conditional loading for view script modules depending on the HTML rendered for the block? Let's say enqueueing only these registered modules for the block that has corresponding directives in the final HTML. That would be a more compelling reason to move the logic after block render filters.

foreach ( $this->block_type->view_script_module_ids as $view_script_module_id ) {
wp_enqueue_script_module( $view_script_module_id );
}
}

if ( ( ! empty( $this->block_type->style_handles ) ) ) {
foreach ( $this->block_type->style_handles as $style_handle ) {
wp_enqueue_style( $style_handle );
Expand Down
12 changes: 12 additions & 0 deletions src/wp-includes/rest-api/endpoints/class-wp-rest-block-types-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ public function get_item( $request ) {
* @since 5.5.0
* @since 5.9.0 Renamed `$block_type` to `$item` to match parent class for PHP 8 named parameter support.
* @since 6.3.0 Added `selectors` field.
* @since 6.5.0 Added `view_script_module_ids` field.
*
* @param WP_Block_Type $item Block type data.
* @param WP_REST_Request $request Full details about the request.
Expand Down Expand Up @@ -291,6 +292,7 @@ public function prepare_item_for_response( $item, $request ) {
'editor_script_handles',
'script_handles',
'view_script_handles',
'view_script_module_ids',
'editor_style_handles',
'style_handles',
'view_style_handles',
Expand Down Expand Up @@ -584,6 +586,16 @@ public function get_item_schema() {
'context' => array( 'embed', 'view', 'edit' ),
'readonly' => true,
),
'view_script_module_ids' => array(
'description' => __( 'Public facing script module IDs.' ),
'type' => array( 'array' ),
'default' => array(),
'items' => array(
'type' => 'string',
),
'context' => array( 'embed', 'view', 'edit' ),
'readonly' => true,
),
'editor_style_handles' => array(
'description' => __( 'Editor style handles.' ),
'type' => array( 'array' ),
Expand Down
1 change: 1 addition & 0 deletions tests/phpunit/data/blocks/notice/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"editorScript": "tests-notice-editor-script",
"script": "tests-notice-script",
"viewScript": [ "tests-notice-view-script", "tests-notice-view-script-2" ],
"viewScriptModule": [ "tests-notice-view-script-module", "tests-notice-view-script-module-2" ],
"editorStyle": "tests-notice-editor-style",
"style": [ "tests-notice-style", "tests-notice-style-2" ],
"viewStyle": [ "tests-notice-view-style" ],
Expand Down
Loading
Loading