-
Notifications
You must be signed in to change notification settings - Fork 135
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 all 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,28 @@ | ||
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.', | ||
}, | ||
}, | ||
create(context) { | ||
return { | ||
NewExpression(node) { | ||
if (node.callee.type === 'Identifier' && node.callee.name === 'MutationObserver') { | ||
context.report({ | ||
node, | ||
messageId: 'noDirectMutationObserver', | ||
}) | ||
} | ||
}, | ||
} | ||
}, | ||
} |
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) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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')) | ||
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