Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix reportAllChanges behavior for LCP when library is loaded late #468

Merged
merged 13 commits into from May 3, 2024
Merged

Fix reportAllChanges behavior for LCP when library is loaded late #468

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b8f7059
Allow reportAllChanges for LCP for late loaded script
tunetheweb Apr 25, 2024
4d00dcc
Lock puppeteer as a test
tunetheweb Apr 25, 2024
267d6ad
Even older version
tunetheweb Apr 25, 2024
ef00c5d
Revert puppeteer lock
tunetheweb Apr 25, 2024
a37cbe1
Review feedback
tunetheweb Apr 26, 2024
a6e80b4
Update src/onLCP.ts
philipwalton Apr 29, 2024
bf1983b
Update src/onLCP.ts
philipwalton Apr 29, 2024
7e43fe7
Add console log to see what's going on
tunetheweb May 2, 2024
9f6a11b
More logging
tunetheweb May 2, 2024
49ae047
Explanation
tunetheweb May 2, 2024
232f892
Revert debugging
tunetheweb May 2, 2024
46e2c5e
Missed one
tunetheweb May 2, 2024
aa5371c
Merge branch 'v4' into allow-multiple-lcps-reportallchanges
tunetheweb May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/onLCP.ts
View file
Open in desktop
Expand Up @@ -58,6 +58,12 @@ export const onLCP = (onReport: LCPReportCallback, opts?: ReportOpts) => {
let report: ReturnType<typeof bindReporter>;

const handleEntries = (entries: LCPMetric['entries']) => {
// If reportAllChanges is set then call this function for each entry
philipwalton marked this conversation as resolved.
Show resolved Hide resolved
// As otherwise only want to emit the last one.
philipwalton marked this conversation as resolved.
Show resolved Hide resolved
if (opts!.reportAllChanges && entries.length > 1) {
entries.forEach((entry) => handleEntries([entry]));
tunetheweb marked this conversation as resolved.
Show resolved Hide resolved
}

const lastEntry = entries[entries.length - 1] as LargestContentfulPaint;
if (lastEntry) {
// Only report if the page wasn't hidden prior to LCP.
Expand Down
36 changes: 23 additions & 13 deletions test/e2e/onLCP-test.js
View file
Open in desktop
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down