#58525 closed enhancement (fixed)
Create util function for getting block assets uri
Reported by: | spacedmonkey | Owned by: | spacedmonkey |
---|---|---|---|
Milestone: | 6.4 | Priority: | normal |
Severity: | normal | Version: | 5.5 |
Component: | Editor | Keywords: | has-patch has-unit-tests |
Focuses: | performance | Cc: |
Description
In the functions register_block_script_handle
and register_block_style_handle
, there is a lot of repeated logic to the uri for the js / css. This is a problem, as the logic is similar but not exactly the same. Performance fixes are applied to styles and not scripts etc. Create a util function, to stop repeated logic and to ensure that logic remains in sync.
Change History (18)
This ticket was mentioned in PR #4607 on WordPress/wordpress-develop by @spacedmonkey.
15 months ago
#1
- Keywords has-patch added
#2
@
12 months ago
- Keywords needs-refresh needs-unit-tests added
- Milestone changed from Awaiting Review to 6.4
This patch needs a refresh and unit tests added.
This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.
12 months ago
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
12 months ago
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
12 months ago
#6
@
12 months ago
- Keywords needs-refresh needs-unit-tests removed
- Owner set to spacedmonkey
- Status changed from new to assigned
The PR looks close to me. Would be good to get a second set of eyes on it.
12 months ago
#8
This is nice. I really like how you were able to refactor the code to handle the paths to styles and scripts. There were multiple issues in the process of arriving at the existing approach before this patch:
- core blocks have special handling in a few places, and different behaviors depending on the filters used like bundle or not bundle core styles together, etc
- some sites change the default path to themes and plugins
- different handling for paths depending on the platform: windows vs unix vs macOS
There is quite extensive test coverage already present, so if the old and new tests pass then we should be good.
@spacedmonkey commented on PR #4607:
12 months ago
#9
@felixarntz I have pushed some changes over the weekend to fix this issue.
- The unit tests are now passing for themes. It is a weird artifact of the fact that WP switches themes in the unit tests. The fix was based on https://github.com/WordPress/wordpress-develop/commit/a92f7bf8cb52b4d00554b5888d39b6a6440f7824
- This change now has unit tests, for locations a block could be, core, theme, child theme, plugin and mu-plugin. This is an improvement over existing tests.
- This function now is extremely fast. I am not worried about it being called multiple times.
- Code base been refactored to one parameter for a better developer API.
#10
@
12 months ago
@joemcgill @flixos90 Improved the patched. Unit tests now passing and improved DX. If you can take a look.
This ticket was mentioned in Slack in #core-performance by flixos90. View the logs.
12 months ago
@spacedmonkey commented on PR #4607:
12 months ago
#13
This ticket was mentioned in PR #5318 on WordPress/wordpress-develop by @westonruter.
12 months ago
#14
See Slack conversation: https://wordpress.slack.com/archives/C02RQBWTW/p1695745900743789
Trac ticket: https://core.trac.wordpress.org/ticket/58525
@westonruter commented on PR #5318:
12 months ago
#15
Two notices remain:
Notice: Function register_block_script_handle was called incorrectly. The asset file (/var/www/src/wp-includes/blocks/image/view.min.asset.php) for the "viewScript" defined in "core/image" block definition is missing. Please see <a>Debugging in WordPress</a> for more information. (This message was added in version 5.5.0.) in /var/www/src/wp-includes/functions.php on line 6034 Notice: Function register_block_script_handle was called incorrectly. The asset file (/var/www/src/wp-includes/blocks/query/view.min.asset.php) for the "viewScript" defined in "core/query" block definition is missing. Please see <a>Debugging in WordPress</a> for more information. (This message was added in version 5.5.0.) in /var/www/src/wp-includes/functions.php on line 6034
#16
@
12 months ago
Just to connect some threads: r56683 introduced some PHP notices when running PHPUnit tests if scripts haven't been built. See https://wordpress.slack.com/archives/C02RQBWTW/p1695745900743789
@spacedmonkey commented on PR #5318:
12 months ago
#17
Let's consider my PR https://github.com/WordPress/wordpress-develop/pull/5320
Trac ticket: https://core.trac.wordpress.org/ticket/58525