#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
assetsand notstylesin 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_assetsistrue:style.cssbecomescommon.css(does not contain block styles)wp_should_load_separate_core_block_assetsisfalse: Everything works exactly as it did in WP 5.7style.cssfile.Setting
wp_should_load_separate_core_block_assetstofalse(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 totruechanges 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_assetsisfalse. Sincewp_should_load_separate_core_block_assetschecks 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-librarystylesheet fromstyle.csstocommon.css.Since
wp_should_load_separate_core_block_assetschecks 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_assetsistrue, 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_assetsisfalse, then styles get enqueued. Core blocks haven't been registered because they get skipped inregister_block_style_handleso 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_assetsisfalse, so only 3rd-party block styles get enqueued. Core block styles are included in the mainstyle.cssfile - just like it was prior to WP5.8.