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

Error in register_block_style when should_load_separate_core_block_assets is turned on. #35912

Closed
shimon246 opened this issue Oct 25, 2021 · 12 comments
Labels
[Feature] Blocks Overall functionality of blocks

Comments

@shimon246
Copy link

shimon246 commented Oct 25, 2021

Description

I get an error with register_block_style when I turn on should_load_separate_core_block_assets.
(= add_filter( 'should_load_separate_core_block_assets', '__return_true' ); )

Code

// Register Style.
wp_register_style(
	'core/heading',
	plugin_dir_url( __FILE__ ) . 'build/core/heading/style.css',
	array()
);

register_block_style(
	'core/heading',
	array(
		'name'       => 'heading-default',
		'label'      => 'Default',
		'style_handle' => 'core/heading',
		'is_default' => true,
	)
);

Error message

Fatal error: Uncaught ArgumentCountError: Too few arguments to function {closure}(), 1 passed in /var/www/html/wp-includes/class-wp-hook.php on line 305 and exactly 2 expected in /var/www/html/wp-includes/script-loader.php:2447 Stack trace: #0 /var/www/html/wp-includes/class-wp-hook.php(305): {closure}('\n

The relevant code

class-wp-hook.php 305
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp-hook.php#L305

script-loader.php 2451
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/script-loader.php#L2451-L2453

Expected behavior

If heading-default is set, the CSS (style.css) will be loaded, and if heading-default is not set, the CSS will not be loaded.

Related references

https://make.wordpress.org/core/2021/07/01/block-styles-loading-enhancements-in-wordpress-5-8/#comment-41478
https://core.trac.wordpress.org/ticket/53616
WordPress/wordpress-develop#1482
register_block_type is working fine.
Is register_block_style still being worked on?
If so, please let me know.

Step-by-step reproduction instructions

1.add_filter( 'should_load_separate_core_block_assets', '__return_true' ); for custom plugins etc.
2.Prepare the CSS build/core/heading/style.css
3.Paste the code to add the style for the heading block

// Register Style.
wp_register_style(
	'core/heading',
	plugin_dir_url( __FILE__ ) . 'build/core/heading/style.css',
	array()
);

register_block_style(
	'core/heading',
	array(
		'name'       => 'heading-default',
		'label'      => 'Default',
		'style_handle' => 'core/heading',
		'is_default' => true,
	)
);
@gziolo gziolo added the [Feature] Blocks Overall functionality of blocks label Oct 25, 2021
@gziolo
Copy link
Member

gziolo commented Oct 25, 2021

@aristath, I believe that wp_enqueue_block_style from #32510 tries to simplify the code shared in the example to reproduce the issue. Anyway, do you think it's something related to how should_load_separate_core_block_assets works?

@aristath
Copy link
Member

@aristath, I believe that wp_enqueue_block_style from #32510 tries to simplify the code shared in the example to reproduce the issue.

Hmmm wp_enqueue_block_style refers to stylesheets while register_block_style is for the block-styles API, so I don't think these 2 are related 🤔

Anyway, do you think it's something related to how should_load_separate_core_block_assets works?

It could be... But using the code posted above I was unable to replicate an issue and therefore couldn't debug it.
I don't get any errors on my localhost with that code... Is the snippet perhaps missing something? Is that code added in an action/hook somewhere? wp_register_style normally goes inside a wp_enqueue_scripts action (I tried that too btw, I still didn't get any errors)

@shimon246
Copy link
Author

shimon246 commented Oct 25, 2021

@gziolo @aristath

Thanks for the comment.

Hmmm. The theme happens with Twenty Twenty-One.
TT1 and other block-based themes don't have the error, but they also don't have split loading 😣

I've never heard of the wp_enqueue_block_style function before.

It means that when I use should_load_separate_core_block_assets to load blocks separately, I should use wp_enqueue_block_style to load only when I use CSS with core styles added by register_block_style. style?

postscript:

If the style_handle of register_block_style is set to a handle name, an error is output.
If you use inline_style, no error will be printed.

@aristath
Copy link
Member

For enqueueing stylesheets specific to a single block, you can use the code from this post: https://make.wordpress.org/core/2021/07/01/block-styles-loading-enhancements-in-wordpress-5-8/
The wp_enqueue_block_style is new and will be available in WP5.9, so if you want to be compatible with older versions on WP I wouldn't recommend using it.

Since I'm not getting an error with the code you posted, a semi-random idea: Did you try naming your stylesheet something that does not conflict with block-names? So instead of core/heading, try naming your stylesheet's handle something like my-theme-block-styles-core-heading

@shimon246
Copy link
Author

@aristath
Thank you.🥺
I'm still getting the following error message when using style_handle

Fatal error: Uncaught ArgumentCountError: Too few arguments to function {closure}(), 1 passed in /Users/〇〇/Local Sites/〇〇/app/public/wp-includes/class-wp-hook.php on line 305 and exactly 2 expected in /Users/〇〇/Local Sites/〇〇/app/public/wp-includes/script-loader.php on line 2447

ArgumentCountError: Too few arguments to function {closure}(), 1 passed in /Users/〇〇/Local Sites/〇〇/app/public/wp-includes/class-wp-hook.php on line 305 and exactly 2 expected in /Users/〇〇/Local Sites/〇〇/app/public/wp-includes/script-loader.php on line 2447

Here is the complete code

<?php
/**
 * Plugin Name: My Plugin
 */
add_filter('should_load_separate_core_block_assets', '__return_true');

