Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: angular change detection mutation observer #1531

Merged
merged 6 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions eslint-rules/no-direct-mutation-observer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module.exports = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, love the linter rule

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.',
},
},
create(context) {
return {
NewExpression(node) {
if (node.callee.type === 'Identifier' && node.callee.name === 'MutationObserver') {
context.report({
node,
messageId: 'noDirectMutationObserver',
})
}
},
}
},
}
3 changes: 2 additions & 1 deletion src/entrypoints/dead-clicks-autocapture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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, {
Expand Down
19 changes: 13 additions & 6 deletions src/utils/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<string, any> & {
__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
Expand Down Expand Up @@ -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<string, any> & {
__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 }
60 changes: 60 additions & 0 deletions src/utils/prototype-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* 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 {
MutationObserver: typeof MutationObserver
}

const cachedImplementations: Partial<NativeImplementationsCache> = {}

export function getNativeImplementation<T extends keyof NativeImplementationsCache>(
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)) {
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])
}

export function getNativeMutationObserverImplementation(assignableWindow: AssignableWindow): typeof MutationObserver {
return getNativeImplementation('MutationObserver', assignableWindow)
}
18 changes: 16 additions & 2 deletions src/utils/type-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,24 @@ 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 (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 (x: unknown): boolean {
if (!isFunction(x)) {
return false
}
const prototypeKeys = Object.getOwnPropertyNames(x.prototype || {})
return prototypeKeys.some((key) => key.indexOf('__zone'))
}

// Underscore Addons
export const isObject = function (x: unknown): x is Record<string, any> {
// eslint-disable-next-line posthog-js/no-direct-object-check
Expand Down
Loading