From 1c332a085529c898fda1f020aca9c32e7b2b4198 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Fri, 30 Aug 2024 17:14:49 +0100 Subject: [PATCH 1/5] Cap INP breakdowns to INP duration --- src/attribution/onINP.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/attribution/onINP.ts b/src/attribution/onINP.ts index 09867652..064beb52 100644 --- a/src/attribution/onINP.ts +++ b/src/attribution/onINP.ts @@ -242,7 +242,14 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { const group = entryToEntriesGroupMap.get(firstEntry)!; const processingStart = firstEntry.processingStart; - const processingEnd = group.processingEnd; + // processingEnd can extend beyond duration for modals where we artificially + // mark event timing duration as that paint. + // See: https://github.com/GoogleChrome/web-vitals/issues/492 + // So cap to the INP value. + const processingEnd = Math.min( + group.processingEnd, + firstEntry.startTime + metric.value, + ); // Sort the entries in processing time order. const processedEventEntries = group.entries.sort((a, b) => { @@ -273,7 +280,14 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { longAnimationFrameEntries.map((loaf) => loaf.startTime + loaf.duration), ); - const nextPaintTime = Math.max.apply(Math, nextPaintTimeCandidates); + // processingEnd can extend beyond duration for modals where we artificially + // mark event timing duration as that paint. + // See: https://github.com/GoogleChrome/web-vitals/issues/492 + // So cap to the INP value. + const nextPaintTime = Math.min( + firstEntry.startTime + metric.value, + Math.max.apply(Math, nextPaintTimeCandidates), + ); const attribution: INPAttribution = { interactionTarget: getSelector(interactionTargetElement), From 677cf8e04276a783607a3322ca3da9a75e31f8fa Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Fri, 30 Aug 2024 17:52:00 +0100 Subject: [PATCH 2/5] Better implementation --- src/attribution/onINP.ts | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/attribution/onINP.ts b/src/attribution/onINP.ts index 064beb52..55f20654 100644 --- a/src/attribution/onINP.ts +++ b/src/attribution/onINP.ts @@ -239,6 +239,7 @@ const getIntersectingLoAFs = ( const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { const firstEntry = metric.entries[0]; + const inpTime = firstEntry.startTime + metric.value; const group = entryToEntriesGroupMap.get(firstEntry)!; const processingStart = firstEntry.processingStart; @@ -246,10 +247,8 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { // mark event timing duration as that paint. // See: https://github.com/GoogleChrome/web-vitals/issues/492 // So cap to the INP value. - const processingEnd = Math.min( - group.processingEnd, - firstEntry.startTime + metric.value, - ); + const processingEnd = + group.processingEnd >= inpTime ? inpTime : group.processingEnd; // Sort the entries in processing time order. const processedEventEntries = group.entries.sort((a, b) => { @@ -280,14 +279,11 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { longAnimationFrameEntries.map((loaf) => loaf.startTime + loaf.duration), ); - // processingEnd can extend beyond duration for modals where we artificially - // mark event timing duration as that paint. - // See: https://github.com/GoogleChrome/web-vitals/issues/492 - // So cap to the INP value. - const nextPaintTime = Math.min( - firstEntry.startTime + metric.value, - Math.max.apply(Math, nextPaintTimeCandidates), - ); + const nextPaintTime = Math.max.apply(Math, nextPaintTimeCandidates); + // If processingEnd has been capped to inpTime then presentationDelay is 0 + // Else use the nextPaintTime + const presentationDelay = + processingEnd == inpTime ? 0 : Math.max(nextPaintTime - processingEnd, 0); const attribution: INPAttribution = { interactionTarget: getSelector(interactionTargetElement), @@ -299,7 +295,7 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { longAnimationFrameEntries: longAnimationFrameEntries, inputDelay: processingStart - firstEntry.startTime, processingDuration: processingEnd - processingStart, - presentationDelay: Math.max(nextPaintTime - processingEnd, 0), + presentationDelay: presentationDelay, loadState: getLoadState(firstEntry.startTime), }; From 915c46056ab0761dbbba55a36895af0b65434e15 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Fri, 27 Sep 2024 11:21:52 +0100 Subject: [PATCH 3/5] Review feedback --- src/attribution/onINP.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/attribution/onINP.ts b/src/attribution/onINP.ts index 55f20654..27c8f604 100644 --- a/src/attribution/onINP.ts +++ b/src/attribution/onINP.ts @@ -239,16 +239,17 @@ const getIntersectingLoAFs = ( const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { const firstEntry = metric.entries[0]; - const inpTime = firstEntry.startTime + metric.value; + const endTime = firstEntry.startTime + metric.value; const group = entryToEntriesGroupMap.get(firstEntry)!; const processingStart = firstEntry.processingStart; // processingEnd can extend beyond duration for modals where we artificially - // mark event timing duration as that paint. + // mark event timing duration at the time modal dialog started (and provided + // visual feedback)" // See: https://github.com/GoogleChrome/web-vitals/issues/492 - // So cap to the INP value. + // So cap to the end time. const processingEnd = - group.processingEnd >= inpTime ? inpTime : group.processingEnd; + group.processingEnd >= endTime ? endTime : group.processingEnd; // Sort the entries in processing time order. const processedEventEntries = group.entries.sort((a, b) => { @@ -280,10 +281,10 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { ); const nextPaintTime = Math.max.apply(Math, nextPaintTimeCandidates); - // If processingEnd has been capped to inpTime then presentationDelay is 0 + // If processingEnd has been capped to endTime then presentationDelay is 0 // Else use the nextPaintTime const presentationDelay = - processingEnd == inpTime ? 0 : Math.max(nextPaintTime - processingEnd, 0); + processingEnd == endTime ? 0 : Math.max(nextPaintTime - processingEnd, 0); const attribution: INPAttribution = { interactionTarget: getSelector(interactionTargetElement), From d4b53c63a758d390b426d4390977ab4e2508d650 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Fri, 27 Sep 2024 11:36:01 -0700 Subject: [PATCH 4/5] Simplify nextPaintTime calculation --- src/attribution/onINP.ts | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/src/attribution/onINP.ts b/src/attribution/onINP.ts index 27c8f604..5f594a88 100644 --- a/src/attribution/onINP.ts +++ b/src/attribution/onINP.ts @@ -239,17 +239,16 @@ const getIntersectingLoAFs = ( const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { const firstEntry = metric.entries[0]; - const endTime = firstEntry.startTime + metric.value; - const group = entryToEntriesGroupMap.get(firstEntry)!; + const nextPaintTime = firstEntry.startTime + firstEntry.duration; + const group = entryToEntriesGroupMap.get(firstEntry)!; const processingStart = firstEntry.processingStart; - // processingEnd can extend beyond duration for modals where we artificially - // mark event timing duration at the time modal dialog started (and provided - // visual feedback)" - // See: https://github.com/GoogleChrome/web-vitals/issues/492 - // So cap to the end time. - const processingEnd = - group.processingEnd >= endTime ? endTime : group.processingEnd; + // `processingEnd` can extend beyond the event duration in some cases + // (e.g. sync modals like `alert()`, where duration is when the modal is + // shown, but since it's sync, `processingEnd` is when it's dismissed). + // So for the purposes of INP attribution, processingEnd is capped at + // `nextPaintTime`: https://github.com/GoogleChrome/web-vitals/issues/492 + const processingEnd = Math.min(group.processingEnd, nextPaintTime); // Sort the entries in processing time order. const processedEventEntries = group.entries.sort((a, b) => { @@ -267,25 +266,9 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { // cases where the element is removed from the DOM before reporting happens). const firstEntryWithTarget = metric.entries.find((entry) => entry.target); const interactionTargetElement = - (firstEntryWithTarget && firstEntryWithTarget.target) || + firstEntryWithTarget?.target ?? interactionTargetMap.get(firstEntry.interactionId); - // Since entry durations are rounded to the nearest 8ms, we need to clamp - // the `nextPaintTime` value to be higher than the `processingEnd` or - // end time of any LoAF entry. - const nextPaintTimeCandidates = [ - firstEntry.startTime + firstEntry.duration, - processingEnd, - ].concat( - longAnimationFrameEntries.map((loaf) => loaf.startTime + loaf.duration), - ); - - const nextPaintTime = Math.max.apply(Math, nextPaintTimeCandidates); - // If processingEnd has been capped to endTime then presentationDelay is 0 - // Else use the nextPaintTime - const presentationDelay = - processingEnd == endTime ? 0 : Math.max(nextPaintTime - processingEnd, 0); - const attribution: INPAttribution = { interactionTarget: getSelector(interactionTargetElement), interactionTargetElement: interactionTargetElement, @@ -296,7 +279,7 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { longAnimationFrameEntries: longAnimationFrameEntries, inputDelay: processingStart - firstEntry.startTime, processingDuration: processingEnd - processingStart, - presentationDelay: presentationDelay, + presentationDelay: nextPaintTime - processingEnd, loadState: getLoadState(firstEntry.startTime), }; From 40f37aec18edfcad0273995a09975ad4018a9477 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Fri, 27 Sep 2024 11:54:09 -0700 Subject: [PATCH 5/5] Update code comment --- src/attribution/onINP.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attribution/onINP.ts b/src/attribution/onINP.ts index 5f594a88..434a0864 100644 --- a/src/attribution/onINP.ts +++ b/src/attribution/onINP.ts @@ -246,7 +246,7 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { // `processingEnd` can extend beyond the event duration in some cases // (e.g. sync modals like `alert()`, where duration is when the modal is // shown, but since it's sync, `processingEnd` is when it's dismissed). - // So for the purposes of INP attribution, processingEnd is capped at + // So for the purposes of INP attribution, `processingEnd` is capped at // `nextPaintTime`: https://github.com/GoogleChrome/web-vitals/issues/492 const processingEnd = Math.min(group.processingEnd, nextPaintTime);