addyosmani / timing.js

Navigation Timing API measurement helpers
1.5k stars 114 forks source link

replaces deprecated window.chrome.loadTimes with PerformancePaintTiming #36

Closed bmdevel closed 4 years ago

bmdevel commented 6 years ago

fixes #35

fecktty commented 6 years ago

Nice to see this PR here ! Thanks @bmdevel Have you validate your change in real production environment ?

bmdevel commented 6 years ago

we are currently using this change in production, yes.

pacoguzman commented 6 years ago

Hi, one question why don't you use the suggested equivalent on the deprecation notice

https://developers.google.com/web/updates/2017/12/chrome-loadtimes-deprecated

function firstPaintTime() {
  if (window.PerformancePaintTiming) {
    const fpEntry = performance.getEntriesByType('paint')[0];
    return (fpEntry.startTime + performance.timeOrigin) / 1000;
  }
}

I guess the fpEntry are equivalent but what about sum performance.timeOrigin instead timing.responseEnd? I'm just curious to understand better your solution, thanks in advance

pacoguzman commented 6 years ago

I've found using performance.timeOrigin is more closer to what we had using the deprecated API loadTimes()

performance.getEntriesByType('paint')[0].startTime
#> 2911.8000000016764
window.timing.getTimes().firstPaintTime
#> 2912 # as calculated before your PR
performance.getEntriesByType('paint')[0].startTime + performance.timeOrigin
#> 1526110571633.0078
performance.getEntriesByType('paint')[0].startTime + performance.timing.responseEnd
#> 1526110574382.8
window.timing.getTimes().firstPaint
#> 1526110571653
bmdevel commented 6 years ago

@pacoguzman i think this slipped in the initial commit
thanks! good catch!