-
Notifications
You must be signed in to change notification settings - Fork 138
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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', | ||
}) | ||
} | ||
} | ||
}, | ||
} | ||
}, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<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)) { | ||
// 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) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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().indexOf('[native code]') !== -1 | ||
} | ||
|
||
// 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')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, wild! I guess I can update the docs to call that out... "if you've changed the The other thing maybe to look on window for something - I think i noticed a |
||
} | ||
|
||
// Underscore Addons | ||
export const isObject = function (x: unknown): x is Record<string, any> { | ||
// eslint-disable-next-line posthog-js/no-direct-object-check | ||
|
There was a problem hiding this comment.
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