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

Plugin: viewScript does not work with multiple handles/files #45870

Closed
rufus87 opened this issue Nov 17, 2022 · 7 comments · Fixed by #50079
Closed

Plugin: viewScript does not work with multiple handles/files #45870

rufus87 opened this issue Nov 17, 2022 · 7 comments · Fixed by #50079
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Block API API that allows to express the block paradigm. Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts

Comments

@rufus87
Copy link

rufus87 commented Nov 17, 2022

Description

During block type registration from metadata ( register_block_type_from_metadata function ) there is a great possibility to edit block metadata as it passes threw $metadata = apply_filters( 'block_type_metadata', $metadata ); filter. However there is a callback hooked in block_type_metadata filter in gutenberg plugin which registers all scripts ( handles or files ) but actually returns only the first handle name as the value of viewScript key in metadata array.

Step-by-step reproduction instructions

  1. Add more than one handles as a value of viewScript key in any block.json file:

{ ... "viewScript": ["file:./slider.min.js", "file:./view.min.js"], }

the result should be
[ ... 'viewScript' => ['slider-handle-name', 'view-handle-name'] ]

the actual result is
[ ... 'viewScript' => 'slider-handle-name' ]

View script never contains an array of handle name, even if block.json does.

Screenshots, screen recording, code snippet

image

The above screenshot shows how is it implemented in current 14.5.3 version.

Environment info

  • WP v6.1.1
  • Google Chrome Version 107.0.5304.107 (Official Build) (64-bit)
  • Desktop with Windows 11

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@t-hamano t-hamano added the Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts label Nov 19, 2022
@t-hamano
Copy link
Contributor

Thanks for the report.

I think this is due to the handling of paths in Windows. I recently had a similar problem with a navigation block that has two viewScript, but this was resolved in #45773.

This is included in Gutenberg 14.7 but has not yet been released as a plugin. If you can build the latest trunk, I believe this issue has been resolved.

@rufus87
Copy link
Author

rufus87 commented Nov 19, 2022

@t-hamano , probably your issue is fixed, but the one described in this ticker still is not. Please refer to the screenshot. The trunk branch in the repository right now contain exactly same code as shown in the screenshot. The issue is still actual

@t-hamano
Copy link
Contributor

t-hamano commented Nov 20, 2022

@rufus87
You're right,
When the Gutenberg plugin is enabled, it appears that multiple viewScripts are only available in the core block.

! str_starts_with( $metadata['file'], wp_normalize_path( gutenberg_dir_path() ) )

@aristath
I have found that you were the first to implement this feature in #36176. Should this feature be applied to other than core blocks?

@gziolo gziolo added the [Feature] Block API API that allows to express the block paradigm. label Nov 21, 2022
@aristath
Copy link
Member

Hmmm the implementation was refactored in Core (see https://core.trac.wordpress.org/ticket/56408#no0), but from what I can tell here, the changes were not backported to Gutenberg.
Perhaps this should be moved to the 6.0-compat folder instead of the 6.1? 🤔

@t-hamano
Copy link
Contributor

Thanks for the information, @aristath.

I looked into it, but I couldn't figure out if we should move it to the wordpress-6.0 folder or if we should backport the core changes to Gutenberg 😅

For now, I would like to add the Needs Technical Feedback label.

@t-hamano t-hamano added the Needs Technical Feedback Needs testing from a developer perspective. label Nov 26, 2022
@gziolo gziolo changed the title viewScript does not work with multiple handles/files Plugin: viewScript does not work with multiple handles/files Feb 10, 2023
@gziolo gziolo added the Backwards Compatibility Issues or PRs that impact backwards compatability label Feb 10, 2023
@dd32
Copy link
Member

dd32 commented Feb 21, 2023

This caused some problems on WordPress.org, I suspect either:

  • gutenberg_block_type_metadata_multiple_view_scripts() should be gated to only run on WordPress < 6.1
  • gutenberg_block_type_metadata_multiple_view_scripts() should only run on scripts that are core blocks, as done in gutenberg_block_type_metadata_view_script(). (Submitting PR)

Even if changes were made in core that were not back-ported to Gutenberg, this breakage seems to be clearly a bug in the Gutenberg filter.

@gziolo gziolo added [Status] In Progress Tracking issues with work in progress and removed Needs Technical Feedback Needs testing from a developer perspective. labels Feb 27, 2023
@gziolo
Copy link
Member

gziolo commented Mar 6, 2023

Hmmm the implementation was refactored in Core (see https://core.trac.wordpress.org/ticket/56408#no0), but from what I can tell here, the changes were not backported to Gutenberg.
Perhaps this should be moved to the 6.0-compat folder instead of the 6.1? 🤔

Good catch. I think we should move it to a different folder and prevent it from running in WordPress 6.1 and higher. The idea was that the plugin continues to work when a block defines an array but and older version of WordPress core expects a string.

By the way, we might go also with an alternative approach and set the minimum version of WordPress to 6.1 as it seems to be good timing with WP 6.2 planned for the end of this month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Block API API that allows to express the block paradigm. Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants