Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 23 months ago

#55513 closed enhancement (fixed)

Allow registration of blocks containing scripts / styles that need to get enqueued from within a Theme

Reported by: fabiankaegy's profile fabiankaegy Owned by: gziolo's profile gziolo
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests needs-docs
Focuses: Cc:

Description

Currently, it is not possible to register a block from within a theme if the block contains a script, editorScript, viewScript, etc. because core assumes that a block must exist in a plugin. Which is and should remain the best practice. But there are cases where registering blocks within a theme makes sense and it would be great for that to also be supported.

Change History (18)

This ticket was mentioned in PR #2494 on WordPress/wordpress-develop by fabiankaegy.


3 years ago
#1

  • Keywords has-patch added

Allows the registration of blocks using block.json to register assets via the script, editorScript, viewScript, style or editorStyle keys from within a Theme. Previously this only worked from within a plugin / mu-plugin.

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

#2 @ocean90
3 years ago

This is a duplicate of #54647 and #55486.

fabiankaegy commented on PR #2494:


3 years ago
#4

@gziolo How would you suggest we go about adding the test block to a theme? Should it be a new theme? Or should it be added to an existing one of the test themes? If so which one 🤔

Also since the check for whether or not a block exists within a theme is reliant on the get_theme_file_path path the theme needs to be active in order to be recognized.

gziolo commented on PR #2494:


3 years ago
#5

@gziolo How would you suggest we go about adding the test block to a theme? Should it be a new theme? Or should it be added to an existing one of the test themes? If so which one 🤔

You can add it to one of the existing test themes. block-theme sounds like a great fit:

https://github.com/WordPress/wordpress-develop/tree/trunk/tests/phpunit/data/themedir1/block-theme

Maybe you could add a subfolder blocks there and switch to the theme as part of the test.

Also since the check for whether or not a block exists within a theme is reliant on the get_theme_file_path path the theme needs to be active in order to be recognized.

Example:

https://github.com/WordPress/wordpress-develop/blob/371966a187081662c155ef98ddb7a681dc10f268/tests/phpunit/tests/theme/wpGetGlobalStylesheet.php#L48

It probably requires some changes in the set up:

https://github.com/WordPress/wordpress-develop/blob/371966a187081662c155ef98ddb7a681dc10f268/tests/phpunit/tests/theme/wpGetGlobalStylesheet.php#L10

fabiankaegy commented on PR #2494:


3 years ago
#6

@gziolo I've added two initial test cases following the example of the already existing tests. However, I'm not quite sure what I would need to change here in order to be able to change the theme within the test: https://github.com/WordPress/wordpress-develop/blob/371966a187081662c155ef98ddb7a681dc10f268/tests/phpunit/tests/theme/wpGetGlobalStylesheet.php#L10

Would love to get some more insight into what is required to get that working :)

fabiankaegy commented on PR #2494:


3 years ago
#7

(force pushed to re-run the GitHub actions as per this Slack conversation)

fabiankaegy commented on PR #2494:


3 years ago
#8

@gziolo Seems that even without any changes to the setup the tests have passed

gziolo commented on PR #2494:


3 years ago
#9

@gziolo Seems that even without any changes to the setup the tests have passed

I think, we should make it explicit that the theme and tests register different scripts and styles so we could ensure that their paths are different. Such check is missing for the script. Otherwise, everything looks good.

fabiankaegy commented on PR #2494:


3 years ago
#10

As part of this PR @gziolo and myself tried to add more robust tests for the assets that get registered via block.json files in the theme.

We've tried to actually test the src url of the registered script instead of only the handle:
{{{php
$expected_script_handle = 'block-theme-example-block-view-script';
$this->assertSame( 'http://example.org/wp-content/themes/block-theme/blocks/example-block/view.js', wp_scripts()->query( $expected_script_handle )->src );
}}}

However the test setup does not seem to have stubs in place for the get_theme_file_uri function calls which resulted in the tests passing successfully when running the test suite locally but failing when the tests were run in CI.

After some considerable time debugging we have decided to forgo the addition of these additional tests for now and they have been removed from the PR.

However it would be nice to further stub out the various theme functions so that testing themes gets a little easier and that we can more confidently test theme functionality.

#11 @gziolo
3 years ago

  • Owner set to gziolo
  • Resolution set to fixed
  • Status changed from new to closed

In 53091:

Editor: Allow registration of blocks that include assets from within a theme

Fixes the issue when you register blocks with block.json in your theme. There is no longer an assets's URL error because it resolves correctly in relation to the theme where it is located.

Props fabiankaegy, ocean90, whoisnegrello, audrasjb, peterwilsoncc,
Fixes #54647, #55513.

#13 @gziolo
3 years ago

  • Keywords has-unit-tests added
  • Milestone changed from Awaiting Review to 6.0

#14 @milana_cap
3 years ago

  • Keywords needs-docs added

#15 @matveb
3 years ago

@fabiankaegy the ticket mentions there are cases where it makes sense for a theme to register blocks but it doesn't say what those are. I'm curious what is the rationale.

#16 @matveb
3 years ago

I see there's a broader perspective in https://core.trac.wordpress.org/ticket/54647#comment:10. I'll comment there.

#17 @fabiankaegy
3 years ago

@matveb I've elaborated a bit more here in the DEV Note: https://make.wordpress.org/core/2022/05/03/block-editor-miscellaneous-dev-notes-for-wordpress-6-0/#blocks-in-themes and in the original discussion during the developer hours.

Essentially it comes down to two things:

  • When building completely custom solutions that don't use custom layout systems that don't use the core columns block for example but instead custom grid systems that are very deeply engrained with the themes custom CSS the markup is so deeply engrained with the styling that the content does not make sense without the theme.
  • In these instances there are additional overheads that come from building and deploying them in custom plugins.

As the DEV Note hopefully brings across I very firmly believe the the best practice should always be to build blocks in plugins and that it is the best option for the vast majority of options. However through talking with the wider community we found several instances of the hacky solutions to allow for the registration of blocks from within themes which are error prone. Therefore this patch was added to make it easier for these valid use cases whilst not changing the best practice.

jack-h2ml commented on PR #2494:


23 months ago
#18

My apologies for commenting well after the fact, please do let me know if this is not an appropriate course of action.

The implementation above does not work within the Parent / Child theme paradigm. The changes introduced within wp-includes/blocks.php do not cover the use case whereby a block may be registered within a parent theme, to be used by subsequent child themes.

I have raised an issue covering this in more detail over on the Gutenberg repo WordPress/gutenberg/#47470

Note: See TracTickets for help on using tickets.