Update tag visitor registration with unique ID constants

[pbearne]

Replaced hardcoded ID strings with unique ID constants for tag visitors to ensure consistency and maintainability. Updated the OD_Tag_Visitor_Registry::register method to handle dependencies, adding them only if they are not already registered.

Summary

Fixes #1419

Relevant technical choices

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @[email protected].

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: [email protected].

Co-authored-by: pbearne <[email protected]>
Co-authored-by: westonruter <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[westonruter]

Thanks for getting this off the ground!

See my inline comments.

The ultimate goal would be to refactor this code:

performance/plugins/auto-sizes/optimization-detective.php

Lines 61 to 66 in 73f2235

	function auto_sizes_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void {
	$registry->register( 'auto-sizes', 'auto_sizes_visit_tag' );
	}
	// Important: The Image Prioritizer's IMG tag visitor is registered at priority 10, so priority 100 ensures that the loading attribute has been correctly set by the time the Auto Sizes visitor runs.
	add_action( 'od_register_tag_visitors', 'auto_sizes_register_tag_visitors', 100 );

So we could eliminate the need for the 100 priority.

For example:

function auto_sizes_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void {
	$registry->register( 
		'auto-sizes', 
		'auto_sizes_visit_tag', 
		array( 'image-prioritizer-img' ) 
	);
}
add_action( 'od_register_tag_visitors', 'auto_sizes_register_tag_visitors' );

An option question is what to do if the image-prioritizer-img tag visitor dependency doesn't exist. Does the auto-sizes then never run? Or would it need to rather do something like this:

function auto_sizes_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void {
	$registry->register( 
		'auto-sizes', 
		'auto_sizes_visit_tag', 
		defined( 'Image_Prioritizer_Img_Tag_Visitor::ID' ) ? array( Image_Prioritizer_Img_Tag_Visitor::ID ) : array()
	);
}
add_action( 'od_register_tag_visitors', 'auto_sizes_register_tag_visitors' );

[westonruter]

Suggested change

	* @param array<mixed> $dependencies array with ID as key and Callable as value
	* @param string[] $dependencies Tag visitors that must run before this tag visitor.

[pbearne]

I feel this should be any so we can have more than one dependency

[westonruter]

Yes, and that's still the case: string[] is an array of strings.

[pbearne]

changed

[westonruter]

I don't see the change.

[westonruter]

I was thinking rather that this would work more like how WP_Scripts works, where you can register a script (or tag visitor) with a given handle (or ID) and supply dependencies which are a list of strings (i.e. handles/IDs). Then later when obtaining the list of tag visitors to apply, it would recursively look up the registered tag visitors to make sure that those which have no dependencies run first, followed by those which have dependencies.

[pbearne]

I can see a way of changing this to just passing the className and the dependencies as a series of class names
and then getting the ID from the class

Or just building an array values and then load them in the first call to get_registered

Part of the problem is I am not sure what the code is doing

[westonruter]

The tag visitor registry is a collection of callables which are invoked when iterating over the tags via HTML Tag Processor. A tag visitor may be an invokable class or it may be a closure or a function or any other callable. See the Enhanced Responsive Images for an example tag visitor which is a regular PHP function and not a class:

performance/plugins/auto-sizes/optimization-detective.php

Lines 13 to 52 in bc37748

	/**
	* Visits responsive lazy-loaded IMG tags to ensure they include sizes=auto.
	*
	* @since 1.1.0
	*
	* @param OD_Tag_Visitor_Context $context Tag visitor context.
	* @return false Whether the tag should be recorded in URL metrics.
	*/
	function auto_sizes_visit_tag( OD_Tag_Visitor_Context $context ): bool {
	if ( 'IMG' !== $context->processor->get_tag() ) {
	return false;
	}
	$sizes = $context->processor->get_attribute( 'sizes' );
	if ( ! is_string( $sizes ) ) {
	return false;
	}
	$sizes = preg_split( '/\s*,\s*/', $sizes );
	if ( ! is_array( $sizes ) ) {
	return false;
	}
	$is_lazy_loaded = ( 'lazy' === $context->processor->get_attribute( 'loading' ) );
	$has_auto_sizes = in_array( 'auto', $sizes, true );
	$changed = false;
	if ( $is_lazy_loaded && ! $has_auto_sizes ) {
	array_unshift( $sizes, 'auto' );
	$changed = true;
	} elseif ( ! $is_lazy_loaded && $has_auto_sizes ) {
	$sizes = array_diff( $sizes, array( 'auto' ) );
	$changed = true;
	}
	if ( $changed ) {
	$context->processor->set_attribute( 'sizes', join( ', ', $sizes ) );
	}
	return false; // Since this tag visitor does not require this tag to be included in the URL Metrics.
	}

So in the same way as with WP_Scripts where registered scripts have handles, here too tag visitors (callables) have IDs. As with script dependencies being able to register a script which depends on another script by passing an array of dependency handles, so too as seen in Enhanced Responsive Images there should be a way to register a tag visitor so that it runs after other tag visitors have done.

[westonruter]

This would also make sense to add to Embed_Optimizer_Tag_Visitor.

[pbearne]

done

[westonruter]

Oh yeah, I don't see any change to the Embed_Optimizer_Tag_Visitor class in the Embed Optimizer plugin?

[pbearne]

@westonruter I am not sure what is going on here you might need to take this over