Update naming of Metric object enums to be consistent #247
Conversation
| navigationType: NavigationTimingType | 'back_forward_cache' | 'prerender' | undefined; | ||
| navigationType: 'navigate' | 'reload' | 'back-forward' | 'back-forward-cache' | 'prerender'; |
There was a problem hiding this comment.
On line 64 you mention undefined but have remove that from here?
There was a problem hiding this comment.
Correct, I updated the code so the undefined would no longer be possible.
Note, I don't think it was ever technically possible before, but according to the type logic is was possible since getNavigationEntry() can return undefined in cases where the browser doesn't support any performance APIs at all (see #185). However, browsers that do not support any performance APIs should not be able to get this far in the code to ever fire these callbacks (all onXXX() functions are gated on some sort of performance API support), which is why I say I don't think it's technically possible, at least not in any browser I'm aware of.
I made the change because I thought it was much riskier to have the type definition advertise that it is possible to get undefined here, and I didn't want developers to be coding defensively for something that I don't think can happen (and now definitely can't because the default is 'navigate').
| metric = initMetric('TTFB', 0); | ||
| report = bindReporter(onReport, metric, thresholds, opts!.reportAllChanges); | ||
| report(true); | ||
| // Only report TTFB after bfcache restores if a `navigation` entry |
There was a problem hiding this comment.
@tunetheweb FYI: I moved this onBFCacheRestore() call inside the check for the navigation entry to ensure that a TTFB metric wasn't reported after a bfcache restore in cases where the browser doesn't support the Navigation Timing API at all.
This still wouldn't affect cases where navigationType was undefined because in bfcache restores the type is set to back-forward-cache, but I do think it was a bug since browsers like Opera mini in extreme data saver mode might have run into this (if Opera mini supports bfcache, not sure about that..).
I checked all the other functions to, but they were all fine since they're gated on support for PerformanceObserver.
This PR updates all enum values exposed on the
Metricobject to use naming conventions consistent with those recommended by the TAG's Design Principles guide (i.e. kebab case).This change updates two of the
navigationTypevalues,'back_forward'and'back_forward_cache', and renames them to'back-forward'and'back-forward-cache'(changing them from snake case to kebab case). Note that this PR also includes other renames as well, but none of those names were from released versions, so they are not a use-visible change.Note that technically this is a breaking change since the
navigationTypeproperty was released in v3.0.0-beta.0, but it is only a breaking change for users of the v3 beta versions. It is not a breaking change for users upgrading from version 2 or older.