Expose the target element in INP attribution

[philipwalton]

Fixes #456.

This PR exposes the HTMLElement object in addition to a selector pointing to that object, to support use cases where additional information from the element itself is useful.

The following properties were added:

• CLSAttribution.largestShiftTargetElement
• INPAttribution.interactionTargetElement
• LCPAttribution.targetElement

Breaking change:

In addition, since we realized we had inconsistent naming for LCP—where the element property was a CSS elector, whereas it's called "target" in every other attribution object—this PR renames element to target to be consistent:

-LCPAttribution.element
+LCPAttribution.target

Given that this will be released with v4 and there is already a breaking change in the LCPAttribution interface, it made sense to rename rather than alias and deprecate.

[Tiggerito]

I think the element->target means the code examples would need updating. e.g.

https://github.com/GoogleChrome/web-vitals?tab=readme-ov-file#send-attribution-data
https://github.com/GoogleChrome/web-vitals?tab=readme-ov-file#lcp-attribution

[tunetheweb]

I'm OK with this (once you address @Tiggerito 's concern about the README examples).

However, at risk of bikeshedding:

In the LCP entry it's known as element not target. For INP it is target in the entry so I guess that's why we originally used it in the attribution object. I think the term "target" kinda makes sense to me for INP (and FID), but less so for LCP.

In fact the more I think about this, the more I think it's CLS that's wrong, not LCP. The shift wasn't "targeted"—in fact it was often the victim of another element being inserted. The LayoutShift entry has no target entry (though it does have a sources.node—maybe it should be largestShiftNode).

However, saying all that, using "target" to mean the selector, does free up "element" to mean node for LCP so, for that reason, I think I'm in agreement with this PR even if it's a little inaccurate for LCP (and CLS).

I noticed we've not added it to FID either. Maybe that's OK since it's deprecated, but it doesn't seem like that much effort to add an eventTargetElement.

[philipwalton]

However, at risk of bikeshedding:

Yeah, it's tricky because I think we want to balance three things:

1. Clarity and intuitiveness of naming
2. Consistency between metrics (despite the underlying APIs not being consistent)
3. Minimal changes to existing users

And I'm honestly not sure what solution best address all three of these...

That said, if this is a "pick two" kinda situation, I'd probably pick (1) and (3) since there is already inconsistency, so maybe it's better to just leave LCP and CLS as they are, and only introduce this for INP.

Also, for LCP and CLS, there isn't actually a need for an element reference (yet), since the entries for those metrics will always contain a reference to the element. This is only needed for INP, since there are cases where entries[0].target will not match attribution.interactionTarget, and maybe that's a good enough reason to only update INP, and to pick the name that makes the most sense when having both.

Also, since we're already changing INP attribution names, we have more leway to pick the best name. And if we ever make a similar change for LCP and CLS (e.g. where we store a reference to the element ASAP to minimize null references), then we can similarly update those attribution interfaces. WDYT?

If we just change INP, how about using these names?

interface INPAttribution {
  interactionTargetSelector: string;
  interactionTargetElement: Node | undefined;
}

[tunetheweb]

That works for me!

[philipwalton]

That works for me!

Updated. I'll also update the PR description once we finalize what changes we're making.

[philipwalton]

cc: @just-jeb in case you have thoughts on this PR (either the naming, or in general).

[philipwalton]

We discussed this offline and decided to close this in favor of #479, which keeps the original name the same.