Skip to content

Commit

Permalink
Save INP target early for when removed from DOM (#477)
Browse files Browse the repository at this point in the history
* Save INP CSS selector early for when target removed from DOM

* Restrict keyboard selector lookups

* Don't hardcode keyboard

* Better comment

* Add test case

* Only do selector lookup on attribution

* Add element

* Fix firefox flakiness

* Update how interaction element is stored

* Update README

* Revert back to using interactionTargetMap

---------

Co-authored-by: Philip Walton <philip@philipwalton.com>
  • Loading branch information
tunetheweb and philipwalton authored May 10, 2024
1 parent 491cfb4 commit 14d3e1d
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 5 deletions.
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();

// 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

0 comments on commit 14d3e1d

Please sign in to comment.