From e7ff349d1cf6b7b9a0d691ccfb2f0c243fc7f94e Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 18 Nov 2024 11:19:08 +0000 Subject: [PATCH 1/6] fix: angular change detection mutation observer --- eslint-rules/no-direct-mutation-observer.js | 58 ++++++++++++++++++ src/entrypoints/dead-clicks-autocapture.ts | 21 ++++++- src/utils/globals.ts | 19 ++++-- src/utils/prototype-utils.ts | 67 +++++++++++++++++++++ src/utils/type-utils.ts | 16 +++++ 5 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 eslint-rules/no-direct-mutation-observer.js create mode 100644 src/utils/prototype-utils.ts diff --git a/eslint-rules/no-direct-mutation-observer.js b/eslint-rules/no-direct-mutation-observer.js new file mode 100644 index 000000000..0f4e7c3f6 --- /dev/null +++ b/eslint-rules/no-direct-mutation-observer.js @@ -0,0 +1,58 @@ +module.exports = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow direct use of MutationObserver and enforce importing NativeMutationObserver from global.ts', + category: 'Best Practices', + recommended: false, + }, + schema: [], + messages: { + noDirectMutationObserver: + 'Direct use of MutationObserver is not allowed. Use NativeMutationObserver from global.ts instead.', + missingNativeMutationObserver: + 'You must import NativeMutationObserver from global.ts to use MutationObserver functionality.', + }, + }, + create(context) { + let importedNativeMutationObserver = false + const targetFileName = 'utils/global' + + return { + ImportDeclaration(node) { + // Check if 'NativeMutationObserver' is imported from 'global.ts' + if (node.source.value.includes(targetFileName)) { + const importedSpecifiers = node.specifiers.map( + (specifier) => specifier.imported && specifier.imported.name + ) + if (importedSpecifiers.includes('NativeMutationObserver')) { + importedNativeMutationObserver = true + } + } + }, + NewExpression(node) { + // Check if `MutationObserver` is used + if (node.callee.type === 'Identifier' && node.callee.name === 'MutationObserver') { + if (!importedNativeMutationObserver) { + context.report({ + node, + messageId: 'noDirectMutationObserver', + }) + } + } + }, + Identifier(node) { + // Warn if `MutationObserver` is directly referenced outside of a `new` expression (rare cases) + if (node.name === 'MutationObserver' && node.parent.type !== 'NewExpression') { + if (!importedNativeMutationObserver) { + context.report({ + node, + messageId: 'missingNativeMutationObserver', + }) + } + } + }, + } + }, +} diff --git a/src/entrypoints/dead-clicks-autocapture.ts b/src/entrypoints/dead-clicks-autocapture.ts index bb4c5d1a1..5e2b9ba00 100644 --- a/src/entrypoints/dead-clicks-autocapture.ts +++ b/src/entrypoints/dead-clicks-autocapture.ts @@ -5,6 +5,7 @@ import { autocaptureCompatibleElements, getEventTarget } from '../autocapture-ut import { DeadClickCandidate, DeadClicksAutoCaptureConfig, Properties } from '../types' import { autocapturePropertiesForElement } from '../autocapture' import { isElementInToolbar, isElementNode, isTag } from '../utils/element-utils' +import { NativeMutationObserver } from '../utils/globals' function asClick(event: MouseEvent): DeadClickCandidate | null { const eventTarget = getEventTarget(event) @@ -66,7 +67,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture private _startMutationObserver(observerTarget: Node) { if (!this._mutationObserver) { - this._mutationObserver = new MutationObserver((mutations) => { + this._mutationObserver = new NativeMutationObserver((mutations) => { this.onMutation(mutations) }) this._mutationObserver.observe(observerTarget, { @@ -99,6 +100,8 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture private _onClick = (event: MouseEvent): void => { const click = asClick(event) if (!isNull(click) && !this._ignoreClick(click)) { + // eslint-disable-next-line no-console + console.log('deadclicks detected a click', click) this._clicks.push(click) } @@ -209,9 +212,25 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture if (hadScroll || hadMutation || hadSelectionChange) { // ignore clicks that had a scroll or mutation + // eslint-disable-next-line no-console + console.log('not a dead click', { + click, + hadScroll, + hadMutation, + hadSelectionChange, + }) continue } + // eslint-disable-next-line no-console + console.log('dead click?', { + isdc: scrollTimeout || mutationTimeout || absoluteTimeout || selectionChangedTimeout, + scrollTimeout, + mutationTimeout, + absoluteTimeout, + selectionChangedTimeout, + }) + if (scrollTimeout || mutationTimeout || absoluteTimeout || selectionChangedTimeout) { this._onCapture(click, { $dead_click_last_mutation_timestamp: this._lastMutation, diff --git a/src/utils/globals.ts b/src/utils/globals.ts index e23e10365..76ce214ad 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -2,6 +2,7 @@ import { ErrorProperties } from '../extensions/exception-autocapture/error-conve import type { PostHog } from '../posthog-core' import { SessionIdManager } from '../sessionid' import { DeadClicksAutoCaptureConfig, ErrorEventArgs, ErrorMetadata, Properties } from '../types' +import { getNativeMutationObserverImplementation } from './prototype-utils' /* * Global helpers to protect access to browser globals in a way that is safer for different targets @@ -16,6 +17,12 @@ import { DeadClicksAutoCaptureConfig, ErrorEventArgs, ErrorMetadata, Properties // eslint-disable-next-line no-restricted-globals const win: (Window & typeof globalThis) | undefined = typeof window !== 'undefined' ? window : undefined +export type AssignableWindow = Window & + typeof globalThis & + Record & { + __PosthogExtensions__?: PostHogExtensions + } + /** * This is our contract between (potentially) lazily loaded extensions and the SDK * changes to this interface can be breaking changes for users of the SDK @@ -86,10 +93,10 @@ export const XMLHttpRequest = global?.XMLHttpRequest && 'withCredentials' in new global.XMLHttpRequest() ? global.XMLHttpRequest : undefined export const AbortController = global?.AbortController export const userAgent = navigator?.userAgent -export const assignableWindow: Window & - typeof globalThis & - Record & { - __PosthogExtensions__?: PostHogExtensions - } = win ?? ({} as any) - +export const assignableWindow: AssignableWindow = win ?? ({} as any) +/** + * We have to sometimes load mutation observer from an iframe + * because Angular change detection really doesn't like sharing it + */ +export const NativeMutationObserver = getNativeMutationObserverImplementation(assignableWindow) export { win as window } diff --git a/src/utils/prototype-utils.ts b/src/utils/prototype-utils.ts new file mode 100644 index 000000000..77115a83a --- /dev/null +++ b/src/utils/prototype-utils.ts @@ -0,0 +1,67 @@ +/** + * adapted from https://github.com/getsentry/sentry-javascript/blob/72751dacb88c5b970d8bac15052ee8e09b28fd5d/packages/browser-utils/src/getNativeImplementation.ts#L27 + * and https://github.com/PostHog/rrweb/blob/804380afbb1b9bed70b8792cb5a25d827f5c0cb5/packages/utils/src/index.ts#L31 + * after a number of performance reports from Angular users + */ + +import { AssignableWindow } from './globals' +import { isAngularZonePatchedFunction, isFunction, isNativeFunction } from './type-utils' +import { logger } from './logger' + +interface NativeImplementationsCache { + // eslint-disable-next-line posthog-js/no-direct-mutation-observer + MutationObserver: typeof MutationObserver + setTimeout: typeof setTimeout + addEventListener: typeof addEventListener + setInterval: typeof setInterval +} + +const cachedImplementations: Partial = {} + +export function getNativeImplementation( + name: T, + assignableWindow: AssignableWindow +): NativeImplementationsCache[T] { + const cached = cachedImplementations[name] + if (cached) { + return cached + } + + let impl = assignableWindow[name] as NativeImplementationsCache[T] + + if (isNativeFunction(impl) && !isAngularZonePatchedFunction(impl)) { + // eslint-disable-next-line no-console + console.log(name + ' is a native function, no need to create a sandbox') + return (cachedImplementations[name] = impl.bind(assignableWindow) as NativeImplementationsCache[T]) + } + + const document = assignableWindow.document + if (document && isFunction(document.createElement)) { + try { + const sandbox = document.createElement('iframe') + sandbox.hidden = true + document.head.appendChild(sandbox) + const contentWindow = sandbox.contentWindow + if (contentWindow && (contentWindow as any)[name]) { + impl = (contentWindow as any)[name] as NativeImplementationsCache[T] + } + document.head.removeChild(sandbox) + } catch (e) { + // Could not create sandbox iframe, just use assignableWindow.xxx + logger.warn(`Could not create sandbox iframe for ${name} check, bailing to assignableWindow.${name}: `, e) + } + } + + // Sanity check: This _should_ not happen, but if it does, we just skip caching... + // This can happen e.g. in tests where fetch may not be available in the env, or similar. + if (!impl || !isFunction(impl)) { + return impl + } + + return (cachedImplementations[name] = impl.bind(assignableWindow) as NativeImplementationsCache[T]) +} + +// eslint-disable-next-line posthog-js/no-direct-mutation-observer +export function getNativeMutationObserverImplementation(assignableWindow: AssignableWindow): typeof MutationObserver { + return getNativeImplementation('MutationObserver', assignableWindow) +} diff --git a/src/utils/type-utils.ts b/src/utils/type-utils.ts index 2d7192f23..b147d736b 100644 --- a/src/utils/type-utils.ts +++ b/src/utils/type-utils.ts @@ -19,6 +19,22 @@ export const isFunction = function (f: any): f is (...args: any[]) => any { // eslint-disable-next-line posthog-js/no-direct-function-check return typeof f === 'function' } + +export const isNativeFunction = function (f: any): f is (...args: any[]) => any { + // eslint-disable-next-line no-console + console.log(f.toString()) + return isFunction(f) && f.toString().includes('[native code]') +} + +// When angular patches functions they pass the above `isNativeFunction` check +export const isAngularZonePatchedFunction = function (f: any): boolean { + if (!isFunction(f)) { + return false + } + const prototypeKeys = Object.getOwnPropertyNames(f.prototype || {}) + return prototypeKeys.some((key) => key.indexOf('__zone')) +} + // Underscore Addons export const isObject = function (x: unknown): x is Record { // eslint-disable-next-line posthog-js/no-direct-object-check From 81a7ca6b2ff1785ac37560a833be836de528ed90 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 18 Nov 2024 11:25:54 +0000 Subject: [PATCH 2/6] fix --- src/utils/type-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/type-utils.ts b/src/utils/type-utils.ts index b147d736b..2dd888e59 100644 --- a/src/utils/type-utils.ts +++ b/src/utils/type-utils.ts @@ -23,7 +23,7 @@ export const isFunction = function (f: any): f is (...args: any[]) => any { export const isNativeFunction = function (f: any): f is (...args: any[]) => any { // eslint-disable-next-line no-console console.log(f.toString()) - return isFunction(f) && f.toString().includes('[native code]') + return isFunction(f) && f.toString().indexOf('[native code]') !== -1 } // When angular patches functions they pass the above `isNativeFunction` check From 921d2e76c56264aef1fccb9cb1b5f41292244d72 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 18 Nov 2024 11:33:55 +0000 Subject: [PATCH 3/6] fix --- src/entrypoints/dead-clicks-autocapture.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/entrypoints/dead-clicks-autocapture.ts b/src/entrypoints/dead-clicks-autocapture.ts index 5e2b9ba00..a372bf563 100644 --- a/src/entrypoints/dead-clicks-autocapture.ts +++ b/src/entrypoints/dead-clicks-autocapture.ts @@ -100,8 +100,6 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture private _onClick = (event: MouseEvent): void => { const click = asClick(event) if (!isNull(click) && !this._ignoreClick(click)) { - // eslint-disable-next-line no-console - console.log('deadclicks detected a click', click) this._clicks.push(click) } @@ -212,25 +210,9 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture if (hadScroll || hadMutation || hadSelectionChange) { // ignore clicks that had a scroll or mutation - // eslint-disable-next-line no-console - console.log('not a dead click', { - click, - hadScroll, - hadMutation, - hadSelectionChange, - }) continue } - // eslint-disable-next-line no-console - console.log('dead click?', { - isdc: scrollTimeout || mutationTimeout || absoluteTimeout || selectionChangedTimeout, - scrollTimeout, - mutationTimeout, - absoluteTimeout, - selectionChangedTimeout, - }) - if (scrollTimeout || mutationTimeout || absoluteTimeout || selectionChangedTimeout) { this._onCapture(click, { $dead_click_last_mutation_timestamp: this._lastMutation, From 399b001b3fc4385cbf68adf517a5b4db040ba83e Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 18 Nov 2024 11:35:59 +0000 Subject: [PATCH 4/6] fix --- eslint-rules/no-direct-mutation-observer.js | 38 +++------------------ src/utils/prototype-utils.ts | 7 ---- 2 files changed, 4 insertions(+), 41 deletions(-) diff --git a/eslint-rules/no-direct-mutation-observer.js b/eslint-rules/no-direct-mutation-observer.js index 0f4e7c3f6..38ab4705d 100644 --- a/eslint-rules/no-direct-mutation-observer.js +++ b/eslint-rules/no-direct-mutation-observer.js @@ -11,46 +11,16 @@ module.exports = { messages: { noDirectMutationObserver: 'Direct use of MutationObserver is not allowed. Use NativeMutationObserver from global.ts instead.', - missingNativeMutationObserver: - 'You must import NativeMutationObserver from global.ts to use MutationObserver functionality.', }, }, create(context) { - let importedNativeMutationObserver = false - const targetFileName = 'utils/global' - return { - ImportDeclaration(node) { - // Check if 'NativeMutationObserver' is imported from 'global.ts' - if (node.source.value.includes(targetFileName)) { - const importedSpecifiers = node.specifiers.map( - (specifier) => specifier.imported && specifier.imported.name - ) - if (importedSpecifiers.includes('NativeMutationObserver')) { - importedNativeMutationObserver = true - } - } - }, NewExpression(node) { - // Check if `MutationObserver` is used if (node.callee.type === 'Identifier' && node.callee.name === 'MutationObserver') { - if (!importedNativeMutationObserver) { - context.report({ - node, - messageId: 'noDirectMutationObserver', - }) - } - } - }, - Identifier(node) { - // Warn if `MutationObserver` is directly referenced outside of a `new` expression (rare cases) - if (node.name === 'MutationObserver' && node.parent.type !== 'NewExpression') { - if (!importedNativeMutationObserver) { - context.report({ - node, - messageId: 'missingNativeMutationObserver', - }) - } + context.report({ + node, + messageId: 'noDirectMutationObserver', + }) } }, } diff --git a/src/utils/prototype-utils.ts b/src/utils/prototype-utils.ts index 77115a83a..5a18b8b49 100644 --- a/src/utils/prototype-utils.ts +++ b/src/utils/prototype-utils.ts @@ -9,11 +9,7 @@ import { isAngularZonePatchedFunction, isFunction, isNativeFunction } from './ty import { logger } from './logger' interface NativeImplementationsCache { - // eslint-disable-next-line posthog-js/no-direct-mutation-observer MutationObserver: typeof MutationObserver - setTimeout: typeof setTimeout - addEventListener: typeof addEventListener - setInterval: typeof setInterval } const cachedImplementations: Partial = {} @@ -30,8 +26,6 @@ export function getNativeImplementation Date: Mon, 18 Nov 2024 11:37:10 +0000 Subject: [PATCH 5/6] fix --- src/utils/type-utils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/utils/type-utils.ts b/src/utils/type-utils.ts index 2dd888e59..8d3c5fceb 100644 --- a/src/utils/type-utils.ts +++ b/src/utils/type-utils.ts @@ -21,8 +21,6 @@ export const isFunction = function (f: any): f is (...args: any[]) => any { } export const isNativeFunction = function (f: any): f is (...args: any[]) => any { - // eslint-disable-next-line no-console - console.log(f.toString()) return isFunction(f) && f.toString().indexOf('[native code]') !== -1 } From 26fbddb350213f4c1c1f476442f375bca36ec256 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 18 Nov 2024 11:38:54 +0000 Subject: [PATCH 6/6] fix --- src/utils/type-utils.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/utils/type-utils.ts b/src/utils/type-utils.ts index 8d3c5fceb..fa0255d1b 100644 --- a/src/utils/type-utils.ts +++ b/src/utils/type-utils.ts @@ -15,21 +15,21 @@ export const isUint8Array = function (x: unknown): x is Uint8Array { // from a comment on http://dbj.org/dbj/?p=286 // fails on only one very rare and deliberate custom object: // let bomb = { toString : undefined, valueOf: function(o) { return "function BOMBA!"; }}; -export const isFunction = function (f: any): f is (...args: any[]) => any { +export const isFunction = function (x: unknown): x is (...args: any[]) => any { // eslint-disable-next-line posthog-js/no-direct-function-check - return typeof f === 'function' + return typeof x === 'function' } -export const isNativeFunction = function (f: any): f is (...args: any[]) => any { - return isFunction(f) && f.toString().indexOf('[native code]') !== -1 +export const isNativeFunction = function (x: unknown): x is (...args: any[]) => any { + return isFunction(x) && x.toString().indexOf('[native code]') !== -1 } // When angular patches functions they pass the above `isNativeFunction` check -export const isAngularZonePatchedFunction = function (f: any): boolean { - if (!isFunction(f)) { +export const isAngularZonePatchedFunction = function (x: unknown): boolean { + if (!isFunction(x)) { return false } - const prototypeKeys = Object.getOwnPropertyNames(f.prototype || {}) + const prototypeKeys = Object.getOwnPropertyNames(x.prototype || {}) return prototypeKeys.some((key) => key.indexOf('__zone')) }