From 5dc824cc671d3923ea1178d652b5e2bd87be8d72 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Wed, 6 Nov 2024 17:22:55 +0000 Subject: [PATCH 01/13] Hold a reference to the LCP element in case it is removed --- src/attribution/onLCP.ts | 20 ++++++++++++++++++-- src/onLCP.ts | 9 +++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index 3f91d68e..78c9cdbc 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -16,7 +16,10 @@ import {getNavigationEntry} from '../lib/getNavigationEntry.js'; import {getSelector} from '../lib/getSelector.js'; -import {onLCP as unattributedOnLCP} from '../onLCP.js'; +import { + onLCP as unattributedOnLCP, + entryPreProcessingCallbacks, +} from '../onLCP.js'; import { LCPAttribution, LCPMetric, @@ -24,6 +27,9 @@ import { ReportOpts, } from '../types.js'; +// A reference to the LCP Target node incase it is removed before reporting +let lcpTargetNode: Node; + const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { // Use a default object if no other attribution has been set. let attribution: LCPAttribution = { @@ -65,7 +71,7 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { ); attribution = { - element: getSelector(lcpEntry.element), + element: getSelector(lcpEntry.element || lcpTargetNode), timeToFirstByte: ttfb, resourceLoadDelay: lcpRequestStart - ttfb, resourceLoadDuration: lcpResponseEnd - lcpRequestStart, @@ -92,6 +98,16 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { return metricWithAttribution; }; +// Get a reference to the LCP target element in case it's removed from the DOM +// later. +const saveLCPTarget = (lcpEntry: LargestContentfulPaint) => { + if (lcpEntry.element) { + lcpTargetNode = lcpEntry.element; + } +}; + +entryPreProcessingCallbacks.push(saveLCPTarget); + /** * Calculates the [LCP](https://web.dev/articles/lcp) value for the current page and * calls the `callback` function once the value is ready (along with the diff --git a/src/onLCP.ts b/src/onLCP.ts index b831119c..25dd02d4 100644 --- a/src/onLCP.ts +++ b/src/onLCP.ts @@ -29,6 +29,12 @@ import {LCPMetric, MetricRatingThresholds, ReportOpts} from './types.js'; /** Thresholds for LCP. See https://web.dev/articles/lcp#what_is_a_good_lcp_score */ export const LCPThresholds: MetricRatingThresholds = [2500, 4000]; +interface EntryPreProcessingHook { + (entry: LargestContentfulPaint): void; +} + +export const entryPreProcessingCallbacks: EntryPreProcessingHook[] = []; + /** * Calculates the [LCP](https://web.dev/articles/lcp) value for the current page and * calls the `callback` function once the value is ready (along with the @@ -57,6 +63,9 @@ export const onLCP = ( } for (const entry of entries) { + for (const cb of entryPreProcessingCallbacks) { + cb(entry); + } // Only report if the page wasn't hidden prior to LCP. if (entry.startTime < visibilityWatcher.firstHiddenTime) { // The startTime attribute returns the value of the renderTime if it is From b1841eb4e01ed007070b9b6dfbd340dd9a5d4601 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Wed, 6 Nov 2024 19:25:20 +0000 Subject: [PATCH 02/13] Switch to WeakMap --- src/attribution/onLCP.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index 78c9cdbc..c451ddc5 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -28,7 +28,7 @@ import { } from '../types.js'; // A reference to the LCP Target node incase it is removed before reporting -let lcpTargetNode: Node; +const lcpTargetMap: WeakMap = new WeakMap(); const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { // Use a default object if no other attribution has been set. @@ -71,7 +71,7 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { ); attribution = { - element: getSelector(lcpEntry.element || lcpTargetNode), + element: getSelector(lcpEntry.element || lcpTargetMap.get(lcpEntry)), timeToFirstByte: ttfb, resourceLoadDelay: lcpRequestStart - ttfb, resourceLoadDuration: lcpResponseEnd - lcpRequestStart, @@ -101,8 +101,8 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { // Get a reference to the LCP target element in case it's removed from the DOM // later. const saveLCPTarget = (lcpEntry: LargestContentfulPaint) => { - if (lcpEntry.element) { - lcpTargetNode = lcpEntry.element; + if (lcpEntry.element && !lcpTargetMap.get(lcpEntry)) { + lcpTargetMap.set(lcpEntry, lcpEntry.element); } }; From 5b8df6f6d49de96a97cf258672c670335dfbb740 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Wed, 6 Nov 2024 20:33:13 +0000 Subject: [PATCH 03/13] Save CLS elements too --- src/attribution/onCLS.ts | 29 +++++++++++++++++++++++++++-- src/attribution/onLCP.ts | 2 +- src/onCLS.ts | 9 +++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/attribution/onCLS.ts b/src/attribution/onCLS.ts index 34e036d7..e6aa3e3e 100644 --- a/src/attribution/onCLS.ts +++ b/src/attribution/onCLS.ts @@ -16,7 +16,10 @@ import {getLoadState} from '../lib/getLoadState.js'; import {getSelector} from '../lib/getSelector.js'; -import {onCLS as unattributedOnCLS} from '../onCLS.js'; +import { + onCLS as unattributedOnCLS, + entryPreProcessingCallbacks, +} from '../onCLS.js'; import { CLSAttribution, CLSMetric, @@ -24,6 +27,10 @@ import { ReportOpts, } from '../types.js'; +// A reference to the LCP Target node incase it is removed before reporting +const layoutShiftTargetMap: WeakMap = + new WeakMap(); + const getLargestLayoutShiftEntry = (entries: LayoutShift[]) => { return entries.reduce((a, b) => (a.value > b.value ? a : b)); }; @@ -42,7 +49,9 @@ const attributeCLS = (metric: CLSMetric): CLSMetricWithAttribution => { const largestSource = getLargestLayoutShiftSource(largestEntry.sources); if (largestSource) { attribution = { - largestShiftTarget: getSelector(largestSource.node), + largestShiftTarget: getSelector( + largestSource.node || layoutShiftTargetMap.get(largestEntry), + ), largestShiftTime: largestEntry.startTime, largestShiftValue: largestEntry.value, largestShiftSource: largestSource, @@ -61,6 +70,22 @@ const attributeCLS = (metric: CLSMetric): CLSMetricWithAttribution => { return metricWithAttribution; }; +// Get a reference to the layout shift target element in case it's removed from the DOM +// later. +const savelayoutShiftLargestTarget = (layoutShiftEntry: LayoutShift) => { + if ( + layoutShiftEntry?.sources && + !layoutShiftTargetMap.get(layoutShiftEntry) + ) { + layoutShiftTargetMap.set( + layoutShiftEntry, + getLargestLayoutShiftSource(layoutShiftEntry.sources)?.node, + ); + } +}; + +entryPreProcessingCallbacks.push(savelayoutShiftLargestTarget); + /** * Calculates the [CLS](https://web.dev/articles/cls) value for the current page and * calls the `callback` function once the value is ready to be reported, along diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index c451ddc5..282328fe 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -71,7 +71,7 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { ); attribution = { - element: getSelector(lcpEntry.element || lcpTargetMap.get(lcpEntry)), + element: getSelector(lcpEntry.element ?? lcpTargetMap.get(lcpEntry)), timeToFirstByte: ttfb, resourceLoadDelay: lcpRequestStart - ttfb, resourceLoadDuration: lcpResponseEnd - lcpRequestStart, diff --git a/src/onCLS.ts b/src/onCLS.ts index 60a30eb6..bce67235 100644 --- a/src/onCLS.ts +++ b/src/onCLS.ts @@ -26,6 +26,12 @@ import {CLSMetric, MetricRatingThresholds, ReportOpts} from './types.js'; /** Thresholds for CLS. See https://web.dev/articles/cls#what_is_a_good_cls_score */ export const CLSThresholds: MetricRatingThresholds = [0.1, 0.25]; +interface EntryPreProcessingHook { + (entry: LayoutShift): void; +} + +export const entryPreProcessingCallbacks: EntryPreProcessingHook[] = []; + /** * Calculates the [CLS](https://web.dev/articles/cls) value for the current page and * calls the `callback` function once the value is ready to be reported, along @@ -65,6 +71,9 @@ export const onCLS = ( for (const entry of entries) { // Only count layout shifts without recent user input. if (!entry.hadRecentInput) { + for (const cb of entryPreProcessingCallbacks) { + cb(entry); + } const firstSessionEntry = sessionEntries[0]; const lastSessionEntry = sessionEntries.at(-1); From ab1cb3adb2034958d434d72a90f00a52ea73909a Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Wed, 6 Nov 2024 21:22:57 +0000 Subject: [PATCH 04/13] Add LCP test --- test/e2e/onLCP-test.js | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index 6983448d..b7f0fc90 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -682,6 +682,48 @@ describe('onLCP()', async function () { assert.equal(lcp2.attribution.lcpResourceEntry, undefined); assert.equal(lcp2.attribution.lcpEntry, undefined); }); + + it('reports the target when removed (reportAllChanges === false)', async function () { + if (!browserSupportsLCP) this.skip(); + + await navigateTo('/test/lcp?attribution=1'); + + // Wait until all images are loaded and fully rendered. + await imagesPainted(); + + await browser.execute(() => { + document.querySelector('img').remove(); + }); + + // Load a new page to trigger the hidden state. + await navigateTo('about:blank'); + + const [lcp] = await getBeacons(); + assertStandardReportsAreCorrect([lcp]); + assert.equal(lcp.attribution.element, 'img.bar.foo'); + }); + + it('reports the target when removed (reportAllChanges === true)', async function () { + if (!browserSupportsLCP) this.skip(); + + await navigateTo('/test/lcp?attribution=1&reportAllChanges=1'); + + // Wait until all images are loaded and fully rendered. + await imagesPainted(); + + await browser.execute(() => { + document.querySelector('img').remove(); + }); + + // Load a new page to trigger the hidden state. + await navigateTo('about:blank'); + + await beaconCountIs(2); + const [lcp1, lcp2] = await getBeacons(); + assertFullReportsAreCorrect([lcp1, lcp2]); + assert.equal(lcp1.attribution.element, 'html>body>main>h1'); + assert.equal(lcp2.attribution.element, 'html>body>main>p>img.bar.foo'); + }); }); }); From 0e940c6042f3b2f1c003adcc3372967c61b35a5c Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Wed, 6 Nov 2024 22:25:36 +0000 Subject: [PATCH 05/13] Add CLS test --- test/e2e/onCLS-test.js | 62 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test/e2e/onCLS-test.js b/test/e2e/onCLS-test.js index 31095521..d28bd0b9 100644 --- a/test/e2e/onCLS-test.js +++ b/test/e2e/onCLS-test.js @@ -835,6 +835,68 @@ describe('onCLS()', async function () { assert.deepEqual(cls.attribution, {}); }); + + it('reports the target when removed (reportAllChanges === false)', async function () { + if (!browserSupportsCLS) this.skip(); + + await navigateTo(`/test/cls?noLayoutShifts=1&attribution=1`, { + readyState: 'complete', + }); + + // Wait until the page is loaded and content is visible before triggering + // a layout shift. + await firstContentfulPaint(); + + await triggerLayoutShift(); + + await browser.execute(() => { + // Remove target element + document.querySelector('h1').remove(); + // Remove other elements too to prevent them reporting + document.querySelector('#p1').remove(); + document.querySelector('#p5').remove(); + }); + + await stubVisibilityChange('hidden'); + + await beaconCountIs(1); + + const [cls] = await getBeacons(); + assert.equal(cls.attribution.largestShiftTarget, 'h1'); + }); + + it('reports the target when removed (reportAllChanges === true)', async function () { + if (!browserSupportsCLS) this.skip(); + + await navigateTo( + `/test/cls?noLayoutShifts=1&attribution=1&reportAllChanges=1`, + { + readyState: 'complete', + }, + ); + + // Wait until the page is loaded and content is visible before triggering + // a layout shift. + await firstContentfulPaint(); + await clearBeacons(); + + await triggerLayoutShift(); + + await browser.execute(() => { + // Remove target element + document.querySelector('h1').remove(); + // Remove other elements too to prevent them reporting + document.querySelector('#p1').remove(); + document.querySelector('#p5').remove(); + }); + + await stubVisibilityChange('hidden'); + + await beaconCountIs(1); + + const [cls] = await getBeacons(); + assert.equal(cls.attribution.largestShiftTarget, 'html>body>main>h1'); + }); }); }); From 930aad0fb0c43c0f539c425a286707e0250845ef Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Wed, 6 Nov 2024 22:51:50 +0000 Subject: [PATCH 06/13] Add INP test and tidy up others --- test/e2e/onCLS-test.js | 18 +++++--------- test/e2e/onINP-test.js | 54 ++++++++++++++++++++++++++++++++++++++++++ test/e2e/onLCP-test.js | 13 +++++----- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/test/e2e/onCLS-test.js b/test/e2e/onCLS-test.js index d28bd0b9..5fd6782f 100644 --- a/test/e2e/onCLS-test.js +++ b/test/e2e/onCLS-test.js @@ -862,10 +862,14 @@ describe('onCLS()', async function () { await beaconCountIs(1); const [cls] = await getBeacons(); + // Note this should be the reduced selector without the full path assert.equal(cls.attribution.largestShiftTarget, 'h1'); }); - it('reports the target when removed (reportAllChanges === true)', async function () { + it('reports the target (reportAllChanges === true)', async function () { + // We can't guarantee the orer or reporting removed targets with + // reportAllChanges so don't even try. Just test the target without + // removal to compare to previous test if (!browserSupportsCLS) this.skip(); await navigateTo( @@ -881,20 +885,10 @@ describe('onCLS()', async function () { await clearBeacons(); await triggerLayoutShift(); - - await browser.execute(() => { - // Remove target element - document.querySelector('h1').remove(); - // Remove other elements too to prevent them reporting - document.querySelector('#p1').remove(); - document.querySelector('#p5').remove(); - }); - - await stubVisibilityChange('hidden'); - await beaconCountIs(1); const [cls] = await getBeacons(); + // Note this should be the full selector with the full path assert.equal(cls.attribution.largestShiftTarget, 'html>body>main>h1'); }); }); diff --git a/test/e2e/onINP-test.js b/test/e2e/onINP-test.js index 80e2175d..e33d673f 100644 --- a/test/e2e/onINP-test.js +++ b/test/e2e/onINP-test.js @@ -715,6 +715,60 @@ describe('onINP()', async function () { const [inp1] = await getBeacons(); assert(inp1.attribution.longAnimationFrameEntries.length > 0); }); + + it('includes target even when removed (reportAllChanges === false)', async function () { + if (!browserSupportsLoAF) this.skip(); + + await navigateTo('/test/inp?attribution=1&pointerdown=100', { + readyState: 'interactive', + }); + + const h1 = await $('h1'); + await simulateUserLikeClick(h1); + + await browser.execute(() => { + // Remove target element + document.querySelector('h1').remove(); + }); + + await nextFrame(); + + await stubVisibilityChange('hidden'); + await beaconCountIs(1); + + const [inp] = await getBeacons(); + // Note this should be the reduced selector without the full path + assert.equal(inp.attribution.interactionTarget, 'h1'); + }); + + it('includes target (reportAllChanges === true)', async function () { + // We can't guarantee the orer or reporting removed targets with + // reportAllChanges so don't even try. Just test the target without + // removal to compare to previous test + if (!browserSupportsLoAF) this.skip(); + + await navigateTo( + '/test/inp?attribution=1&pointerdown=100&reportAllChanges=1', + { + readyState: 'interactive', + }, + ); + + const h1 = await $('h1'); + await simulateUserLikeClick(h1); + + // Can't guarantee order so let's wait for beacon + await beaconCountIs(1); + + await browser.execute(() => { + // Remove target element + document.querySelector('h1').remove(); + }); + + const [inp] = await getBeacons(); + // Note this should be the full selector with the full path + assert.equal(inp.attribution.interactionTarget, 'html>body>main>h1'); + }); }); }); diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index b7f0fc90..d1238525 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -683,7 +683,7 @@ describe('onLCP()', async function () { assert.equal(lcp2.attribution.lcpEntry, undefined); }); - it('reports the target when removed (reportAllChanges === false)', async function () { + it('reports the target even when removed (reportAllChanges === false)', async function () { if (!browserSupportsLCP) this.skip(); await navigateTo('/test/lcp?attribution=1'); @@ -700,10 +700,14 @@ describe('onLCP()', async function () { const [lcp] = await getBeacons(); assertStandardReportsAreCorrect([lcp]); + // Note this should be the reduced selector without the full path assert.equal(lcp.attribution.element, 'img.bar.foo'); }); - it('reports the target when removed (reportAllChanges === true)', async function () { + it('reports the target (reportAllChanges === true)', async function () { + // We can't guarantee the orer or reporting removed targets with + // reportAllChanges so don't even try. Just test the targets without + // removal to compare to previous test if (!browserSupportsLCP) this.skip(); await navigateTo('/test/lcp?attribution=1&reportAllChanges=1'); @@ -711,16 +715,13 @@ describe('onLCP()', async function () { // Wait until all images are loaded and fully rendered. await imagesPainted(); - await browser.execute(() => { - document.querySelector('img').remove(); - }); - // Load a new page to trigger the hidden state. await navigateTo('about:blank'); await beaconCountIs(2); const [lcp1, lcp2] = await getBeacons(); assertFullReportsAreCorrect([lcp1, lcp2]); + // Note these should be the full selectors with the full paths assert.equal(lcp1.attribution.element, 'html>body>main>h1'); assert.equal(lcp2.attribution.element, 'html>body>main>p>img.bar.foo'); }); From e0dac4bfb55309d8d4515299e6d817f0d1c7f5ca Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Wed, 6 Nov 2024 23:40:00 +0000 Subject: [PATCH 07/13] Typos --- src/attribution/onCLS.ts | 4 ++-- test/e2e/onCLS-test.js | 2 +- test/e2e/onINP-test.js | 2 +- test/e2e/onLCP-test.js | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/attribution/onCLS.ts b/src/attribution/onCLS.ts index e6aa3e3e..9157f496 100644 --- a/src/attribution/onCLS.ts +++ b/src/attribution/onCLS.ts @@ -27,7 +27,7 @@ import { ReportOpts, } from '../types.js'; -// A reference to the LCP Target node incase it is removed before reporting +// A reference to the layout shift target node incase it is removed before reporting const layoutShiftTargetMap: WeakMap = new WeakMap(); @@ -50,7 +50,7 @@ const attributeCLS = (metric: CLSMetric): CLSMetricWithAttribution => { if (largestSource) { attribution = { largestShiftTarget: getSelector( - largestSource.node || layoutShiftTargetMap.get(largestEntry), + largestSource.node ?? layoutShiftTargetMap.get(largestEntry), ), largestShiftTime: largestEntry.startTime, largestShiftValue: largestEntry.value, diff --git a/test/e2e/onCLS-test.js b/test/e2e/onCLS-test.js index 5fd6782f..14db3f7c 100644 --- a/test/e2e/onCLS-test.js +++ b/test/e2e/onCLS-test.js @@ -867,7 +867,7 @@ describe('onCLS()', async function () { }); it('reports the target (reportAllChanges === true)', async function () { - // We can't guarantee the orer or reporting removed targets with + // We can't guarantee the order or reporting removed targets with // reportAllChanges so don't even try. Just test the target without // removal to compare to previous test if (!browserSupportsCLS) this.skip(); diff --git a/test/e2e/onINP-test.js b/test/e2e/onINP-test.js index e33d673f..e6cd54a3 100644 --- a/test/e2e/onINP-test.js +++ b/test/e2e/onINP-test.js @@ -742,7 +742,7 @@ describe('onINP()', async function () { }); it('includes target (reportAllChanges === true)', async function () { - // We can't guarantee the orer or reporting removed targets with + // We can't guarantee the order or reporting removed targets with // reportAllChanges so don't even try. Just test the target without // removal to compare to previous test if (!browserSupportsLoAF) this.skip(); diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index d1238525..37d6a8a1 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -705,7 +705,7 @@ describe('onLCP()', async function () { }); it('reports the target (reportAllChanges === true)', async function () { - // We can't guarantee the orer or reporting removed targets with + // We can't guarantee the order or reporting removed targets with // reportAllChanges so don't even try. Just test the targets without // removal to compare to previous test if (!browserSupportsLCP) this.skip(); From a48ded1c0dc9e47c8f48a492ac5747f5eb03d107 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 7 Nov 2024 13:06:00 +0000 Subject: [PATCH 08/13] Add LCP and CLS node --- CHANGELOG.md | 1 + README.md | 9 +++++++-- src/attribution/onCLS.ts | 2 ++ src/attribution/onLCP.ts | 1 + src/types/cls.ts | 6 ++++++ src/types/lcp.ts | 7 ++++++- 6 files changed, 23 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39869a78..e6066a18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - **[BREAKING]** Remove the deprecated `onFID()` function ([#519](https://github.com/GoogleChrome/web-vitals/pull/519)) - **[BREAKING]** Change browser support policy to Baseline Widely Available ([#525](https://github.com/GoogleChrome/web-vitals/pull/525)) - **[BREAKING]** Sort the classes that appear in attribution selectors to reduce cardinality ([#518](https://github.com/GoogleChrome/web-vitals/pull/518)) +- LCP and CLS attribution now saves the selector and element for when the element is removed from the DOM ([#562](https://github.com/GoogleChrome/web-vitals/pull/562)) - Cap INP breakdowns to INP duration ([#528](https://github.com/GoogleChrome/web-vitals/pull/528)) - Cap LCP load duration to LCP time ([#527](https://github.com/GoogleChrome/web-vitals/pull/527)) diff --git a/README.md b/README.md index 55c033c1..87905533 100644 --- a/README.md +++ b/README.md @@ -351,7 +351,7 @@ function sendToGoogleAnalytics({name, delta, value, id, attribution}) { eventParams.debug_target = attribution.interactionTarget; break; case 'LCP': - eventParams.debug_target = attribution.element; + eventParams.debug_target = attribution.elementSelector; break; } @@ -926,10 +926,15 @@ interface INPAttribution { ```ts interface LCPAttribution { + /** + * The selecotr of the element corresponding to the largest contentful paint + * for the page. + */ + elementSelector?: string; /** * The element corresponding to the largest contentful paint for the page. */ - element?: string; + element?: Node; /** * The URL (if applicable) of the LCP image resource. If the LCP element * is a text node, this value will not be set. diff --git a/src/attribution/onCLS.ts b/src/attribution/onCLS.ts index 9157f496..fb467197 100644 --- a/src/attribution/onCLS.ts +++ b/src/attribution/onCLS.ts @@ -52,6 +52,8 @@ const attributeCLS = (metric: CLSMetric): CLSMetricWithAttribution => { largestShiftTarget: getSelector( largestSource.node ?? layoutShiftTargetMap.get(largestEntry), ), + largestShiftTargetElement: + largestSource.node ?? layoutShiftTargetMap.get(largestEntry), largestShiftTime: largestEntry.startTime, largestShiftValue: largestEntry.value, largestShiftSource: largestSource, diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index 282328fe..ababf57f 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -72,6 +72,7 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { attribution = { element: getSelector(lcpEntry.element ?? lcpTargetMap.get(lcpEntry)), + elementNode: lcpEntry.element ?? lcpTargetMap.get(lcpEntry), timeToFirstByte: ttfb, resourceLoadDelay: lcpRequestStart - ttfb, resourceLoadDuration: lcpResponseEnd - lcpRequestStart, diff --git a/src/types/cls.ts b/src/types/cls.ts index c79ce4c0..b4d0ac1b 100644 --- a/src/types/cls.ts +++ b/src/types/cls.ts @@ -36,6 +36,12 @@ export interface CLSAttribution { * CLS score occurred. */ largestShiftTarget?: string; + /** + * The first element (in document order) that shifted when the single + * largest layout shift contributing to the page's + * CLS score occurred. + */ + largestShiftTargetElement?: Node; /** * The time when the single largest layout shift contributing to the page's * CLS score occurred. diff --git a/src/types/lcp.ts b/src/types/lcp.ts index 4761fdd1..9f5b3bd2 100644 --- a/src/types/lcp.ts +++ b/src/types/lcp.ts @@ -31,9 +31,14 @@ export interface LCPMetric extends Metric { */ export interface LCPAttribution { /** - * The element corresponding to the largest contentful paint for the page. + * The selector of the element corresponding to the largest contentful paint + * for the page. */ element?: string; + /** + * The element corresponding to the largest contentful paint for the page. + */ + elementNode?: Node; /** * The URL (if applicable) of the LCP image resource. If the LCP element * is a text node, this value will not be set. From 537359348a1a05fbb967de074df7345347049e7e Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 7 Nov 2024 13:07:27 +0000 Subject: [PATCH 09/13] README changes --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 87905533..14fac360 100644 --- a/README.md +++ b/README.md @@ -351,7 +351,7 @@ function sendToGoogleAnalytics({name, delta, value, id, attribution}) { eventParams.debug_target = attribution.interactionTarget; break; case 'LCP': - eventParams.debug_target = attribution.elementSelector; + eventParams.debug_target = attribution.element; break; } @@ -930,11 +930,11 @@ interface LCPAttribution { * The selecotr of the element corresponding to the largest contentful paint * for the page. */ - elementSelector?: string; + element?: string; /** * The element corresponding to the largest contentful paint for the page. */ - element?: Node; + elementNode?: Node; /** * The URL (if applicable) of the LCP image resource. If the LCP element * is a text node, this value will not be set. From 81bc2dfdefbec1ce44576d79ed54771812d3bfe1 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Mon, 18 Nov 2024 11:28:00 +0000 Subject: [PATCH 10/13] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 14fac360..24289dee 100644 --- a/README.md +++ b/README.md @@ -927,7 +927,7 @@ interface INPAttribution { ```ts interface LCPAttribution { /** - * The selecotr of the element corresponding to the largest contentful paint + * The selector of the element corresponding to the largest contentful paint * for the page. */ element?: string; From 18ff5da6ddbea788e530f79dc1363b8968854488 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Tue, 10 Dec 2024 12:02:21 +0000 Subject: [PATCH 11/13] Apply suggestions from code review Co-authored-by: Philip Walton --- README.md | 2 +- src/attribution/onCLS.ts | 2 +- src/attribution/onLCP.ts | 2 +- src/types/lcp.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 24289dee..13fdbaf9 100644 --- a/README.md +++ b/README.md @@ -927,7 +927,7 @@ interface INPAttribution { ```ts interface LCPAttribution { /** - * The selector of the element corresponding to the largest contentful paint + * A selector identifying the element corresponding to the largest contentful paint * for the page. */ element?: string; diff --git a/src/attribution/onCLS.ts b/src/attribution/onCLS.ts index fb467197..e3404a8d 100644 --- a/src/attribution/onCLS.ts +++ b/src/attribution/onCLS.ts @@ -27,7 +27,7 @@ import { ReportOpts, } from '../types.js'; -// A reference to the layout shift target node incase it is removed before reporting +// A reference to the layout shift target node in case it's removed before reporting. const layoutShiftTargetMap: WeakMap = new WeakMap(); diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index ababf57f..69c271ca 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -27,7 +27,7 @@ import { ReportOpts, } from '../types.js'; -// A reference to the LCP Target node incase it is removed before reporting +// A reference to the LCP target node in case it's removed before reporting. const lcpTargetMap: WeakMap = new WeakMap(); const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { diff --git a/src/types/lcp.ts b/src/types/lcp.ts index 9f5b3bd2..e265495c 100644 --- a/src/types/lcp.ts +++ b/src/types/lcp.ts @@ -31,7 +31,7 @@ export interface LCPMetric extends Metric { */ export interface LCPAttribution { /** - * The selector of the element corresponding to the largest contentful paint + * A selector identifying the element corresponding to the largest contentful paint * for the page. */ element?: string; From 63846c8ccc2efa1aaf1acd679b19a075368e5d77 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Tue, 10 Dec 2024 12:12:02 +0000 Subject: [PATCH 12/13] LCP element -> target --- README.md | 6 +++--- src/attribution/onLCP.ts | 4 ++-- src/types/lcp.ts | 4 ++-- test/e2e/onLCP-test.js | 18 +++++++++--------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 5bd69307..3cb8d5de 100644 --- a/README.md +++ b/README.md @@ -351,7 +351,7 @@ function sendToGoogleAnalytics({name, delta, value, id, attribution}) { eventParams.debug_target = attribution.interactionTarget; break; case 'LCP': - eventParams.debug_target = attribution.element; + eventParams.debug_target = attribution.target; break; } @@ -930,11 +930,11 @@ interface LCPAttribution { * A selector identifying the element corresponding to the largest contentful paint * for the page. */ - element?: string; + target?: string; /** * The element corresponding to the largest contentful paint for the page. */ - elementNode?: Node; + targetElement?: Node; /** * The URL (if applicable) of the LCP image resource. If the LCP element * is a text node, this value will not be set. diff --git a/src/attribution/onLCP.ts b/src/attribution/onLCP.ts index 69c271ca..2f6eaa73 100644 --- a/src/attribution/onLCP.ts +++ b/src/attribution/onLCP.ts @@ -71,8 +71,8 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => { ); attribution = { - element: getSelector(lcpEntry.element ?? lcpTargetMap.get(lcpEntry)), - elementNode: lcpEntry.element ?? lcpTargetMap.get(lcpEntry), + target: getSelector(lcpEntry.element ?? lcpTargetMap.get(lcpEntry)), + targetElement: lcpEntry.element ?? lcpTargetMap.get(lcpEntry), timeToFirstByte: ttfb, resourceLoadDelay: lcpRequestStart - ttfb, resourceLoadDuration: lcpResponseEnd - lcpRequestStart, diff --git a/src/types/lcp.ts b/src/types/lcp.ts index e265495c..2f00abf0 100644 --- a/src/types/lcp.ts +++ b/src/types/lcp.ts @@ -34,11 +34,11 @@ export interface LCPAttribution { * A selector identifying the element corresponding to the largest contentful paint * for the page. */ - element?: string; + target?: string; /** * The element corresponding to the largest contentful paint for the page. */ - elementNode?: Node; + targetElement?: Node; /** * The URL (if applicable) of the LCP image resource. If the LCP element * is a text node, this value will not be set. diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index 37d6a8a1..19f1fe2c 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -465,7 +465,7 @@ describe('onLCP()', async function () { assertStandardReportsAreCorrect([lcp]); assert(lcp.attribution.url.endsWith('/test/img/square.png?delay=500')); - assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo'); + assert.equal(lcp.attribution.target, 'html>body>main>p>img.bar.foo'); assert.equal( lcp.attribution.timeToFirstByte + lcp.attribution.resourceLoadDelay + @@ -515,7 +515,7 @@ describe('onLCP()', async function () { assertStandardReportsAreCorrect([lcp]); assert(lcp.attribution.url.endsWith('/test/img/square.png?delay=500')); - assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo'); + assert.equal(lcp.attribution.target, 'html>body>main>p>img.bar.foo'); // Specifically check that resourceLoadDelay falls back to `startTime`. assert.equal( @@ -565,7 +565,7 @@ describe('onLCP()', async function () { assert(lcp.attribution.url.endsWith('/test/img/square.png?delay=500')); assert.equal(lcp.navigationType, 'prerender'); - assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo'); + assert.equal(lcp.attribution.target, 'html>body>main>p>img.bar.foo'); // Assert each individual LCP sub-part accounts for `activationStart` assert.equal( @@ -624,7 +624,7 @@ describe('onLCP()', async function () { const [lcp] = await getBeacons(); assert.equal(lcp.attribution.url, undefined); - assert.equal(lcp.attribution.element, 'html>body>main>h1'); + assert.equal(lcp.attribution.target, 'html>body>main>h1'); assert.equal(lcp.attribution.resourceLoadDelay, 0); assert.equal(lcp.attribution.resourceLoadDuration, 0); assert.equal( @@ -641,7 +641,7 @@ describe('onLCP()', async function () { // Deep equal won't work since some of the properties are removed before // sending to /collect, so just compare some. const lcpEntry = lcp.entries.slice(-1)[0]; - assert.equal(lcp.attribution.lcpEntry.element, lcpEntry.element); + assert.equal(lcp.attribution.lcpEntry.target, lcpEntry.target); assert.equal(lcp.attribution.lcpEntry.size, lcpEntry.size); assert.equal(lcp.attribution.lcpEntry.startTime, lcpEntry.startTime); }); @@ -673,7 +673,7 @@ describe('onLCP()', async function () { assert.strictEqual(lcp2.entries.length, 0); assert.strictEqual(lcp2.navigationType, 'back-forward-cache'); - assert.equal(lcp2.attribution.element, undefined); + assert.equal(lcp2.attribution.target, undefined); assert.equal(lcp2.attribution.timeToFirstByte, 0); assert.equal(lcp2.attribution.resourceLoadDelay, 0); assert.equal(lcp2.attribution.resourceLoadDuration, 0); @@ -701,7 +701,7 @@ describe('onLCP()', async function () { const [lcp] = await getBeacons(); assertStandardReportsAreCorrect([lcp]); // Note this should be the reduced selector without the full path - assert.equal(lcp.attribution.element, 'img.bar.foo'); + assert.equal(lcp.attribution.target, 'img.bar.foo'); }); it('reports the target (reportAllChanges === true)', async function () { @@ -722,8 +722,8 @@ describe('onLCP()', async function () { const [lcp1, lcp2] = await getBeacons(); assertFullReportsAreCorrect([lcp1, lcp2]); // Note these should be the full selectors with the full paths - assert.equal(lcp1.attribution.element, 'html>body>main>h1'); - assert.equal(lcp2.attribution.element, 'html>body>main>p>img.bar.foo'); + assert.equal(lcp1.attribution.target, 'html>body>main>h1'); + assert.equal(lcp2.attribution.target, 'html>body>main>p>img.bar.foo'); }); }); }); From 5ae530aa3edc7b449a211d3ac3a46027fe7d8054 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Tue, 10 Dec 2024 12:15:38 +0000 Subject: [PATCH 13/13] Update changelog --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6066a18..37b8aa57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,15 @@ # Changelog +### v5.0.0-rc.1 (???) + +- **[BREAKING]** LCP `element` renamed to `target` ([#562](https://github.com/GoogleChrome/web-vitals/pull/562)) +- LCP and CLS attribution now saves the target selector and element for when the element is removed from the DOM ([#562](https://github.com/GoogleChrome/web-vitals/pull/562)) + ### v5.0.0-rc.0 (2024-10-03) - **[BREAKING]** Remove the deprecated `onFID()` function ([#519](https://github.com/GoogleChrome/web-vitals/pull/519)) - **[BREAKING]** Change browser support policy to Baseline Widely Available ([#525](https://github.com/GoogleChrome/web-vitals/pull/525)) - **[BREAKING]** Sort the classes that appear in attribution selectors to reduce cardinality ([#518](https://github.com/GoogleChrome/web-vitals/pull/518)) -- LCP and CLS attribution now saves the selector and element for when the element is removed from the DOM ([#562](https://github.com/GoogleChrome/web-vitals/pull/562)) - Cap INP breakdowns to INP duration ([#528](https://github.com/GoogleChrome/web-vitals/pull/528)) - Cap LCP load duration to LCP time ([#527](https://github.com/GoogleChrome/web-vitals/pull/527))