Incorporate breakpoints into preconnect links added by Embed Optimizer

[westonruter]

It would be slightly better if the viewport widths were also taken into account, so that an embed only visible on desktop would not get preconnected on mobile. This can be addressed in a follow-up PR.

Originally posted by @westonruter in #1302 (comment)

Code in question:

performance/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php

Lines 89 to 96 in 84bf4ee

	foreach ( $preconnect_hrefs as $preconnect_href ) {
	$this->link_collection->add_link(
	array(
	'rel' => 'preconnect',
	'href' => $preconnect_href,
	)
	);
	}

[ShyamGadde]

Hi, I've reviewed this issue and have an idea for how to proceed. If it's alright and this issue isn't currently being worked on, could it be assigned to me?

[ShyamGadde]

@westonruter I have some questions regarding the merging logic for preconnect links.

Code in question:

performance/plugins/optimization-detective/class-od-link-collection.php

Lines 106 to 207 in df29967

	/**
	* Prepares links by deduplicating adjacent links and adding media attributes.
	*
	* When two links are identical except for their minimum/maximum widths which are also consecutive, then merge them
	* together. Also, add media attributes to the links.
	*
	* @return LinkAttributes[] Prepared links with adjacent-duplicates merged together and media attributes added.
	*/
	private function get_prepared_links(): array {
	return array_merge(
	...array_map(
	function ( array $links ): array {
	return $this->merge_consecutive_links( $links );
	},
	array_values( $this->links_by_rel )
	)
	);
	}
	/**
	* Merges consecutive links.
	*
	* @param Link[] $links Links.
	* @return LinkAttributes[] Merged consecutive links.
	*/
	private function merge_consecutive_links( array $links ): array {
	// Ensure links are sorted by the minimum_viewport_width.
	usort(
	$links,
	/**
	* Comparator.
	*
	* @param Link $a First link.
	* @param Link $b Second link.
	* @return int Comparison result.
	*/
	static function ( array $a, array $b ): int {
	return $a['minimum_viewport_width'] <=> $b['minimum_viewport_width'];
	}
	);
	/**
	* Deduplicated adjacent links.
	*
	* @var Link[] $prepared_links
	*/
	$prepared_links = array_reduce(
	$links,
	/**
	* Reducer.
	*
	* @param array<int, Link> $carry Carry.
	* @param Link $link Link.
	* @return non-empty-array<int, Link> Potentially-reduced links.
	*/
	static function ( array $carry, array $link ): array {
	/**
	* Last link.
	*
	* @var Link $last_link
	*/
	$last_link = end( $carry );
	if (
	is_array( $last_link )
	&&
	$last_link['attributes'] === $link['attributes']
	&&
	is_int( $last_link['minimum_viewport_width'] )
	&&
	is_int( $last_link['maximum_viewport_width'] )
	&&
	$last_link['maximum_viewport_width'] + 1 === $link['minimum_viewport_width']
	) {
	$last_link['maximum_viewport_width'] = max( $last_link['maximum_viewport_width'], $link['maximum_viewport_width'] );
	// Update the last link with the new maximum viewport width.
	$carry[ count( $carry ) - 1 ] = $last_link;
	} else {
	$carry[] = $link;
	}
	return $carry;
	},
	array()
	);
	// Add media attributes to the deduplicated links.
	return array_map(
	static function ( array $link ): array {
	$media_query = od_generate_media_query( $link['minimum_viewport_width'], $link['maximum_viewport_width'] );
	if ( null !== $media_query ) {
	if ( ! isset( $link['attributes']['media'] ) ) {
	$link['attributes']['media'] = $media_query;
	} else {
	$link['attributes']['media'] .= " and $media_query";
	}
	}
	return $link['attributes'];
	},
	$prepared_links
	);
	}

I noticed that consecutive links with identical attributes are meant to be merged. However, when breakpoints are introduced, the merging doesn’t seem to occur correctly. Specifically, preconnect links are duplicated across viewport widths:

Upon examining the sorted links (full log: https://pastebin.com/7eKfdzgt), I found that this duplication arises because the links are only sorted by minimum_viewport_width. As a result, links with the same href remain separated in the array.

I tried modifying the sorting logic to sort primarily by href and then by minimum_viewport_width. This change produced the following result:

This approach also works for scenarios where embeds should be hidden on specific viewport ranges (e.g., hidden on phablet):

Could you confirm if my approach aligns with the intended merging logic? Specifically, is it correct to group links by href first and then sort by minimum_viewport_width? Or is there an aspect of the merging logic I might be overlooking?

[westonruter]

@ShyamGadde I think this sounds correct!

[westonruter]

The issue didn't come up before because generally only the preload links were being added with media queries. When there are multiple link types and multiple URLs being linked to, it makes sense that the current logic or sorting by the minimum viewport width would be insufficient.