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(''); + }); +});