Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TRY] Optimization Detective debug helper #1759

Draft
wants to merge 13 commits into
base: trunk
Choose a base branch
from
Draft

[TRY] Optimization Detective debug helper #1759

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fc95f4a
Bundle web-vitals attribution build
swissspidy Dec 18, 2024
651577e
Allow loading web-vitals attribution build
swissspidy Dec 18, 2024
ff3f120
Collect some INP data for testing
swissspidy Dec 18, 2024
9dd8159
Add debugging helper for LCP elements
swissspidy Dec 18, 2024
12a3494
some lint fixes
swissspidy Dec 18, 2024
be0be49
Add INP elements with JS
swissspidy Dec 18, 2024
e9bbe99
Apply suggestions from code review
swissspidy Dec 19, 2024
ac4d153
Add missing return statement
swissspidy Dec 19, 2024
098191c
Lint fixes
swissspidy Dec 19, 2024
9288025
Clarify comment
swissspidy Dec 19, 2024
2f507a0
PHPStan fixes
swissspidy Dec 19, 2024
8ead9ae
Multiline comment
swissspidy Dec 19, 2024
ad09dea
Merge branch 'trunk' into try/1736-debug-detective
swissspidy Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions plugins/optimization-detective/class-optimization-detective-debug-tag-visitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php
/**
* Image Prioritizer: IP_Img_Tag_Visitor class
*
* @package image-prioritizer
* @since n.e.x.t
*/

// Exit if accessed directly.
if ( ! defined( 'ABSPATH' ) ) {
exit;
}

/**
* Tag visitor that optimizes IMG tags.
*
* @phpstan-import-type LinkAttributes from OD_Link_Collection
*
* @since n.e.x.t
* @access private
*/
final class Optimization_Detective_Debug_Tag_Visitor {

/**
* Visits a tag.
*
* @since n.e.x.t
*
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @return bool Whether the tag should be tracked in URL Metrics.
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
*/
public function __invoke( OD_Tag_Visitor_Context $context ): bool {
$processor = $context->processor;

if ( ! $context->url_metric_group_collection->is_any_group_populated() ) {
return false;
}

$xpath = $processor->get_xpath();

$visited = false;

swissspidy marked this conversation as resolved.
Show resolved Hide resolved
/**
* @var OD_URL_Metric_Group $group
*/
foreach ( $context->url_metric_group_collection as $group ) {
// This is the LCP element for this group.
if ( $group->get_lcp_element() instanceof OD_Element && $xpath === $group->get_lcp_element()->get_xpath() ) {
$uuid = wp_generate_uuid4();

$processor->set_meta_attribute(
'viewport',
$group->get_minimum_viewport_width()
);

$processor->set_attribute(
'style',
"--anchor-name: --od-debug-element-$uuid;" . $processor->get_attribute( 'style' ) ?? ''
);

$processor->set_meta_attribute(
'debug-is-lcp',
true
);

$anchor_text = __( 'Optimization Detective', 'optimization-detective' );
$popover_text = __( 'LCP Element', 'optimization-detective' );

$processor->append_body_html(
<<<HTML
<button
class="od-debug-dot"
type="button"
popovertarget="od-debug-popover-$uuid"
popovertargetaction="toggle"
style="--anchor-name: --od-debug-dot-$uuid; position-anchor: --od-debug-element-$uuid;"
aria-details="od-debug-popover-$uuid"
>
$anchor_text
</button>
<div
id="od-debug-popover-$uuid"
popover
class="od-debug-popover"
style="position-anchor: --od-debug-dot-$uuid;"
>
$popover_text
</div>
HTML
Comment on lines +68 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So cool!

);

$visited = true;
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
}
}

return $visited;
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
}
}
217 changes: 217 additions & 0 deletions plugins/optimization-detective/debug.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
<?php
/**
* Debug helpers used for Optimization Detective.
*
* @package optimization-detective
* @since n.e.x.t
*/

if ( ! defined( 'ABSPATH' ) ) {
exit; // Exit if accessed directly.
}

