Make WordPress Core

Opened 3 years ago

Last modified 2 years ago

#54956 new defect (bug)

[5.9] wp_block_type args - "style" and "script" are always loaded on Frontend

Reported by: pkostadinov's profile pkostadinov Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.9
Component: Script Loader Keywords: has-screenshots 2nd-opinion changes-requested needs-patch needs-unit-tests
Focuses: Cc:

Description

Before 5.9 - style and script are loaded when the block is added to specific page.
After 5.9 - style and script are always loaded, on all pages, all the time, everywhere, regardless if the block is displayed on specific page or not.

More info about my use case:

  • style and script both reference a registered script/style, which have multiple dependencies (wp_register_style has $deps array defined that references more registered styles)

Temporary fix I did to the client code:

  • Replacing style and script with styles and view_script fixed the issue

However this "fix" requires specific WP Core version - 5.8.3 should use style and script, while 5.9 should use styles and view_script.

Change History (8)

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

#2 @hellofromTonya
3 years ago

  • Component changed from General to Editor

Test Report

Env:

  • WordPress: 5.9
  • Plugins: None
  • Theme: TT2
  • OS: macOS Big Sur
  • Browsers: Chrome, Firefox, Edge, and Safari
  • Turn on debug and scripts in wp-config.php
    define( 'WP_DEBUG', true );
    define( 'WP_DEBUG_LOG', true );
    define( 'WP_DEBUG_DISPLAY', true );
    define( 'SCRIPT_DEBUG', true );
    define( 'WP_ENVIRONMENT_TYPE', 'local' );
    

Steps

  1. Add a post with a button and header block. Then change the button's font size and background color.
  2. Add another post with only an image block.
  3. Then view the first post, i.e. the one with the button and header blocks, but not the image block.
  4. Open the Network tab in the browser. Refresh to load the assets into the tab.
  5. Click on each styleshee and look at the GET URL. Expected: the image block's stylesheet should not be in the list. For example: There should not be a
    GET http://test59.local/wp-includes/blocks/image/style.css?ver=5.9
    

Results

  • The image block's stylesheet is not loaded in the frontend when viewing the post that doesn't have an image block ✅
  • What about when removing the debug constants? The image stylesheet is not in the style.min.css nor in the inline styles ✅

Can't reproduce the reported issue.

For reference: here's the `block.json` file for the image block. Notice it's using the style attribute.

#3 @hellofromTonya
3 years ago

  • Keywords reporter-feedback added

Hello @pkostadinov 👋,

Welcome to WordPress Core's Trac! Thank you for opening ticket to report the issue you're experiencing.

I attempted to reproduce the issue using the bundled blocks, see the Test Report, but was not able to reproduce.

Can you share the following to help testers investigate?

  • Step-by-step instructions
  • The code for any custom blocks you're using
  • The theme you're using
  • A list of plugins that are activated
  • Any environment information such as nginx or IIS, browser, etc.

Any information you share is helpful in the process of investigating the issue to find what's causing it. Thanks in advance!

This ticket was mentioned in Slack in #core-test by azouamauriac. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

#6 @pkostadinov
2 years ago

In my original comment I mentioned using "styles and view_script" parameters, but this didn't solve the issue.

So far I haven't found a solution, I went through some of the code changes between 5.8.3 and 5.9, and still haven't found a way to fix this myself. I still havent found the exact location where the WP Core decides if it should load registered styles.

I have prepared an example child theme, for showing the most simple code that reproduces the issue. The child was made for the 3rd most popular theme - hello-elementor, but it will work with almost any theme.

https://github.com/pkostadinov90/test-register-block

The entire issue could be described with one screenshot (actually 4 screenshots):

https://raw.githubusercontent.com/pkostadinov90/test-register-block/main/custom-child/screenshot.jpg

I am really sorry @hellofromTonya, but your testing was related to the native gutenberg blocks, and not custom ones. As per my original comment - the problem is when you register a custom block with register_block_type.

TLDR; Short answers to the questions above:

  • Step-by-step instructions

register_block_type - register 2 different blocks, each with diffent css handle in the "style" argument;
Also it doesn't matter when wp_register_style was called, the issue is happening regardless if called on init or wp_enqueue_scripts; This is visible in my test child theme

  • The code for any custom blocks you're using

https://github.com/pkostadinov90/test-register-block

  • The theme you're using

Any theme, but in the test case I created a child theme for hello-elementor

  • A list of plugins that are activated

None / No plugins active

  • Any environment information such as nginx or IIS, browser, etc.

nginx/1.18.0 (Ubuntu) and nginx/1.14.0 (Ubuntu), PHP 8.0.18 and PHP 8.0.9, not browser dependant

#7 @pkostadinov
2 years ago

Also seems to be related to this:

https://developer.wordpress.org/reference/hooks/should_load_separate_core_block_assets/

The name of the filter makes it sound like it only affect Core blocks, bit this seems to affect custom blocks assets as well.

When I did a __return_true, this fixed my issue.

I don't understand why this filter exists in the first place, and why the default is false - the default should always be to not load CSS that you are not using on the page, aka true.

#8 @pkostadinov
2 years ago

  • Component changed from Editor to Script Loader
  • Keywords has-screenshots 2nd-opinion changes-requested needs-patch needs-unit-tests added; reporter-feedback removed

My suggestion is to do something about this filter https://developer.wordpress.org/reference/hooks/should_load_separate_core_block_assets/ (it's default value is "false") :

  • it's name misleads that it affects only core blocks, while it affects custom blocks as well
  • in my opinion filters should affect all types of blocks, and should not be meant for core blocks only
  • this filter name suggests it does something different that it actually does
  • if I have to name the filter based on what it does right now, it would be called "should_load_all_registered_block_assets" - and because it's the oposite meaning of the original filter any uses of it in the core should be reversed (!). And it's default value should be false, since you do want to load only required assets, not all that are registered.
  • based on @hellofromTonya testing, it's possible that it affects core blocks correctly, and it may be unintended and untested side effect that it affects custom blocks, and that it essentially breaks the expected behaivior. Alternatively this filter could be patched to not affect custom blocks.
  • also it would be amazing if someone manages to create unit tests that catch exactly this issue - assets for custom blocks being loaded only when the custom block is used on the page.

This issue is still relevant in 6.0 and 6.0.1 core versions.

Note: See TracTickets for help on using tickets.