Skip to content

Commit

Permalink
Revert back to target now that element is no longer stored
Browse files Browse the repository at this point in the history
  • Loading branch information
tunetheweb committed Dec 16, 2024
1 parent 73dfad4 commit 547e630
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 20 deletions.
3 changes: 1 addition & 2 deletions src/attribution/onLCP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,9 @@ const attributeLCP = (metric: LCPMetric): LCPMetricWithAttribution => {
);

attribution = {
target: lcpEntry.element
element: lcpEntry.element
? getSelector(lcpEntry.element)
: lcpTargetMap.get(lcpEntry),
targetElement: lcpEntry.element || undefined,
timeToFirstByte: ttfb,
resourceLoadDelay: lcpRequestStart - ttfb,
resourceLoadDuration: lcpResponseEnd - lcpRequestStart,
Expand Down
6 changes: 1 addition & 5 deletions src/types/lcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ export interface LCPAttribution {
* A selector identifying the element corresponding to the largest contentful paint
* for the page.
*/
target?: string;
/**
* The element corresponding to the largest contentful paint for the page.
*/
targetElement?: Node;
element: string;
/**
* The URL (if applicable) of the LCP image resource. If the LCP element
* is a text node, this value will not be set.
Expand Down
26 changes: 13 additions & 13 deletions test/e2e/onLCP-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.target, 'html>body>main>p>img.bar.foo');
assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo');
assert.equal(
lcp.attribution.timeToFirstByte +
lcp.attribution.resourceLoadDelay +
Expand Down Expand Up @@ -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.target, 'html>body>main>p>img.bar.foo');
assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo');

// Specifically check that resourceLoadDelay falls back to `startTime`.
assert.equal(
Expand Down Expand Up @@ -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.target, 'html>body>main>p>img.bar.foo');
assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo');

// Assert each individual LCP sub-part accounts for `activationStart`
assert.equal(
Expand Down Expand Up @@ -624,7 +624,7 @@ describe('onLCP()', async function () {
const [lcp] = await getBeacons();

assert.equal(lcp.attribution.url, undefined);
assert.equal(lcp.attribution.target, 'html>body>main>h1');
assert.equal(lcp.attribution.element, 'html>body>main>h1');
assert.equal(lcp.attribution.resourceLoadDelay, 0);
assert.equal(lcp.attribution.resourceLoadDuration, 0);
assert.equal(
Expand All @@ -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.target, lcpEntry.target);
assert.equal(lcp.attribution.lcpEntry.element, lcpEntry.element);
assert.equal(lcp.attribution.lcpEntry.size, lcpEntry.size);
assert.equal(lcp.attribution.lcpEntry.startTime, lcpEntry.startTime);
});
Expand Down Expand Up @@ -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.target, undefined);
assert.equal(lcp2.attribution.element, undefined);
assert.equal(lcp2.attribution.timeToFirstByte, 0);
assert.equal(lcp2.attribution.resourceLoadDelay, 0);
assert.equal(lcp2.attribution.resourceLoadDuration, 0);
Expand All @@ -683,7 +683,7 @@ describe('onLCP()', async function () {
assert.equal(lcp2.attribution.lcpEntry, undefined);
});

it('reports the target even when removed (reportAllChanges === false)', async function () {
it('reports the element even when removed (reportAllChanges === false)', async function () {
if (!browserSupportsLCP) this.skip();

await navigateTo('/test/lcp?attribution=1');
Expand All @@ -700,12 +700,12 @@ describe('onLCP()', async function () {

const [lcp] = await getBeacons();
assertStandardReportsAreCorrect([lcp]);
assert.equal(lcp.attribution.target, 'html>body>main>p>img.bar.foo');
assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo');
});

it('reports the target (reportAllChanges === true)', async function () {
// We can't guarantee the order or reporting removed targets with
// reportAllChanges so don't even try. Just test the targets without
it('reports the element (reportAllChanges === true)', async function () {
// We can't guarantee the order or reporting removed element with
// reportAllChanges so don't even try. Just test the elements without
// removal to compare to previous test
if (!browserSupportsLCP) this.skip();

Expand All @@ -721,8 +721,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.target, 'html>body>main>h1');
assert.equal(lcp2.attribution.target, 'html>body>main>p>img.bar.foo');
assert.equal(lcp1.attribution.element, 'html>body>main>h1');
assert.equal(lcp2.attribution.element, 'html>body>main>p>img.bar.foo');
});
});
});
Expand Down

0 comments on commit 547e630

Please sign in to comment.