Make WordPress Core

Opened 4 months ago

Closed 5 days ago

Last modified 4 days ago

#58525 closed enhancement (fixed)

Create util function for getting block assets uri

Reported by: spacedmonkey's profile spacedmonkey Owned by: spacedmonkey's profile 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 (17)

This ticket was mentioned in PR #4607 on WordPress/wordpress-develop by @spacedmonkey.


4 months ago
#1

  • Keywords has-patch added

#2 @spacedmonkey
3 weeks 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.


3 weeks ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


13 days ago

#6 @joemcgill
9 days 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.

#7 @spacedmonkey
9 days ago

  • Keywords has-unit-tests added

@joemcgill I have added unit tests.

@gziolo commented on PR #4607:


8 days 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:


6 days 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.

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/237508/9211f242-8cd0-4716-93ff-c75f4c0acaa0

#10 @spacedmonkey
6 days 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.


5 days ago

#12 @spacedmonkey
5 days ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56683:

Editor: Introduce get_block_asset_url Utility Function.

This commit introduces a valuable utility function, get_block_asset_url, designed to simplify the retrieval of block asset URLs, such as those for CSS and JavaScript files. This utility eliminates redundancy in both register_block_script_handle and register_block_style_handle. Additionally, get_block_asset_url incorporates an early exit mechanism to optimize performance.

This update includes comprehensive unit tests, covering various scenarios, including asset registration from core (wp-includes), themes, child themes, plugins, and mu-plugins.

Props spacedmonkey, joemcgill, flixos90, gziolo.
Fixes #58525.

@westonruter commented on PR #5318:


4 days 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 @westonruter
4 days 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

Note: See TracTickets for help on using tickets.