Avoid passing positional parameters in Optimization Detective

[westonruter]

This fixes something that has been bugging me for awhile.

The way that you register new tag visitors for Optimization Detective is via the od_register_tag_visitors action. At the moment, this action is passed instances of the following three classes as positional parameters:

1. OD_Tag_Visitor_Registry
2. OD_URL_Metrics_Group_Collection
3. OD_Link_Collection

In reality, only the first parameter is needed. The other two are extras for convenience. Nevertheless, passing them as positional parameters means that developers have to remember which one is which, and when new possible classes are introduced, they just get added on to the end. The situation is similar for the tag visitor, as this callable is passed instances of the following classes:

1. OD_HTML_Tag_Processor
2. OD_URL_Metrics_Group_Collection
3. OD_Link_Collection

Again, when writing a tag visitor and you need to use the OD_Link_Collection instance to add a link to the head, you have to skip over an unused OD_URL_Metrics_Group_Collection instance:

add_action(
    'od_register_tag_visitors',
    static function ( OD_Tag_Visitor_Registry $registry ) {
        $registry->register( 
            'video', 
            static function ( OD_HTML_Tag_Processor $processor, OD_URL_Metrics_Group_Collection $url_metrics_group_collection, OD_Link_Collection $link_collection ): bool {
                if ( 'VIDEO' !== $processor->get_tag() ) {
                    return false;
                }
                if ( $processor->get_attribute( 'poster' ) ) {
                    $url_metrics_group_collection->add_link( array( 
                        'rel' => 'preload',
                        'as' => 'image',
                        'href' => $processor->get_attribute( 'poster' ) ),
                    ) );
                }
                return true;
            }
        );
    }
);

Not only is the $url_metrics_group_collection not used here, but the possible strict type hints on the tag visitor callable mean that these positional parameters would be locked in stone forever. There would not be any flexibility to reorder the parameters or change the names of the classes.

These issues can be fixed by introducing a context object that has these class instances exposed as properties. Compare the above with the below:

add_action(
    'od_register_tag_visitors',
    static function ( OD_Tag_Visitor_Registry $registry ) {
        $registry->register( 
            'video', 
            static function ( OD_Tag_Visitor_Context $ctx ): bool {
                if ( 'VIDEO' !== $ctx->processor->get_tag() ) {
                    return false;
                }
                if ( $ctx->processor->get_attribute( 'poster' ) ) {
                    $ctx->url_metrics_group_collection->add_link( array( 
                        'rel' => 'preload',
                        'as' => 'image',
                        'href' => $processor->get_attribute( 'poster' ) ),
                    ) );
                }
                return true;
            }
        );
    }
);

The benefits here are:

1. No more positional parameters.
2. If we want to introduce new objects in the context, we can do so without breaking any existing code.
3. If we want to rename a context object, we can easily do so with a magic getter.

Similarly, there is no need for the additional parameters being passed to the od_register_tag_visitors since the parameters are passed as part of the context anyway. So they can just be eliminated entirely. They are not needed by Image Prioritizer or Embed Optimizer.

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

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

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>

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

[westonruter]

The base PR has been merged so this is ready to go!

[westonruter]

They are not needed by Image Prioritizer or Embed Optimizer.

See also proposed usage in the auto-sizes plugin: https://github.com/WordPress/performance/pull/1322/files

[adamsilverstein]

Nifty!