#53505 closed defect (bug) (fixed)
`wp_should_load_separate_core_block_assets()` is confusing
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 5.8 | Priority: | normal |
Severity: | normal | Version: | 5.8 |
Component: | Script Loader | Keywords: | has-patch |
Focuses: | Cc: |
Description
The wp_should_load_separate_core_block_assets()
function is confusing. There is not enough context to understand what "separate" means.
"Separate" means that the CSS assets will be loaded individually instead of one combined asset. It's also confusing which outcome true
or false
results in. This extends to the filter as well.
See the discussion on Slack.
Attachments (2)
Change History (16)
This ticket was mentioned in Slack in #core-editor by desrosj. View the logs.
4 years ago
#3
@
4 years ago
Thanks for these details, @aristath! I am still giving this one some thought, and will circle back with what I come up with a bit later today.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
4 years ago
This ticket was mentioned in PR #1444 on WordPress/wordpress-develop by aaronjorbin.
4 years ago
#6
- Keywords has-patch added; needs-patch removed
See https://core.trac.wordpress.org/ticket/53505
Props aristath who gave a fantastic explanation of how and why this function exists. I synthasysed that into a more docs friendly paragraph and added a couple of @see for places that use this function.
Trac ticket:
#7
@
4 years ago
Took a pass at turning @aristath's excellent explanation into something that will fit in an inline doc
#9
follow-up:
↓ 10
@
4 years ago
Thanks everyone! This is much better. I still don't love the function name, but a better alternative is not coming to mind.
53505.2.diff has some small tweaks to [51256] to refer to assets more instead of stylesheet as the plan is to use this for scripts as well. It also copies some of the improvements already made to the should_load_separate_core_block_assets
filter docblock.
#10
in reply to:
↑ 9
@
4 years ago
Replying to desrosj:
53505.2.diff has some small tweaks to [51256] to refer to assets more instead of stylesheet as the plan is to use this for scripts as well.
Looks good to me :)
My only concern is that mentioning scripts here is technically not quite correct yet, but I guess that will fix itself when the function is also used for scripts in a future release.
#12
@
4 years ago
- Resolution set to fixed
- Status changed from new to closed
With RC later today, I'm going to close this one out. A better function/hook name has not been suggested, but the documentation is much more clear now.
4 years ago
#14
Merged into Core in https://core.trac.wordpress.org/changeset/51256.
I agree that the function-name is not perfect and could be improved, suggestions are welcomed.
Since I see in a few Slack conversations & tickets there's some confusion regarding what the function does, I'll post some details below cause it's admittedly not easy.
Currently, the implementation deals mostly with stylesheets but the plan is to expand the behavior to scripts - which is why it's using the term
assets
and notstyles
in its name.The goal is to have sites that only load the scripts and styles they need instead of monolithic, large assets, therefore improving performance & sustainability.
What the function does:
Editor
On the editor it doesn't really make a difference and things work as they did prior to 5.8. Nothin changes there for backwards-compatibility reasons.
Frontend
On the frontend:
wp_should_load_separate_core_block_assets
istrue
:style.css
becomescommon.css
(does not contain block styles)wp_should_load_separate_core_block_assets
isfalse
: Everything works exactly as it did in WP 5.7style.css
file.Setting
wp_should_load_separate_core_block_assets
tofalse
(which is the default) doesn't change anything, and things work exactly the way they did prior to WP 5.8. The reason this was done is compatibility with themes, as setting the value totrue
changes the loading order of styles and themes may need to change some of their styles to make them more specific. So this is opt-in for classic themes, as they will need to test it and possibly tweak some styles.Backtracking where the function is used and how:
register_block_style_handle
: (Runs both in frontend and backend)Skips registering styles for core-only blocks when
should_load_separate_core_block_assets
isfalse
. Sincewp_should_load_separate_core_block_assets
checks foris_admin()
, core blocks won't register their separate styles in the editor and therefore separate styles won't be enqueued, making this change only apply to the frontend.wp_default_styles
: (Runs both in frontend and backend)Changes the URL of the
block-library
stylesheet fromstyle.css
tocommon.css
.Since
wp_should_load_separate_core_block_assets
checks foris_admin()
, this change will only happen on the frontend.wp_enqueue_registered_block_scripts_and_styles
: (Runs both in frontend and backend)wp_should_load_separate_core_block_assets
istrue
, then styles both for core & 3rd-party blocks get enqueued onrender_block
. The check in this function ensures that we exit early and therefore these styles won't always get enqueued, but instead will only be enqueued on-render.wp_should_load_separate_core_block_assets
isfalse
, then styles get enqueued. Core blocks haven't been registered because they get skipped inregister_block_style_handle
so only 3rd-party block styles get enqueued. This makes 3rd-party blocks styles always be enqueued - just like it was prior to WP5.8.wp_should_load_separate_core_block_assets
isfalse
, so only 3rd-party block styles get enqueued. Core block styles are included in the mainstyle.css
file - just like it was prior to WP5.8.