/**
* Registers tag visitors.
*
* @since n.e.x.t
*
* @param OD_Tag_Visitor_Registry $registry Tag visitor registry.
*/
function od_debug_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void {
$debug_visitor = new Optimization_Detective_Debug_Tag_Visitor();
$registry->register( 'optimization-detective/debug', $debug_visitor );
}

add_action( 'od_register_tag_visitors', 'od_debug_register_tag_visitors', PHP_INT_MAX );


/**
* Filters additional properties for the element item schema for Optimization Detective.
*
* @since n.e.x.t
*
* @param array<string, array{type: string}> $additional_properties Additional properties.
* @return array<string, array{type: string}> Additional properties.
*/
function od_debug_add_inp_schema_properties( array $additional_properties ): array {
$additional_properties['inpData'] = array(
'description' => __( 'INP metrics', 'optimization-detective' ),
'type' => 'array',
// All extended properties must be optional so that URL Metrics are not all immediately invalidated once an extension is deactivated.
'required' => false,
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
'items' => array(
'type' => 'object',
'required' => true,
'properties' => array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to include the LoAF data here at some point.

'value' => array(
'type' => 'number',
'required' => true,
),
'rating' => array(
'type' => 'string',
'enum' => array( 'good', 'needs-improvement', 'poor' ),
'required' => true,
),
'interactionTarget' => array(
'type' => 'string',
'required' => true,
),
),
),
);
return $additional_properties;
}

add_filter( 'od_url_metric_schema_root_additional_properties', 'od_debug_add_inp_schema_properties' );

/**
* Adds a new admin bar menu item for Optimization Detective debug mode.
*
* @since n.e.x.t
*
* @param WP_Admin_Bar $wp_admin_bar The WP_Admin_Bar instance, passed by reference.
*/
function od_debug_add_admin_bar_menu_item( WP_Admin_Bar &$wp_admin_bar ): void {
if ( ! current_user_can( 'customize' ) && ! wp_is_development_mode( 'plugin' ) ) {
return;
}

if ( is_admin() ) {
return;
}

$wp_admin_bar->add_menu(
array(
'id' => 'optimization-detective-debug',
'parent' => null,
'group' => null,
'title' => __( 'Optimization Detective', 'optimization-detective' ),
'meta' => array(
'onclick' => 'document.body.classList.toggle("od-debug");',
Copy link
Member

@westonruter westonruter Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O invoker commands, where art thou?

),
)
);
}

add_action( 'admin_bar_menu', 'od_debug_add_admin_bar_menu_item', 100 );

