@swissspidy I get TypeScript warnings locally here, like navigation not being a valid property on window, etc. The same in regards to the various (valid) properties accessed on the event objects. Do you have any advice for how to fix them?

Actually, looks like they show here too: https://github.com/WordPress/performance/actions/runs/14766566981/job/41459069785?pr=1999

The Navigation API is still relatively new and only supported by Chromium-based browsers. TypeScript by default only ships such types if the feature is supported by at least 2 different browser engines.

Until that happens, you can use the community-maintained navigation-api-types package.

I tried that, but it looks like that package doesn't properly support the latest specification either. It doesn't support e.g. the activation property of https://developer.mozilla.org/en-US/docs/Web/API/Navigation, which is what's needed here. So using this package doesn't resolve the issue, for now I ended up manually typing it on a custom ExtendedWindow type.

That should help with those errors below.

I had it like that originally actually, but that would give me other errors about PageSwapEvent / PageRevealEvent being undefined.

I made several updates in 8545b4f, which resolves all errors except those related to PageSwapEvent and PageRevealEvent.

I am confused why these warnings still appear because:

• Both PageSwapEvent and PageRevealEvent are declared in TypeScript's own dom library. I verified that they're there in the version we currently use.
• The errors all refer to Event (instead of the above two), which I'm puzzled why they do that, even though the JS and TS code refers to PageSwapEvent and PageRevealEvent.

Let me know if you have any ideas.

Hmm looking at 8545b4f, instead of adding such a ExtendedWindow type, you should extend the Window interface. Like so:

declare global {
	interface Window {
		plvtInitViewTransitions?: InitViewTransitionsFunction;
		navigation?: {
			activation: NavigationActivation;
		};
	}
}

There's no need for this const win = window; part (and it doesn't affect minification anyway, so the comment there is incorrect).

The PageSwapEvent errors I can look at tomorrow.

Great, thanks, fixed in eaf9060. I took the comment from Optimization Detective's detect.js, which assigns const win = window with that comment, so I thought that had a purpose.

In that case I would follow #1999 (comment) for now

Great, thanks, fixed in eaf9060. I took the comment from Optimization Detective's detect.js, which assigns const win = window with that comment, so I thought that had a purpose.

For this line:

The purpose was indeed to facilitate minification, since the window global symbol won't be used in favor of win which can be further reduced during minification to something like w.

AFAIK minifiers automatically do that kind of stuff. I'd be very surprised if one needs to help them like that 🤔

I just tried eliminating win in favor of reusing window and the minification used in this repo doesn't introduce any minified alias:

if (0 === window.innerWidth || 0 === window.innerHeight) return void y("Window must have non-zero dimensions for URL Metric collection.");

Otherwise, when minified it is passing through the symbol as-is:

if (0 === win.innerWidth || 0 === win.innerHeight) return void y("Window must have non-zero dimensions for URL Metric collection.");

I'm also surprised that the existing minifier isn't doing something smarter by reducing win to w since it is a local variable in the module.

Regarding the ESLint problem, I applied the suggested code from @ShyamGadde, that's indeed better. Everything passes now 👍

Idea: Instead of having plvt_sanitize_view_transitions_theme_support() sanitize the global variable and then override $_wp_theme_features with the sanitized value, what if there was instead a plvt_get_view_transitions_theme_support() which always returned the sanitized value? Then there would be no need to set plvt_sanitize_view_transitions_theme_support() at init action, and there wouldn't be a risk that a theme made changes to the theme support after the init action happened.

That's a good idea, though I personally think the risk is low enough to ignore it in favor of having a more Core-like API, relying on the actual functions that should be used for this and "patching" the lack of integrated sanitization via a hook like this.

If later there are reports of problems with this, we could reassess the relevance.

Per above, this could be replaced with:

No need for the is_array() checks if the theme support value has been sanitized, right?

Not really, but since the data comes from a global variable, I think it's better to use defensive coding.