WordPress / performance

Performance plugin from the WordPress Performance Group, which is a collection of standalone performance modules.
https://wordpress.org/plugins/performance-lab/
GNU General Public License v2.0
362 stars 98 forks source link

Dependencies for tag visitors in Optimization Detective #1419

Open westonruter opened 2 months ago

westonruter commented 2 months ago

In at least one case, there is a need currently for a tag visitor to run after another tag visitor has run:

https://github.com/WordPress/performance/blob/e34711dbf286a16523b3c3d35462e0e79ea4c6f9/plugins/auto-sizes/optimization-detective.php#L54-L66

The tag visitor in Enhanced Responsive Sizes needs to run after the IMG tag visitor be

The Image Prioritizer's tag visitor is currently registered with the name ID of img-tags which is not ideal:

https://github.com/WordPress/performance/blob/e34711dbf286a16523b3c3d35462e0e79ea4c6f9/plugins/image-prioritizer/helper.php#L33-L35

It should rather be prefixed like image-prioritizer-img. The image-prioritizer-img string could even be defined as a class constant like Image_Prioritizer_Img_Tag_Visitor::ID. When Enhanced Responsive Sizes registers its tag visitor, it should be able to explicitly declare this dependency:

function auto_sizes_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void { 
    $dependencies = array();
    if ( class_exists( 'Image_Prioritizer_Img_Tag_Visitor' ) ) {
        $dependencies[] = Image_Prioritizer_Img_Tag_Visitor::ID;
    }
    $registry->register( 'auto-sizes', 'auto_sizes_visit_tag', $dependencies ); 
} 

Then there wouldn't be a need to rely on action priorities (and the current implementation of OD_Tag_Visitor_Registry) to ensure that one tag visitor runs before another.

westonruter commented 2 months ago

Note: The one current use case for tag visitor dependencies (Enhanced Responsive Images depending on Image Prioritizer) is going to be undone per https://github.com/WordPress/performance/pull/1476#issuecomment-2297032967. The code the former is going to be moved to the latter.