wp_register_style(
  'my-theme-block-styles-core-heading',
  plugin_dir_url(__FILE__) . 'style.css',
  array()
);

register_block_style(
  'core/heading',
  array(
    'name'         => 'my-theme-block-styles-core-heading-add',
    'label'        => 'Add',
    'style_handle' => 'my-theme-block-styles-core-heading',
    // 'inline_style' => '.wp-block-heading.is-style-heading-add , .is-style-heading-add { color: blue; }',
    )
);

Of course, this error message will only be displayed on pages that use the heading block.

@aristath
Copy link
Member

aristath commented Oct 26, 2021

I tried the code posted above:

  • Created a new PHP file in wp-content/plugins and pasted your code
  • Activated the plugin from the dashboard.

As soon as I activate the plugin, I'm getting the following error:

Notice: wp_register_style was called incorrectly. Scripts and styles should not be registered or enqueued until the wp_enqueue_scripts, admin_enqueue_scripts, or login_enqueue_scripts hooks. This notice was triggered by the my-theme-block-styles-core-heading handle. Please see Debugging in WordPress for more information. (This message was added in version 3.3.0.) in /var/www/src/wp-includes/functions.php on line 5714

After fixing that, I can replicate the issue. I've already found the solution to it, so I'll create a new ticket in WP-Core and push a patch for it 👍

The code I used to replicate:

<?php
/**
 * Plugin Name: My Plugin
 */

add_filter( 'should_load_separate_core_block_assets', '__return_true' );
add_action( 'wp_enqueue_scripts', 'test_registering_a_block_style' );
add_action( 'admin_enqueue_scripts', 'test_registering_a_block_style' );

function test_registering_a_block_style() {
	wp_register_style(
		'my-theme-block-styles-core-heading',
		plugins_url( 'style.css', __FILE__ ),
	);
}

register_block_style(
	'core/heading',
	array(
		'name'         => 'my-theme-block-styles-core-heading-add',
		'label'        => 'Add',
		'style_handle' => 'my-theme-block-styles-core-heading',
		// 'inline_style' => '.wp-block-heading.is-style-heading-add , .is-style-heading-add { color: blue; }',
	)
);

Thank you for sticking with it and providing the necessary information to debug it! ❤️

@aristath
Copy link
Member

Error reported in https://core.trac.wordpress.org/ticket/54323 and patch submitted.
Most likely the fix will be included in WP 5.9 👍

@skorasaurus
Copy link
Member

@aristath thanks for the prompt fix; can we close this ticket then?

@shimon246
Thank you for the detailed feedback and reports. Please reopen if you're still experiencing this in WordPress 5.9.

david-binda added a commit to david-binda/gutenberg that referenced this issue Oct 29, 2021
The WordPress `enqueue_block_styles_assets` function contains a bug
which may lead to fatal errors for full site editing enabled themes
which register custom styles and are missing a HTML template for
request (for instance 404.html).

The bug was patched in WordPress core ( see
https://core.trac.wordpress.org/ticket/54323 ), but since the Gutenberg
11.8.0 introduces the following line:

```
add_filter( 'should_load_separate_core_block_assets', '__return_true' );
```

it also should patch the issue, in order to prevent fatal errors for the
themes matching the above mentioned criteria.

See WordPress#35912 and WordPress#35903
@david-binda
Copy link
Contributor

Sorry for commenting on an already closed issue, but I believe that it might be actually helpful, if Gutenberg could patch the issue now originating in WordPress, since in v11.8.0, it enforces the add_filter( 'should_load_separate_core_block_assets', '__return_true' ); via 5.9 compat, and thus eventually makes the fatal errors to happen in themes, which were otherwise working fine, till the v11.8.0 version.

One such an example is the blockbase theme, which in it's latest version removes the block template for 404 page, and hits the bug (if run on WordPress 5.8 with Gutenberg 11.8.0).

I have kindly opened a PR with proposed fix for Gutenberg.

@shimon246
Copy link
Author

@aristath
Thanks for the fix.
I can confirm that WP 5.8.2 doesn't raise an Error!
But I think there is one problem.

I think this code only loads the css specified in style_handle when the core heading block is used.

<?php
/**
 * Plugin Name: My Plugin
 */

add_filter( 'should_load_separate_core_block_assets', '__return_true' );
add_action( 'wp_enqueue_scripts', 'test_registering_a_block_style' );
add_action( 'admin_enqueue_scripts', 'test_registering_a_block_style' );

function test_registering_a_block_style() {
	wp_register_style(
		'my-theme-block-styles-core-heading',
		plugins_url( 'style.css', __FILE__ ),
	);
}

register_block_style(
	'core/heading',
	array(
		'name'         => 'my-theme-block-styles-core-heading-add',
		'label'        => 'Add',
		'style_handle' => 'my-theme-block-styles-core-heading',
		// 'inline_style' => '.wp-block-heading.is-style-heading-add , .is-style-heading-add { color: blue; }',
	)
);

#35912 (comment)
However, the script is loaded even when I'm not using the heading block.

Is this normal behavior?

@aristath
Copy link
Member

However, the script is loaded even when I'm not using the heading block.

Is this normal behavior?

Yes, it's normal. We haven't (yet) applied to JS the same optimizations we've applied to CSS, that is a work in progress and will land at some point. 👍

@shimon246
Copy link
Author

@aristath
Thanks for the reply! I guess it was still impossible in the current situation.
I've created a ticket in the core with an idea to solve this problem, can you take a look here?🥺https://core.trac.wordpress.org/ticket/54457

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

No branches or pull requests

5 participants