search

GoogleChrome / web-vitals

Essential metrics for a healthy site.
https://web.dev/vitals
Apache License 2.0
7.59k stars 415 forks source link

Better attribution type #414

Closed tunetheweb closed 6 months ago

tunetheweb commented 10 months ago

Fixes #408

tunetheweb commented 10 months ago

Alternatively we could introduce a new MetricWithAttributionType with that definition (which would be consistent with MetricType) and leave MetricWithAttribution alone for now. Then make the MetricWithAttribution change in v4, to avoid a breaking change for now incase we have any more v3 releases.

It does seem odd to have both interfaces and types defined though but maybe that's OK in this case?

@mcanu any thoughts here?

tunetheweb commented 8 months ago

Anyone any more thoughts on this?

@brendankenny @mcanu @philipwalton

philipwalton commented 8 months ago

@brendankenny can you take a look at this and make a recommendation? I think any change here might be a breaking change, so it'd be good to get this in the v4 release if possible.

philipwalton commented 8 months ago

Also, once we have a decision we can update this PR's base branch to the v4 branch.

brendankenny commented 8 months ago

If there's going to be a breaking change, definitely switching to the unions at the top level would be ideal (e.g. get rid of Metric and have MetricType be the real thing, similar with MetricWithAttribution), then move the types used for documentation to just be documentation.

I would love to give all the types a little refresh but I don't know if I'll have the time in the next ~24 hours if that's the goal for v4.

tunetheweb commented 6 months ago

CLosing in favour of #471