/**
* Adds inline JS & CSS for debugging.
*/
function od_debug_add_assets(): void {
if ( ! od_can_optimize_response() ) {
return;
}
Comment on lines +105 to +107
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since pages aren't optimized by default for users who can customize, this presents a dilemma. We'd really want to show data collected for end users who aren't logged-in, but as it stands right now only the URL Metrics collected from an admin user would be collected here, which again would normally be none.

Otherwise, URL Metrics are collected for logged-in users but they are stored discretely from those stored for logged-out users. See the user_logged_in "query var" added in the od_get_normalized_query_vars() function:

performance/plugins/optimization-detective/storage/data.php

Lines 66 to 86 in c446d8c

function od_get_normalized_query_vars(): array {
global $wp;
// Note that the order of this array is naturally normalized since it is
// assembled by iterating over public_query_vars.
$normalized_query_vars = $wp->query_vars;
// Normalize unbounded query vars.
if ( is_404() ) {
$normalized_query_vars = array(
'error' => 404,
);
}
// Vary URL Metrics by whether the user is logged in since additional elements may be present.
if ( is_user_logged_in() ) {
$normalized_query_vars['user_logged_in'] = true;
}
return $normalized_query_vars;
}

I'm not entirely happy with how I "abused" the query vars in this way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, well, if we want to access the URL Metrics for unauthenticated requests then we just need to unset the user_logged_in key from the array returned by od_get_normalized_query_vars() below. In fact, we could obtain the URL Metrics from both and combine them into one OD_URL_Metric_Group_Collection, perhaps with a sample size of 2 * od_get_url_metrics_breakpoint_sample_size().

The remaining issue is what to do about the XPaths from the unauthenticated URL Metrics not applying to the current authenticated response (e.g. due to the admin bar). I think we may need an alternative to how we currently check if the current XPath via $processor->get_xpath() matches an XPath stored in a URL Metric. In particular, consider

  Logged-out Logged-in
Screenshot image image
XPath /*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]/*[2][self::MAIN]/*[1][self::DIV]/*[3][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG] /*[1][self::HTML]/*[2][self::BODY]/*[2][self::DIV]/*[2][self::MAIN]/*[1][self::DIV]/*[3][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]

Note the slight difference in the index of the DIV caused by the admin bar:

- /*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]
+ /*[1][self::HTML]/*[2][self::BODY]/*[2][self::DIV]

Maybe we should have a helper that strips out the index from elements which appear as direct children of BODY given that arbitrary elements are added/removed at wp_body_open and wp_footer? If this instead became normalized to:

/*[1][self::HTML]/*[2][self::BODY]/*[self::DIV]

Or more simply:

/*[1][self::HTML]/*[2][self::BODY]/DIV

Then this would match the DIV.wp-site-blocks both for when the user is logged-in and logged-out. This would be particularly effective for block themes which always have this root DIV.wp-site-blocks element from get_the_block_template_html(). It would be more prone to confusion in classic themes, for example, if there are two root DIV elements for both the header and footer. However, in looking at the core themes, every single theme except for Twenty Twenty has a root DIV#page wrapper for the page contents after wp_body_open(). The one exception is Twenty Twenty but it uses HEADER, MAIN, and FOOTER at the root. So my concerns are likely unfounded and we can just leave the index off of elements which are children of BODY.


$slug = od_get_url_metrics_slug( od_get_normalized_query_vars() );
$post = OD_URL_Metrics_Post_Type::get_post( $slug );

global $wp_the_query;

$tag_visitor_registry = new OD_Tag_Visitor_Registry();

$current_etag = od_get_current_url_metrics_etag( $tag_visitor_registry, $wp_the_query, od_get_current_theme_template() );
$group_collection = new OD_URL_Metric_Group_Collection(
$post instanceof WP_Post ? OD_URL_Metrics_Post_Type::get_url_metrics_from_post( $post ) : array(),
$current_etag,
od_get_breakpoint_max_widths(),
od_get_url_metrics_breakpoint_sample_size(),
od_get_url_metric_freshness_ttl()
);
Comment on lines +109 to +123
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this code is duplicated with what is located in od_optimize_template_output_buffer(), maybe we should factor that out into a helper function to return the $group_collection.


$inp_dots = array();

/**
* @var OD_URL_Metric_Group $group
*/
foreach ( $group_collection as $group ) {
/**
* @var OD_URL_Metric $url_metric
*/
foreach ( $group as $url_metric ) {
foreach ( $url_metric->get( 'inpData' ) as $inp_data ) {
if ( isset( $inp_dots[ $inp_data['interactionTarget'] ] ) ) {
$inp_dots[ $inp_data['interactionTarget'] ][] = $inp_data;
} else {
$inp_dots[ $inp_data['interactionTarget'] ] = array( $inp_data );
}
}
}
}

?>
<script>
/* TODO: Add INP elements here */
let count = 0;
for ( const [ interactionTarget, entries ] of Object.entries( <?php echo wp_json_encode( $inp_dots ); ?> ) ) {
const el = document.querySelector( interactionTarget );
if ( ! el ) {
continue;
}

count++;

const anchor = document.createElement( 'button' );
anchor.setAttribute( 'class', 'od-debug-dot' );
anchor.setAttribute( 'popovertarget', `od-debug-popover-${count}` );
anchor.setAttribute( 'popovertargetaction', 'toggle' );
anchor.setAttribute( 'style', `--anchor-name: --od-debug-dot-${count}; position-anchor: --od-debug-element-${count};` );
anchor.setAttribute( 'aria-details', `od-debug-popover-${count}` );
anchor.textContent = 'Optimization Detective';

const tooltip = document.createElement( 'div' );
tooltip.setAttribute( 'id', `od-debug-popover-${count}` );
tooltip.setAttribute( 'popover', '' );
tooltip.setAttribute( 'class', 'od-debug-popover' );
tooltip.setAttribute( 'style', `position-anchor: --od-debug-dot-${count};` );
tooltip.textContent = `INP Element (Value: ${entries[0].value}) (Rating: ${entries[0].rating})`;

document.body.append(anchor);
document.body.append(tooltip);
}
</script>
<style>
body:not(.od-debug) .od-debug-dot,
body:not(.od-debug) .od-debug-popover {
/*display: none;*/
}

.od-debug-dot {
height: 2em;
width: 2em;
background: rebeccapurple;
border-radius: 50%;
animation: pulse 2s infinite;
position: absolute;
position-area: center center;
margin: 5px 0 0 5px;
}

.od-debug-popover {
position: absolute;
position-area: right;
margin: 5px 0 0 5px;
}

@keyframes pulse {
0% {
transform: scale(0.8);
opacity: 0.5;
box-shadow: 0 0 0 0 rgba(102, 51, 153, 0.7);
}
70% {
transform: scale(1);
opacity: 1;
box-shadow: 0 0 0 10px rgba(102, 51, 153, 0);
}
100% {
transform: scale(0.8);
opacity: 0.5;
box-shadow: 0 0 0 0 rgba(102, 51, 153, 0);
}
}
</style>
<?php
}

