Closed brendankenny closed 5 months ago
We could have the attribute* methods return the newly attributed object to avoid the extra type assertion, but I wasn't sure if that was too big a change.
I like this way better, actually, so I added it. Separate commit so it's easy to remove, though.
The main thing missing right now is to copy the type changes to the relevant parts of the README documentation
PTAL
@brendankenny I made some updates to your updates (and merged in @tunetheweb's test fixes). Let me know if you have any concerns with any of the changes I made.
Oh and just realised there's a merge conflict due to #468 being merged.
Actually I ended up just removing all the links to the source types because it looks like they no longer work with GitHub's new file viewer UI.
Fixes #408
Relatively light type clean up for v4. Things are looking pretty good and I ended up not changing that much, which is great. For many use cases there won't be breaking changes here in practice (e.g. if you already had a
LCPMetric
specifically, the name of the base interface or a union it might belong to doesn't matter).It's possible some things still went too far for some sensibilities; let me know and I can just roll those back. Once this is looking good, I'll update the README entries around these.
Changes:
Switch
MetricType
forMetric
(as originally in #359)Most of the time as a user you just want to know which of the union of possible metric types you have, you don't want the combined type in the old
Metric
. If you do somehow end up with a genericMetric
, it's now a discriminated union, so checking e.g.metric.name === 'LCP'
will narrow the type toLCPMetric
. The oldMetric
is still available asMetricBase
(open for name bike shedding)CLSReportCallback
/CLSReportCallbackWithAttribution
, etc. Maybe controversial, but IMO these end up as too many named types for things users will usually never explicitly type, and the only important part is the parameter type, so let's just use that directly.slight clean up of
unattributedOn*
methodse.g.
unattributedOnCLS
was supposedly calling back with ametric
of typeCLSMetricWithAttribution
, which isn't true.metric
is aCLSMetric
and only becomes aCLSMetricWithAttribution
afterattributeCLS
is called. Still a type assertionas CLSMetricWithAttribution
in there, but it better reflects what's really going on. We could have theattribute*
methods return the newly attributed object to avoid the extra type assertion, but I wasn't sure if that was too big a change.as PerformanceEventTiming
when the compiler already knows they'rePerformanceEventTiming
s