Make WordPress Core

Opened 10 months ago

Closed 7 months ago

Last modified 8 weeks 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 (18)

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


10 months ago
#1

  • Keywords has-patch added

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


7 months ago

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


7 months ago

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


7 months ago

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

#7 @spacedmonkey
7 months ago

  • Keywords has-unit-tests added

@joemcgill I have added unit tests.

@gziolo commented on PR #4607:


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


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

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

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


7 months ago

#12 @spacedmonkey
7 months 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:


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

#18 @audrasjb
8 weeks ago

In 57675:

Editor: Allow symlinks in directories provided to get_block_asset_url().

This changeset allows using symbolic links for editorScript and editorStyle in register_block_type(). This adds realpath to template and stylesheet in get_block_asset_url() to ensure returning canonicalized absolute pathnames.

Follow-up to [56683].

Props antonlukin, spacedmonkey, maxpertici.
Fixes #59175.
See #58525.

Note: See TracTickets for help on using tickets.