Clarify docs around a tag visitor's boolean return value

[westonruter]

Important

This is a follow-up PR to #1478. Merge that PR into trunk before merging this PR, since it is branched off of that PR's branch.

Fixes #1342

The terminology around a tag visitor "visiting" a tag is fuzzy. It's not clear what it means for a tag to be "visited". In reality, what this means is that the tag is tracked/recorded in URL metrics so that it appears among its elements, including the intersectionRect and clientBoundingRect among other aspects captured by detect.js. If a tag visitor does not need any of that information, then it can just always return false, even though it may have "visited" a tag by applying some optimization to it. For example. Enhanced Responsive Images (auto-sizes) has a tag visitor doesn't need that information to apply its optimizations as it just adds auto to the sizes attribute if it is lazy-loaded and doesn't have auto already. This is a case where the tag visitor always returns false.

As explained in #1342, instead of returning a boolean value for whether to track the tag in URL metrics, it could be more sophisticated by detecting whether the tag visitor tried to access information about that tag from the stored URL metrics. Or instead of returning a boolean there could be a method on the context object like $context->track_in_url_metrics() which would do the same. I could go either way. But these changes in the PR are a good iterative improvement at least and we can add such a method later.

[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: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: joemcgill <joemcgill@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.

[felixarntz]

@westonruter LGTM!

Given the clearer terminology of "track" vs "visit", should we consider renaming relevant code bits as well? Even if any of the API pieces within Optimization Detective use that, we could probably rename, assuming that so far it's only ourselves that have built integrations with it and we haven't documented it much anyway? Alternatively, we could deprecate the old ones to be super safe.

[westonruter]

@felixarntz which code bits are you referring to?

[felixarntz]

@westonruter

which code bits are you referring to?

Haven't looked at the codebase outside this PR, but this function for example would better have a different name now. Of course this is just a plugin consuming Optimization Detective, but it led me to bring this up.

But there's definitely also relevant code in Optimization Detective itself, e.g. the od_register_tag_visitors action could probably use a more accurate name too.

[westonruter]

These are still tag visitors though. That doesn't change here. What is changed is that tag visitors always visit tags (and they always have) but what is made more clear is what the return value means from a tag visitor. A true return value means that it is indicating that the tag should be tracked/registered/monitored in URL metrics. In the case of the Enhanced Responsive Images plugin, the tag visitor never needs to read the tag out of URL metrics so it always returns false.

[felixarntz]

But why are they called "tag visitors"? I agree with the rationale for the changes in this PR, but in the same sense I find it equally unclear what "visiting" is about when looking at the API as a whole. "Tracking" a tag makes a lot more sense for me in describing the purpose of why I should be using this API. The purpose, as far as I understand, is to track certain tags - which implies "visiting", but that's not really the purpose and less clear to me.

[westonruter]

Tag visitors visit every open tag in the document.

Only a subset of tag visitors need to track the tag in URL metrics. If a tag visitor doesn't need URL metrics to perform it's optimizations (as is the case for auto-sizes) then no tracking is happening.

[felixarntz]

Ah, thanks for clarifying!

[westonruter]

To merge this we first need #1478 to be reviewed and merged 🙏