From 7ec1d7e5c2e38aa4c305358ca42e64af3d412b1a Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Wed, 9 Sep 2026 18:29:54 -0400 Subject: [PATCH] fix(browser): Keep the best element name captured for an interaction The INP element-name cache is keyed by the rounded event timestamp and every event of one interaction shares that timestamp, so the last write won. When a click handler swaps out the element under the cursor, the browser then fires `pointerover`/`mouseover` for the new element carrying that same timestamp, and those overwrote the cached name. An INP span for a click that navigated away was named after the post-mutation DOM rather than the element that was clicked. The earliest name in the sequence is the one that describes the element actually interacted with, so it wins. Names that describe nothing are not cached at all: not every event in a sequence has a describable target, and one that doesn't would otherwise claim the timestamp and leave the span named ``. --- packages/browser-utils/src/web-vitals/inp.ts | 19 +++- .../browser-utils/test/web-vitals/inp.test.ts | 98 +++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 packages/browser-utils/test/web-vitals/inp.test.ts diff --git a/packages/browser-utils/src/web-vitals/inp.ts b/packages/browser-utils/src/web-vitals/inp.ts index 04c6e7d86c44..a0f7124fb944 100644 --- a/packages/browser-utils/src/web-vitals/inp.ts +++ b/packages/browser-utils/src/web-vitals/inp.ts @@ -13,6 +13,9 @@ const LAST_INTERACTIONS: number[] = []; const INTERACTIONS_SPAN_MAP = new Map(); // Map to store element names by timestamp, since we get the DOM event before the PerformanceObserver entry +/** What `htmlTreeAsString` returns when it cannot describe the target. */ +const UNKNOWN_ELEMENT_NAME = ''; + const ELEMENT_NAME_TIMESTAMP_MAP = new Map(); /** @@ -84,6 +87,20 @@ export function registerInpInteractionListener(): void { const elementName = htmlTreeAsString(target); const timestamp = Math.round(event.timeStamp); + // Not every event of an interaction has a describable target, and one that doesn't would + // otherwise claim the timestamp and leave the span unnamed. + if (!elementName || elementName === UNKNOWN_ELEMENT_NAME) { + return; + } + + // Every event of one interaction shares a timestamp, and so do the `pointerover`/`mouseover` + // the browser fires afterwards when a handler swaps out the element under the cursor. Those + // arrive last and describe the new DOM, so keeping the first usable name is what pins the entry + // to the element that was actually interacted with. + if (ELEMENT_NAME_TIMESTAMP_MAP.has(timestamp)) { + return; + } + // Store the element name by timestamp so we can match it with the PerformanceEntry ELEMENT_NAME_TIMESTAMP_MAP.set(timestamp, elementName); @@ -114,7 +131,7 @@ export function registerInpInteractionListener(): void { } } - return elementName || ''; + return elementName || UNKNOWN_ELEMENT_NAME; } const handleEntries = ({ entries }: { entries: PerformanceEntry[] }): void => { diff --git a/packages/browser-utils/test/web-vitals/inp.test.ts b/packages/browser-utils/test/web-vitals/inp.test.ts new file mode 100644 index 000000000000..ad21d8c86b42 --- /dev/null +++ b/packages/browser-utils/test/web-vitals/inp.test.ts @@ -0,0 +1,98 @@ +/** + * @vitest-environment jsdom + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const windowListeners = vi.hoisted(() => new Map void>()); +const performanceHandlers = vi.hoisted(() => new Map void>()); + +// `isBrowser()` is false under vitest even with the jsdom environment, and it gates the listeners. +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { ...actual, isBrowser: () => true }; +}); + +vi.mock('../../src/types', () => ({ + WINDOW: { + addEventListener: (type: string, listener: (event: unknown) => void) => windowListeners.set(type, listener), + }, +})); + +vi.mock('../../src/instrumentation/performanceObserver', async () => { + const actual = await vi.importActual('../../src/instrumentation/performanceObserver'); + return { + ...actual, + addPerformanceInstrumentationHandler: (type: string, callback: (data: { entries: unknown[] }) => void) => { + performanceHandlers.set(type, callback); + return () => undefined; + }, + }; +}); + +/** Each test needs a fresh module: the element name cache is per page, so it's module-level. */ +async function loadInp() { + vi.resetModules(); + return import('../../src/web-vitals/inp'); +} + +/** A target `htmlTreeAsString` can describe, versus one it cannot (yields ``). */ +function element(id: string): unknown { + return { tagName: 'DIV', id, nodeType: 1 }; +} + +/** Names the interaction by feeding an entry whose `target` is gone, forcing the timestamp lookup. */ +function nameFor(interactionId: number, startTime: number): void { + performanceHandlers.get('event')?.({ + entries: [{ entryType: 'event', name: 'click', interactionId, startTime, duration: 100, target: null }], + }); +} + +describe('INP element name cache', () => { + beforeEach(() => { + windowListeners.clear(); + performanceHandlers.clear(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('keeps the first name when later events of the same interaction describe a different element', async () => { + const { getCachedInteractionContext, registerInpInteractionListener } = await loadInp(); + registerInpInteractionListener(); + + // The whole sequence shares a timestamp. The trailing `pointerover` is what the browser fires + // once the click handler has swapped out the element under the cursor. + windowListeners.get('click')?.({ target: element('clicked'), timeStamp: 1000 }); + windowListeners.get('pointerover')?.({ target: element('replacement'), timeStamp: 1000 }); + + nameFor(42, 1000); + + expect(getCachedInteractionContext(42)?.elementName).toBe('div#clicked'); + }); + + it('replaces an unresolvable name with a real one from the same interaction', async () => { + const { getCachedInteractionContext, registerInpInteractionListener } = await loadInp(); + registerInpInteractionListener(); + + // Targets that are not elements cannot be described, and they can come first in the sequence. + windowListeners.get('pointerover')?.({ target: {}, timeStamp: 2000 }); + windowListeners.get('click')?.({ target: element('clicked'), timeStamp: 2000 }); + + nameFor(43, 2000); + + expect(getCachedInteractionContext(43)?.elementName).toBe('div#clicked'); + }); + + it('reports an unresolvable name when nothing in the interaction resolves', async () => { + const { getCachedInteractionContext, registerInpInteractionListener } = await loadInp(); + registerInpInteractionListener(); + + windowListeners.get('click')?.({ target: {}, timeStamp: 3000 }); + + nameFor(44, 3000); + + expect(getCachedInteractionContext(44)?.elementName).toBe(''); + }); +});