add_action( 'wp_footer', 'od_debug_add_assets' );
34 changes: 32 additions & 2 deletions plugins/optimization-detective/detect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/**
* @typedef {import("web-vitals").LCPMetric} LCPMetric
* @typedef {import("web-vitals").LCPMetricWithAttribution} LCPMetricWithAttribution
* @typedef {import("web-vitals").INPMetric} INPMetric
* @typedef {import("web-vitals").INPMetricWithAttribution} INPMetricWithAttribution
* @typedef {import("./types.ts").ElementData} ElementData
* @typedef {import("./types.ts").OnTTFBFunction} OnTTFBFunction
* @typedef {import("./types.ts").OnFCPFunction} OnFCPFunction
Expand Down Expand Up @@ -490,13 +493,17 @@ export default async function detect( {
} );
}

/** @type {LCPMetric[]} */
/** @type {(LCPMetric|LCPMetricWithAttribution)[]} */
const lcpMetricCandidates = [];

// Obtain at least one LCP candidate. More may be reported before the page finishes loading.
await new Promise( ( resolve ) => {
onLCP(
( /** @type LCPMetric */ metric ) => {
/**
*
* @param {LCPMetric|LCPMetricWithAttribution} metric
*/
( metric ) => {
lcpMetricCandidates.push( metric );
resolve();
},
Expand All @@ -511,6 +518,26 @@ export default async function detect( {

// Stop observing.
disconnectIntersectionObserver();

const inpData = [];

onINP(
/**
*
* @param {INPMetric|INPMetricWithAttribution} metric
*/
( metric ) => {
if ( 'attribution' in metric ) {
// TODO: Store xpath instead?
inpData.push( {
value: metric.value,
rating: metric.rating,
interactionTarget: metric.attribution.interactionTarget,
} );
}
}
);

if ( isDebug ) {
log( 'Detection is stopping.' );
}
Expand All @@ -522,6 +549,7 @@ export default async function detect( {
height: win.innerHeight,
},
elements: [],
inpData: [],
};

const lcpMetric = lcpMetricCandidates.at( -1 );
Expand Down Expand Up @@ -581,6 +609,8 @@ export default async function detect( {
);
} );

urlMetric.inpData = inpData;

// Only proceed with submitting the URL Metric if viewport stayed the same size. Changing the viewport size (e.g. due
// to resizing a window or changing the orientation of a device) will result in unexpected metrics being collected.
if ( didWindowResize ) {
Expand Down
Loading
Loading