From 56e49c96b57b49c9688d2431211f1df6ba8109ad Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Fri, 3 May 2024 13:22:08 +0100 Subject: [PATCH] Allow reportAllChanges for LCP for late loaded script (#468) * Allow reportAllChanges for LCP for late loaded script * Lock puppeteer as a test * Even older version * Revert puppeteer lock * Review feedback * Update src/onLCP.ts * Update src/onLCP.ts * Add console log to see what's going on * More logging * Explanation * Revert debugging * Missed one --------- Co-authored-by: Philip Walton --- src/onLCP.ts | 20 +++++++++++--------- test/e2e/onLCP-test.js | 36 +++++++++++++++++++++++------------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/onLCP.ts b/src/onLCP.ts index 622c3638..02352209 100644 --- a/src/onLCP.ts +++ b/src/onLCP.ts @@ -58,24 +58,26 @@ export const onLCP = (onReport: LCPReportCallback, opts?: ReportOpts) => { let report: ReturnType; const handleEntries = (entries: LCPMetric['entries']) => { - const lastEntry = entries[entries.length - 1] as LargestContentfulPaint; - if (lastEntry) { + // If reportAllChanges is set then call this function for each entry, + // otherwise only consider the last one. + if (!opts!.reportAllChanges) { + entries = entries.slice(-1); + } + + entries.forEach((entry) => { // Only report if the page wasn't hidden prior to LCP. - if (lastEntry.startTime < visibilityWatcher.firstHiddenTime) { + if (entry.startTime < visibilityWatcher.firstHiddenTime) { // The startTime attribute returns the value of the renderTime if it is // not 0, and the value of the loadTime otherwise. The activationStart // reference is used because LCP should be relative to page activation // rather than navigation start if the page was prerendered. But in cases // where `activationStart` occurs after the LCP, this time should be // clamped at 0. - metric.value = Math.max( - lastEntry.startTime - getActivationStart(), - 0, - ); - metric.entries = [lastEntry]; + metric.value = Math.max(entry.startTime - getActivationStart(), 0); + metric.entries = [entry]; report(); } - } + }); }; const po = observe('largest-contentful-paint', handleEntries); diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index 21107a5e..4f92fb36 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -121,14 +121,24 @@ describe('onLCP()', async function () { // Wait until all images are loaded and fully rendered. await imagesPainted(); - // Load a new page to trigger the hidden state. - await navigateTo('about:blank'); + await beaconCountIs(2); + const [lcp1, lcp2] = await getBeacons(); - // Even though the test sets `reportAllChanges` to true, since the library - // is lazy loaded after all elements have been rendered, only a single - // change will be reported. - await beaconCountIs(1); - assertStandardReportsAreCorrect(await getBeacons()); + assert(lcp1.value > 0); + assert(lcp1.id.match(/^v4-\d+-\d+$/)); + assert.strictEqual(lcp1.name, 'LCP'); + assert.strictEqual(lcp1.value, lcp1.delta); + assert.strictEqual(lcp1.rating, 'good'); + assert.strictEqual(lcp1.entries.length, 1); + assert.strictEqual(lcp1.navigationType, 'navigate'); + + assert(lcp2.value > 500); // Greater than the image load delay. + assert(lcp2.id.match(/^v4-\d+-\d+$/)); + assert.strictEqual(lcp2.name, 'LCP'); + assert(lcp2.value > lcp2.delta); + assert.strictEqual(lcp2.rating, 'good'); + assert.strictEqual(lcp2.entries.length, 1); + assert.strictEqual(lcp2.navigationType, 'navigate'); }); it('accounts for time prerendering the page', async function () { @@ -332,7 +342,7 @@ describe('onLCP()', async function () { const [lcp1] = await getBeacons(); - assert(lcp1.value > 0); // Greater than the image load delay. + assert(lcp1.value > 0); assert(lcp1.id.match(/^v4-\d+-\d+$/)); assert.strictEqual(lcp1.name, 'LCP'); assert.strictEqual(lcp1.value, lcp1.delta); @@ -346,7 +356,7 @@ describe('onLCP()', async function () { const [lcp2] = await getBeacons(); - assert(lcp2.value > 0); // Greater than the image load delay. + assert(lcp2.value > 0); assert(lcp2.id.match(/^v4-\d+-\d+$/)); assert.strictEqual(lcp2.name, 'LCP'); assert.strictEqual(lcp2.value, lcp2.delta); @@ -377,7 +387,7 @@ describe('onLCP()', async function () { const [lcp1] = await getBeacons(); - assert(lcp1.value > 0); // Greater than the image load delay. + assert(lcp1.value > 0); assert(lcp1.id.match(/^v4-\d+-\d+$/)); assert.strictEqual(lcp1.name, 'LCP'); assert.strictEqual(lcp1.value, lcp1.delta); @@ -391,7 +401,7 @@ describe('onLCP()', async function () { const [lcp2] = await getBeacons(); - assert(lcp2.value > 0); // Greater than the image load delay. + assert(lcp2.value > 0); assert(lcp2.id.match(/^v4-\d+-\d+$/)); assert.strictEqual(lcp2.name, 'LCP'); assert.strictEqual(lcp2.value, lcp2.delta); @@ -415,7 +425,7 @@ describe('onLCP()', async function () { const [lcp] = await getBeacons(); - assert(lcp.value > 0); // Greater than the image load delay. + assert(lcp.value > 0); assert(lcp.id.match(/^v4-\d+-\d+$/)); assert.strictEqual(lcp.name, 'LCP'); assert.strictEqual(lcp.value, lcp.delta); @@ -654,7 +664,7 @@ describe('onLCP()', async function () { const [lcp2] = await getBeacons(); - assert(lcp2.value > 0); // Greater than the image load delay. + assert(lcp2.value > 0); assert(lcp2.id.match(/^v4-\d+-\d+$/)); assert.strictEqual(lcp2.name, 'LCP'); assert.strictEqual(lcp2.value, lcp2.delta);