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.

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

Save INP target after interactions to reduce null values when removed from the DOM #477

Merged
merged 11 commits into from
May 10, 2024
43 changes: 39 additions & 4 deletions src/attribution/onINP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {getSelector} from '../lib/getSelector.js';
import {
longestInteractionList,
entryPreProcessingCallbacks,
longestInteractionMap,
} from '../lib/interactions.js';
import {observe} from '../lib/observe.js';
import {whenIdle} from '../lib/whenIdle.js';
Expand Down Expand Up @@ -65,6 +66,9 @@ const entryToRenderTimeMap: WeakMap<
DOMHighResTimeStamp
> = new WeakMap();

// A mapping of interactions to the target selector
export const interactionTargetMap: Map<number, Node> = new Map();

// A reference to the idle task used to clean up entries from the above
// variables. If the value is -1 it means no task is queue, and if it's
// greater than -1 the value corresponds to the idle callback handle.
Expand All @@ -77,6 +81,18 @@ const handleLoAFEntries = (entries: PerformanceLongAnimationFrameTiming[]) => {
entries.forEach((entry) => pendingLoAFs.push(entry));
};

// Get a reference to the interaction target element in case it's removed
// from the DOM later.
const saveInteractionTarget = (entry: PerformanceEventTiming) => {
if (
entry.interactionId &&
entry.target &&
!interactionTargetMap.has(entry.interactionId)
) {
interactionTargetMap.set(entry.interactionId, entry.target);
}
};

/**
* Groups entries that were presented within the same animation frame by
* a common `renderTime`. This function works by referencing
Expand Down Expand Up @@ -127,14 +143,26 @@ const groupEntriesByRenderTime = (entry: PerformanceEventTiming) => {
if (entry.interactionId || entry.entryType === 'first-input') {
entryToRenderTimeMap.set(entry, renderTime);
}
};

const queueCleanup = () => {
// Queue cleanup of entries that are not part of any INP candidates.
if (idleHandle < 0) {
idleHandle = whenIdle(cleanupEntries);
}
};

const cleanupEntries = () => {
// Delete any stored interaction target elements if they're not part of one
// of the 10 longest interactions.
if (interactionTargetMap.size > 10) {
interactionTargetMap.forEach((_, key) => {
if (!longestInteractionMap.has(key)) {
interactionTargetMap.delete(key);
}
});
}

// The list of previous render times is used to handle cases where
// events are dispatched out of order. When this happens they're generally
// only off by a frame or two, so keeping the most recent 50 should be
Expand Down Expand Up @@ -169,7 +197,11 @@ const cleanupEntries = () => {
idleHandle = -1;
};

entryPreProcessingCallbacks.push(groupEntriesByRenderTime);
entryPreProcessingCallbacks.push(
saveInteractionTarget,
groupEntriesByRenderTime,
queueCleanup,
);

const getIntersectingLoAFs = (
start: DOMHighResTimeStamp,
Expand Down Expand Up @@ -211,7 +243,12 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => {
// first one found in the entry list.
// TODO: when the following bug is fixed just use `firstInteractionEntry`.
// https://bugs.chromium.org/p/chromium/issues/detail?id=1367329
// As a fallback, also check the interactionTargetMap (to account for
// 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) ||
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
Expand All @@ -226,9 +263,7 @@ const attributeINP = (metric: INPMetric): INPMetricWithAttribution => {
const nextPaintTime = Math.max.apply(Math, nextPaintTimeCandidates);

const attribution: INPAttribution = {
interactionTarget: getSelector(
firstEntryWithTarget && firstEntryWithTarget.target,
),
interactionTarget: getSelector(interactionTargetElement),
interactionType: firstEntry.name.startsWith('key') ? 'keyboard' : 'pointer',
interactionTime: firstEntry.startTime,
nextPaintTime: nextPaintTime,
Expand Down
24 changes: 24 additions & 0 deletions test/e2e/onINP-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,30 @@ describe('onINP()', async function () {
assert.equal(inp1.attribution.interactionTarget, 'html>body>main>h1');
});

it('reports the interaction target when target is removed from the DOM', async function () {
if (!browserSupportsINP) this.skip();

await navigateTo('/test/inp?attribution=1&mouseup=100&click=50', {
readyState: 'interactive',
});

const button = await $('#reset');
await simulateUserLikeClick(button);

await nextFrame();
tunetheweb marked this conversation as resolved.
Show resolved Hide resolved

// Remove the element after the interaction.
await browser.execute('document.querySelector("#reset").remove()');

await stubVisibilityChange('hidden');
await beaconCountIs(1);

const [inp] = await getBeacons();

assert.equal(inp.attribution.interactionType, 'pointer');
assert.equal(inp.attribution.interactionTarget, '#reset');
});

it('includes LoAF entries if the browser supports it', async function () {
if (!browserSupportsLoAF) this.skip();

Expand Down
6 changes: 5 additions & 1 deletion test/e2e/onLCP-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,11 @@ const assertStandardReportsAreCorrect = (beacons) => {
const assertFullReportsAreCorrect = (beacons) => {
const [lcp1, lcp2] = beacons;

assert(lcp1.value < 500); // Less than the image load delay.
// Temp fix to address Firefox flakiness.
// See https://github.com/GoogleChrome/web-vitals/issues/472
if (browser.capabilities.browserName !== 'firefox') {
assert(lcp1.value < 500); // Less than the image load delay.
}
assert(lcp1.id.match(/^v4-\d+-\d+$/));
assert.strictEqual(lcp1.name, 'LCP');
assert.strictEqual(lcp1.value, lcp1.delta);
Expand Down