From e8f254b1ceeca19d294bb44f07c3b25056d1e723 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Tue, 5 Mar 2024 17:10:29 +0000 Subject: [PATCH 1/4] Cap TTFB in attribution --- src/attribution/onFCP.ts | 14 +++++++++++++- src/attribution/onLCP.ts | 17 +++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/attribution/onFCP.ts b/src/attribution/onFCP.ts index f2257d21..0501228d 100644 --- a/src/attribution/onFCP.ts +++ b/src/attribution/onFCP.ts @@ -31,9 +31,21 @@ const attributeFCP = (metric: FCPMetric): void => { const navigationEntry = getNavigationEntry(); const fcpEntry = metric.entries[metric.entries.length - 1]; + // Should always have an fcpEntry to be able to attribute + if (!fcpEntry) return; + if (navigationEntry) { const activationStart = navigationEntry.activationStart || 0; - const ttfb = Math.max(0, navigationEntry.responseStart - activationStart); + // Cap at 0 and ignore negative responseStart, future responseStart, + // or responseStart after LCP + const ttfb = Math.max( + 0, + navigationEntry.responseStart <= 0 || + navigationEntry.responseStart > performance.now() || + navigationEntry.responseStart - activationStart > fcpEntry.startTime + ? 0 + : navigationEntry.responseStart - activationStart, + ); (metric as FCPMetricWithAttribution).attribution = { timeToFirstByte: ttfb, diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index fcea5625..7a9ce22b 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -33,13 +33,26 @@ const attributeLCP = (metric: LCPMetric) => { if (navigationEntry) { const activationStart = navigationEntry.activationStart || 0; const lcpEntry = metric.entries[metric.entries.length - 1]; + + // Should always have an lcpEntry to be able to attribute + if (!lcpEntry) return; + const lcpResourceEntry = lcpEntry.url && performance .getEntriesByType('resource') .filter((e) => e.name === lcpEntry.url)[0]; - const ttfb = Math.max(0, navigationEntry.responseStart - activationStart); + // Cap at 0 and ignore negative responseStart, future responseStart, + // or responseStart after LCP + const ttfb = Math.max( + 0, + navigationEntry.responseStart <= 0 || + navigationEntry.responseStart > performance.now() || + navigationEntry.responseStart - activationStart > lcpEntry.startTime + ? 0 + : navigationEntry.responseStart - activationStart, + ); const lcpRequestStart = Math.max( ttfb, @@ -55,7 +68,7 @@ const attributeLCP = (metric: LCPMetric) => { ); const lcpRenderTime = Math.max( lcpResponseEnd, - lcpEntry ? lcpEntry.startTime - activationStart : 0, + lcpEntry.startTime - activationStart, ); const attribution: LCPAttribution = { From f63896a14008cb698c0a19fccb2b22b2839b7cca Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Mon, 11 Mar 2024 14:26:12 +0000 Subject: [PATCH 2/4] Refactor --- src/attribution/onFCP.ts | 19 ++++++------------- src/attribution/onLCP.ts | 19 +++++-------------- src/lib/invalidTiming.ts | 25 +++++++++++++++++++++++++ src/onTTFB.ts | 9 ++------- 4 files changed, 38 insertions(+), 34 deletions(-) create mode 100644 src/lib/invalidTiming.ts diff --git a/src/attribution/onFCP.ts b/src/attribution/onFCP.ts index 0501228d..e955e1ac 100644 --- a/src/attribution/onFCP.ts +++ b/src/attribution/onFCP.ts @@ -17,6 +17,7 @@ import {getBFCacheRestoreTime} from '../lib/bfcache.js'; import {getLoadState} from '../lib/getLoadState.js'; import {getNavigationEntry} from '../lib/getNavigationEntry.js'; +import {invalidTiming} from '../lib/invalidTiming.js'; import {onFCP as unattributedOnFCP} from '../onFCP.js'; import { FCPMetric, @@ -29,23 +30,15 @@ import { const attributeFCP = (metric: FCPMetric): void => { if (metric.entries.length) { const navigationEntry = getNavigationEntry(); - const fcpEntry = metric.entries[metric.entries.length - 1]; - // Should always have an fcpEntry to be able to attribute - if (!fcpEntry) return; + const fcpEntry = metric.entries[metric.entries.length - 1]; if (navigationEntry) { + const responseStart = navigationEntry.responseStart; + if (invalidTiming(responseStart)) return; + const activationStart = navigationEntry.activationStart || 0; - // Cap at 0 and ignore negative responseStart, future responseStart, - // or responseStart after LCP - const ttfb = Math.max( - 0, - navigationEntry.responseStart <= 0 || - navigationEntry.responseStart > performance.now() || - navigationEntry.responseStart - activationStart > fcpEntry.startTime - ? 0 - : navigationEntry.responseStart - activationStart, - ); + const ttfb = Math.max(0, responseStart - activationStart); (metric as FCPMetricWithAttribution).attribution = { timeToFirstByte: ttfb, diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index 7a9ce22b..6af25a7a 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -16,6 +16,7 @@ import {getNavigationEntry} from '../lib/getNavigationEntry.js'; import {getSelector} from '../lib/getSelector.js'; +import {invalidTiming} from '../lib/invalidTiming.js'; import {onLCP as unattributedOnLCP} from '../onLCP.js'; import { LCPAttribution, @@ -31,29 +32,19 @@ const attributeLCP = (metric: LCPMetric) => { const navigationEntry = getNavigationEntry(); if (navigationEntry) { + const responseStart = navigationEntry.responseStart; + if (invalidTiming(responseStart)) return; + const activationStart = navigationEntry.activationStart || 0; + const ttfb = Math.max(0, responseStart - activationStart); const lcpEntry = metric.entries[metric.entries.length - 1]; - // Should always have an lcpEntry to be able to attribute - if (!lcpEntry) return; - const lcpResourceEntry = lcpEntry.url && performance .getEntriesByType('resource') .filter((e) => e.name === lcpEntry.url)[0]; - // Cap at 0 and ignore negative responseStart, future responseStart, - // or responseStart after LCP - const ttfb = Math.max( - 0, - navigationEntry.responseStart <= 0 || - navigationEntry.responseStart > performance.now() || - navigationEntry.responseStart - activationStart > lcpEntry.startTime - ? 0 - : navigationEntry.responseStart - activationStart, - ); - const lcpRequestStart = Math.max( ttfb, // Prefer `requestStart` (if TOA is set), otherwise use `startTime`. diff --git a/src/lib/invalidTiming.ts b/src/lib/invalidTiming.ts new file mode 100644 index 00000000..3decdb9b --- /dev/null +++ b/src/lib/invalidTiming.ts @@ -0,0 +1,25 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export const invalidTiming = (timing: number) => { + // In some cases no value is reported by the browser (for + // privacy/security reasons), and in other cases (bugs) the value is + // negative or is larger than the current page time. Ignore these cases: + // https://github.com/GoogleChrome/web-vitals/issues/137 + // https://github.com/GoogleChrome/web-vitals/issues/162 + // https://github.com/GoogleChrome/web-vitals/issues/275 + return timing <= 0 || timing > performance.now(); +}; diff --git a/src/onTTFB.ts b/src/onTTFB.ts index cd3d3aeb..dbab4448 100644 --- a/src/onTTFB.ts +++ b/src/onTTFB.ts @@ -16,6 +16,7 @@ import {bindReporter} from './lib/bindReporter.js'; import {initMetric} from './lib/initMetric.js'; +import {invalidTiming} from './lib/invalidTiming.js'; import {onBFCacheRestore} from './lib/bfcache.js'; import {getNavigationEntry} from './lib/getNavigationEntry.js'; import { @@ -77,13 +78,7 @@ export const onTTFB = (onReport: TTFBReportCallback, opts?: ReportOpts) => { if (navEntry) { const responseStart = navEntry.responseStart; - // In some cases no value is reported by the browser (for - // privacy/security reasons), and in other cases (bugs) the value is - // negative or is larger than the current page time. Ignore these cases: - // https://github.com/GoogleChrome/web-vitals/issues/137 - // https://github.com/GoogleChrome/web-vitals/issues/162 - // https://github.com/GoogleChrome/web-vitals/issues/275 - if (responseStart <= 0 || responseStart > performance.now()) return; + if (invalidTiming(responseStart)) return; // The activationStart reference is used because TTFB should be // relative to page activation rather than navigation start if the From 2b6ff7090b8739a27a7a55225d65ad392be466aa Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Mon, 11 Mar 2024 14:34:33 +0000 Subject: [PATCH 3/4] Tidy up --- src/attribution/onFCP.ts | 1 - src/attribution/onLCP.ts | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/attribution/onFCP.ts b/src/attribution/onFCP.ts index e955e1ac..6c5bbd43 100644 --- a/src/attribution/onFCP.ts +++ b/src/attribution/onFCP.ts @@ -30,7 +30,6 @@ import { const attributeFCP = (metric: FCPMetric): void => { if (metric.entries.length) { const navigationEntry = getNavigationEntry(); - const fcpEntry = metric.entries[metric.entries.length - 1]; if (navigationEntry) { diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index 6af25a7a..fc898de5 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -36,15 +36,15 @@ const attributeLCP = (metric: LCPMetric) => { if (invalidTiming(responseStart)) return; const activationStart = navigationEntry.activationStart || 0; - const ttfb = Math.max(0, responseStart - activationStart); const lcpEntry = metric.entries[metric.entries.length - 1]; - const lcpResourceEntry = lcpEntry.url && performance .getEntriesByType('resource') .filter((e) => e.name === lcpEntry.url)[0]; + const ttfb = Math.max(0, responseStart - activationStart); + const lcpRequestStart = Math.max( ttfb, // Prefer `requestStart` (if TOA is set), otherwise use `startTime`. From 4372eb20162b13986761cccecdfb72836fe81f25 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Tue, 19 Mar 2024 12:26:22 +0000 Subject: [PATCH 4/4] Review feedback --- src/attribution/onFCP.ts | 4 ++-- src/attribution/onLCP.ts | 4 ++-- src/lib/{invalidTiming.ts => isInvalidTimestamp.ts} | 4 ++-- src/onTTFB.ts | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename src/lib/{invalidTiming.ts => isInvalidTimestamp.ts} (88%) diff --git a/src/attribution/onFCP.ts b/src/attribution/onFCP.ts index 6c5bbd43..54f23110 100644 --- a/src/attribution/onFCP.ts +++ b/src/attribution/onFCP.ts @@ -17,7 +17,7 @@ import {getBFCacheRestoreTime} from '../lib/bfcache.js'; import {getLoadState} from '../lib/getLoadState.js'; import {getNavigationEntry} from '../lib/getNavigationEntry.js'; -import {invalidTiming} from '../lib/invalidTiming.js'; +import {isInvalidTimestamp} from '../lib/isInvalidTimestamp.js'; import {onFCP as unattributedOnFCP} from '../onFCP.js'; import { FCPMetric, @@ -34,7 +34,7 @@ const attributeFCP = (metric: FCPMetric): void => { if (navigationEntry) { const responseStart = navigationEntry.responseStart; - if (invalidTiming(responseStart)) return; + if (isInvalidTimestamp(responseStart)) return; const activationStart = navigationEntry.activationStart || 0; const ttfb = Math.max(0, responseStart - activationStart); diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index fc898de5..b546826b 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -16,7 +16,7 @@ import {getNavigationEntry} from '../lib/getNavigationEntry.js'; import {getSelector} from '../lib/getSelector.js'; -import {invalidTiming} from '../lib/invalidTiming.js'; +import {isInvalidTimestamp} from '../lib/isInvalidTimestamp.js'; import {onLCP as unattributedOnLCP} from '../onLCP.js'; import { LCPAttribution, @@ -33,7 +33,7 @@ const attributeLCP = (metric: LCPMetric) => { if (navigationEntry) { const responseStart = navigationEntry.responseStart; - if (invalidTiming(responseStart)) return; + if (isInvalidTimestamp(responseStart)) return; const activationStart = navigationEntry.activationStart || 0; const lcpEntry = metric.entries[metric.entries.length - 1]; diff --git a/src/lib/invalidTiming.ts b/src/lib/isInvalidTimestamp.ts similarity index 88% rename from src/lib/invalidTiming.ts rename to src/lib/isInvalidTimestamp.ts index 3decdb9b..4311cf71 100644 --- a/src/lib/invalidTiming.ts +++ b/src/lib/isInvalidTimestamp.ts @@ -14,12 +14,12 @@ * limitations under the License. */ -export const invalidTiming = (timing: number) => { +export const isInvalidTimestamp = (timestamp: DOMHighResTimeStamp) => { // In some cases no value is reported by the browser (for // privacy/security reasons), and in other cases (bugs) the value is // negative or is larger than the current page time. Ignore these cases: // https://github.com/GoogleChrome/web-vitals/issues/137 // https://github.com/GoogleChrome/web-vitals/issues/162 // https://github.com/GoogleChrome/web-vitals/issues/275 - return timing <= 0 || timing > performance.now(); + return timestamp <= 0 || timestamp > performance.now(); }; diff --git a/src/onTTFB.ts b/src/onTTFB.ts index dbab4448..5944bfb8 100644 --- a/src/onTTFB.ts +++ b/src/onTTFB.ts @@ -16,7 +16,7 @@ import {bindReporter} from './lib/bindReporter.js'; import {initMetric} from './lib/initMetric.js'; -import {invalidTiming} from './lib/invalidTiming.js'; +import {isInvalidTimestamp} from './lib/isInvalidTimestamp.js'; import {onBFCacheRestore} from './lib/bfcache.js'; import {getNavigationEntry} from './lib/getNavigationEntry.js'; import { @@ -78,7 +78,7 @@ export const onTTFB = (onReport: TTFBReportCallback, opts?: ReportOpts) => { if (navEntry) { const responseStart = navEntry.responseStart; - if (invalidTiming(responseStart)) return; + if (isInvalidTimestamp(responseStart)) return; // The activationStart reference is used because TTFB should be // relative to page activation rather than navigation start if the