From fb03438799678a533cb0ec2e65847e875581209f Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Fri, 12 Apr 2024 14:28:20 +0200 Subject: [PATCH 1/8] [RUM-3684] Capture scroll record on shadow dom elements --- packages/rum/src/domain/record/record.ts | 15 +++-- .../domain/record/shadowRootsController.ts | 24 ++++--- .../rum/src/domain/record/trackers/index.ts | 1 + .../src/domain/record/trackers/trackScroll.ts | 5 +- .../scenario/recorder/shadowDom.scenario.ts | 64 ++++++++++++++++++- 5 files changed, 91 insertions(+), 18 deletions(-) diff --git a/packages/rum/src/domain/record/record.ts b/packages/rum/src/domain/record/record.ts index fd156cdaba..4b4f152727 100644 --- a/packages/rum/src/domain/record/record.ts +++ b/packages/rum/src/domain/record/record.ts @@ -51,10 +51,15 @@ export function record(options: RecordOptions): RecordAPI { const elementsScrollPositions = createElementsScrollPositions() - const shadowRootsController = initShadowRootsController(configuration, { - mutationCb: emitAndComputeStats, - inputCb: emitAndComputeStats, - }) + const shadowRootsController = initShadowRootsController( + configuration, + { + mutationCb: emitAndComputeStats, + inputCb: emitAndComputeStats, + scrollCb: emitAndComputeStats, + }, + elementsScrollPositions + ) const { stop: stopFullSnapshots } = startFullSnapshots( elementsScrollPositions, @@ -76,7 +81,7 @@ export function record(options: RecordOptions): RecordAPI { mutationTracker, trackMove(configuration, emitAndComputeStats), trackMouseInteraction(configuration, emitAndComputeStats, recordIds), - trackScroll(configuration, emitAndComputeStats, elementsScrollPositions), + trackScroll(configuration, emitAndComputeStats, elementsScrollPositions, document), trackViewportResize(configuration, emitAndComputeStats), trackInput(configuration, emitAndComputeStats), trackMediaInteraction(configuration, emitAndComputeStats), diff --git a/packages/rum/src/domain/record/shadowRootsController.ts b/packages/rum/src/domain/record/shadowRootsController.ts index 5a5ad60125..1d8c765a9b 100644 --- a/packages/rum/src/domain/record/shadowRootsController.ts +++ b/packages/rum/src/domain/record/shadowRootsController.ts @@ -1,6 +1,8 @@ import type { RumConfiguration } from '@datadog/browser-rum-core' import type { InputCallback, MutationCallBack } from './trackers' -import { trackInput, trackMutation } from './trackers' +import { trackInput, trackMutation, trackScroll } from './trackers' +import type { ScrollCallback } from './trackers' +import { ElementsScrollPositions } from './elementsScrollPositions' interface ShadowRootController { stop: () => void @@ -16,15 +18,16 @@ export interface ShadowRootsController { flush: () => void } +type ShadowRootControllerCallbacks = { + mutationCb: MutationCallBack + inputCb: InputCallback + scrollCb: ScrollCallback +} + export const initShadowRootsController = ( configuration: RumConfiguration, - { - mutationCb, - inputCb, - }: { - mutationCb: MutationCallBack - inputCb: InputCallback - } + { mutationCb, inputCb, scrollCb }: ShadowRootControllerCallbacks, + elementsScrollPositions: ElementsScrollPositions ): ShadowRootsController => { const controllerByShadowRoot = new Map() @@ -34,13 +37,16 @@ export const initShadowRootsController = ( return } const mutationTracker = trackMutation(mutationCb, configuration, shadowRootsController, shadowRoot) - // the change event no do bubble up across the shadow root, we have to listen on the shadow root + // The change event does not bubble up across the shadow root, we have to listen on the shadow root const inputTracker = trackInput(configuration, inputCb, shadowRoot) + // The scroll event does not bubble up across the shadow root, we have to listen on the shadow root + const scrollTracker = trackScroll(configuration, scrollCb, elementsScrollPositions, shadowRoot) controllerByShadowRoot.set(shadowRoot, { flush: () => mutationTracker.flush(), stop: () => { mutationTracker.stop() inputTracker.stop() + scrollTracker.stop() }, }) }, diff --git a/packages/rum/src/domain/record/trackers/index.ts b/packages/rum/src/domain/record/trackers/index.ts index e0ef802644..4a4fb792f4 100644 --- a/packages/rum/src/domain/record/trackers/index.ts +++ b/packages/rum/src/domain/record/trackers/index.ts @@ -8,5 +8,6 @@ export { trackFocus } from './trackFocus' export { trackFrustration } from './trackFrustration' export { trackViewEnd } from './trackViewEnd' export { InputCallback, trackInput } from './trackInput' +export { ScrollCallback } from './trackScroll' export { trackMutation, MutationCallBack, RumMutationRecord } from './trackMutation' export { Tracker } from './types' diff --git a/packages/rum/src/domain/record/trackers/trackScroll.ts b/packages/rum/src/domain/record/trackers/trackScroll.ts index d06ae8c3a0..b97430612b 100644 --- a/packages/rum/src/domain/record/trackers/trackScroll.ts +++ b/packages/rum/src/domain/record/trackers/trackScroll.ts @@ -18,7 +18,8 @@ export type ScrollCallback = (incrementalSnapshotRecord: BrowserIncrementalSnaps export function trackScroll( configuration: RumConfiguration, scrollCb: ScrollCallback, - elementsScrollPositions: ElementsScrollPositions + elementsScrollPositions: ElementsScrollPositions, + target: Document | ShadowRoot = document ): Tracker { const { throttled: updatePosition, cancel: cancelThrottle } = throttle((event: Event) => { const target = getEventTarget(event) as HTMLElement | Document @@ -50,7 +51,7 @@ export function trackScroll( ) }, SCROLL_OBSERVER_THRESHOLD) - const { stop: removeListener } = addEventListener(configuration, document, DOM_EVENT.SCROLL, updatePosition, { + const { stop: removeListener } = addEventListener(configuration, target, DOM_EVENT.SCROLL, updatePosition, { capture: true, passive: true, }) diff --git a/test/e2e/scenario/recorder/shadowDom.scenario.ts b/test/e2e/scenario/recorder/shadowDom.scenario.ts index 8318727383..0e55f47ccd 100644 --- a/test/e2e/scenario/recorder/shadowDom.scenario.ts +++ b/test/e2e/scenario/recorder/shadowDom.scenario.ts @@ -1,11 +1,17 @@ -import type { DocumentFragmentNode, MouseInteractionData, SerializedNodeWithId } from '@datadog/browser-rum/src/types' -import { MouseInteractionType, NodeType } from '@datadog/browser-rum/src/types' +import type { + DocumentFragmentNode, + MouseInteractionData, + ScrollData, + SerializedNodeWithId, +} from '@datadog/browser-rum/src/types' +import { IncrementalSource, MouseInteractionType, NodeType } from '@datadog/browser-rum/src/types' import { createMutationPayloadValidatorFromSegment, findElementWithIdAttribute, findElementWithTagName, findFullSnapshot, + findIncrementalSnapshot, findMouseInteractionRecords, findNode, findTextContent, @@ -80,6 +86,37 @@ const divShadowDom = ` ` +const scrollableDivShadowDom = ` + ` + /** Will generate the following HTML * ```html * @@ -244,6 +281,29 @@ describe('recorder with shadow DOM', () => { ], }) }) + + createTest('can record scroll from inside the shadow root') + .withRum({}) + .withSetup(bundleSetup) + .withBody(html` + ${scrollableDivShadowDom} + + `) + .run(async ({ intakeRegistry }) => { + const div = await getNodeInsideShadowDom('my-scrollable-div', 'button') + + await div.click() + + await flushEvents() + expect(intakeRegistry.replaySegments.length).toBe(1) + const scrollRecord = findIncrementalSnapshot(intakeRegistry.replaySegments[0], IncrementalSource.Scroll) + const fullSnapshot = findFullSnapshot(intakeRegistry.replaySegments[0])! + const divNode = findElementWithIdAttribute(fullSnapshot.data.node, 'scrollable-div')! + + expect(scrollRecord).toBeTruthy() + expect((scrollRecord?.data as ScrollData).id).toBe(divNode.id) + expect((scrollRecord?.data as ScrollData).y).toBe(250) + }) }) function findElementsInShadowDom(node: SerializedNodeWithId, id: string) { From e15b0c2c2ccacae3191940939e21f74bc94a1085 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Mon, 15 Apr 2024 16:41:18 +0200 Subject: [PATCH 2/8] [RUM-3684] Fix imports --- packages/rum/src/domain/record/shadowRootsController.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/rum/src/domain/record/shadowRootsController.ts b/packages/rum/src/domain/record/shadowRootsController.ts index 1d8c765a9b..4945933183 100644 --- a/packages/rum/src/domain/record/shadowRootsController.ts +++ b/packages/rum/src/domain/record/shadowRootsController.ts @@ -1,8 +1,7 @@ import type { RumConfiguration } from '@datadog/browser-rum-core' -import type { InputCallback, MutationCallBack } from './trackers' +import type { InputCallback, MutationCallBack, ScrollCallback } from './trackers' import { trackInput, trackMutation, trackScroll } from './trackers' -import type { ScrollCallback } from './trackers' -import { ElementsScrollPositions } from './elementsScrollPositions' +import type { ElementsScrollPositions } from './elementsScrollPositions' interface ShadowRootController { stop: () => void From b65fdbdddf8a62506b67c8291c6d54ce88d42528 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Wed, 17 Apr 2024 10:13:26 +0200 Subject: [PATCH 3/8] Simplify API --- packages/rum/src/domain/record/record.ts | 10 +--------- .../src/domain/record/shadowRootsController.ts | 16 +++++----------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/packages/rum/src/domain/record/record.ts b/packages/rum/src/domain/record/record.ts index 4b4f152727..f5b027d1ea 100644 --- a/packages/rum/src/domain/record/record.ts +++ b/packages/rum/src/domain/record/record.ts @@ -51,15 +51,7 @@ export function record(options: RecordOptions): RecordAPI { const elementsScrollPositions = createElementsScrollPositions() - const shadowRootsController = initShadowRootsController( - configuration, - { - mutationCb: emitAndComputeStats, - inputCb: emitAndComputeStats, - scrollCb: emitAndComputeStats, - }, - elementsScrollPositions - ) + const shadowRootsController = initShadowRootsController(configuration, emitAndComputeStats, elementsScrollPositions) const { stop: stopFullSnapshots } = startFullSnapshots( elementsScrollPositions, diff --git a/packages/rum/src/domain/record/shadowRootsController.ts b/packages/rum/src/domain/record/shadowRootsController.ts index 4945933183..065750c46f 100644 --- a/packages/rum/src/domain/record/shadowRootsController.ts +++ b/packages/rum/src/domain/record/shadowRootsController.ts @@ -1,7 +1,7 @@ import type { RumConfiguration } from '@datadog/browser-rum-core' -import type { InputCallback, MutationCallBack, ScrollCallback } from './trackers' import { trackInput, trackMutation, trackScroll } from './trackers' import type { ElementsScrollPositions } from './elementsScrollPositions' +import { BrowserIncrementalSnapshotRecord } from '../../types' interface ShadowRootController { stop: () => void @@ -17,15 +17,9 @@ export interface ShadowRootsController { flush: () => void } -type ShadowRootControllerCallbacks = { - mutationCb: MutationCallBack - inputCb: InputCallback - scrollCb: ScrollCallback -} - export const initShadowRootsController = ( configuration: RumConfiguration, - { mutationCb, inputCb, scrollCb }: ShadowRootControllerCallbacks, + callback: (record: BrowserIncrementalSnapshotRecord) => void, elementsScrollPositions: ElementsScrollPositions ): ShadowRootsController => { const controllerByShadowRoot = new Map() @@ -35,11 +29,11 @@ export const initShadowRootsController = ( if (controllerByShadowRoot.has(shadowRoot)) { return } - const mutationTracker = trackMutation(mutationCb, configuration, shadowRootsController, shadowRoot) + const mutationTracker = trackMutation(callback, configuration, shadowRootsController, shadowRoot) // The change event does not bubble up across the shadow root, we have to listen on the shadow root - const inputTracker = trackInput(configuration, inputCb, shadowRoot) + const inputTracker = trackInput(configuration, callback, shadowRoot) // The scroll event does not bubble up across the shadow root, we have to listen on the shadow root - const scrollTracker = trackScroll(configuration, scrollCb, elementsScrollPositions, shadowRoot) + const scrollTracker = trackScroll(configuration, callback, elementsScrollPositions, shadowRoot) controllerByShadowRoot.set(shadowRoot, { flush: () => mutationTracker.flush(), stop: () => { From ddffb1f331b80960619e75a96052c0398ccbff83 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Wed, 17 Apr 2024 13:25:48 +0200 Subject: [PATCH 4/8] Fix linter issues --- packages/rum/src/domain/record/shadowRootsController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum/src/domain/record/shadowRootsController.ts b/packages/rum/src/domain/record/shadowRootsController.ts index 065750c46f..d505b19b84 100644 --- a/packages/rum/src/domain/record/shadowRootsController.ts +++ b/packages/rum/src/domain/record/shadowRootsController.ts @@ -1,7 +1,7 @@ import type { RumConfiguration } from '@datadog/browser-rum-core' +import type { BrowserIncrementalSnapshotRecord } from '../../types' import { trackInput, trackMutation, trackScroll } from './trackers' import type { ElementsScrollPositions } from './elementsScrollPositions' -import { BrowserIncrementalSnapshotRecord } from '../../types' interface ShadowRootController { stop: () => void From e0ac35c81787b1645fc778bd5540367e052f712d Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Thu, 18 Apr 2024 17:47:59 +0200 Subject: [PATCH 5/8] Add integration test --- packages/rum/src/domain/record/record.spec.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/rum/src/domain/record/record.spec.ts b/packages/rum/src/domain/record/record.spec.ts index f13b824838..579df70567 100644 --- a/packages/rum/src/domain/record/record.spec.ts +++ b/packages/rum/src/domain/record/record.spec.ts @@ -334,6 +334,22 @@ describe('record', () => { expect(inputRecords.length).toBe(1) }) + it('should record the scroll event inside a shadow root', () => { + const div = appendElement('
', createShadow()) as HTMLDivElement + startRecording() + expect(getEmittedRecords().length).toBe(recordsPerFullSnapshot()) + + div.dispatchEvent(createNewEvent('scroll', { target: div, composed: false })) + + recordApi.flushMutations() + const innerMutationData = getLastIncrementalSnapshotData( + getEmittedRecords(), + IncrementalSource.Scroll + ) + + expect(innerMutationData).toBeDefined() + }) + it('should clean the state once the shadow dom is removed to avoid memory leak', () => { const shadowRoot = createShadow() appendElement('
', shadowRoot) From 687a1bd085f884c335988fded1dd3df5aec44d09 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Fri, 19 Apr 2024 10:38:16 +0200 Subject: [PATCH 6/8] Improving tests and commenting choices --- packages/rum/src/domain/record/record.spec.ts | 17 ++++++++++++----- .../scenario/recorder/shadowDom.scenario.ts | 18 ++++++++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/rum/src/domain/record/record.spec.ts b/packages/rum/src/domain/record/record.spec.ts index 579df70567..39826c0bad 100644 --- a/packages/rum/src/domain/record/record.spec.ts +++ b/packages/rum/src/domain/record/record.spec.ts @@ -3,7 +3,7 @@ import type { RumConfiguration, ViewCreatedEvent } from '@datadog/browser-rum-co import { LifeCycle, LifeCycleEventType } from '@datadog/browser-rum-core' import type { Clock } from '@datadog/browser-core/test' import { createNewEvent, collectAsyncCalls } from '@datadog/browser-core/test' -import { findFullSnapshot, findNode, recordsPerFullSnapshot } from '../../../test' +import { findElement, findFullSnapshot, findNode, recordsPerFullSnapshot } from '../../../test' import type { BrowserIncrementalSnapshotRecord, BrowserMutationData, @@ -11,6 +11,7 @@ import type { DocumentFragmentNode, ElementNode, FocusRecord, + ScrollData, } from '../../types' import { NodeType, RecordType, IncrementalSource } from '../../types' import { appendElement } from '../../../../rum-core/test' @@ -334,20 +335,26 @@ describe('record', () => { expect(inputRecords.length).toBe(1) }) - it('should record the scroll event inside a shadow root', () => { - const div = appendElement('
', createShadow()) as HTMLDivElement + fit('should record the scroll event inside a shadow root', () => { + const div = appendElement('
', createShadow()) as HTMLDivElement startRecording() expect(getEmittedRecords().length).toBe(recordsPerFullSnapshot()) div.dispatchEvent(createNewEvent('scroll', { target: div, composed: false })) recordApi.flushMutations() - const innerMutationData = getLastIncrementalSnapshotData( + const innerMutationData = getLastIncrementalSnapshotData( getEmittedRecords(), IncrementalSource.Scroll ) - expect(innerMutationData).toBeDefined() + const fs = findFullSnapshot({ records: getEmittedRecords() })! + const scrollableNode = findElement( + fs.data.node, + (node) => { return node.attributes['unique-selector'] === "enabled" } + )! + + expect(innerMutationData.id).toBe(scrollableNode.id) }) it('should clean the state once the shadow dom is removed to avoid memory leak', () => { diff --git a/test/e2e/scenario/recorder/shadowDom.scenario.ts b/test/e2e/scenario/recorder/shadowDom.scenario.ts index 0e55f47ccd..3d56264ddd 100644 --- a/test/e2e/scenario/recorder/shadowDom.scenario.ts +++ b/test/e2e/scenario/recorder/shadowDom.scenario.ts @@ -86,6 +86,18 @@ const divShadowDom = ` ` +/** Will generate the following HTML + * ```html + * + * #shadow-root + *
+ *
+ *
+ * + *
+ *``` + when called like `` + */ const scrollableDivShadowDom = `