From 117cf6264503265aee3b6d0684bd054f12d48cd8 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Fri, 24 Jan 2025 16:36:30 -0300 Subject: [PATCH 1/8] feat: Add new rule to guarantee events are tracked as passive We'll never call `preventDefault` on our events and, for that reason, it's smarter to tag our events as `passive: true` letting the browser know that it can continue processing the event, it doesn't need to wait for us. This should improve performance on lower-end devices. --- .eslintrc.js | 2 + eslint-rules/custom-eslint-rules.test.js | 45 +++++++++++++- eslint-rules/passive-event-listeners.js | 75 ++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 eslint-rules/passive-event-listeners.js diff --git a/.eslintrc.js b/.eslintrc.js index 294db20a3..0d78895c1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -89,7 +89,9 @@ module.exports = { rules: { 'prettier/prettier': 'error', '@typescript-eslint/no-var-requires': 'off', + '@typescript-eslint/no-require-imports': 'off', 'posthog-js/no-direct-null-check': 'off', + 'posthog-js/no-direct-boolean-check': 'off', }, env: { node: true, diff --git a/eslint-rules/custom-eslint-rules.test.js b/eslint-rules/custom-eslint-rules.test.js index 1717e5f49..852a3b752 100644 --- a/eslint-rules/custom-eslint-rules.test.js +++ b/eslint-rules/custom-eslint-rules.test.js @@ -7,10 +7,19 @@ const noDirectStringCheck = require('./no-direct-string-check') const noDirectDateCheck = require('./no-direct-date-check') const noDirectNumberCheck = require('./no-direct-number-check') const noDirectBooleanCheck = require('./no-direct-boolean-check') +const passiveEventListeners = require('./passive-event-listeners') const { RuleTester } = require('eslint') -const ruleTester = new RuleTester() +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2015, + sourceType: 'module', + }, + env: { + browser: true, + }, +}) ruleTester.run('no-direct-null-check', noDirectNullCheck, { valid: [{ code: `isNull(x)` }], @@ -122,3 +131,37 @@ ruleTester.run('no-direct-boolean-check', noDirectBooleanCheck, { }, ], }) + +ruleTester.run('passive-event-listeners', passiveEventListeners, { + valid: [ + { code: "document.addEventListener('click', () => {}, { passive: true })" }, + { code: "document.addEventListener('scroll', () => {}, { capture: true, passive: true })" }, + ], + invalid: [ + { + code: "document.addEventListener('mouseleave', () => {})", + errors: [{ message: 'addEventListener should include { passive: true } as the third argument' }], + output: "document.addEventListener('mouseleave', () => {}, { passive: true })", + }, + { + code: "document.addEventListener('click', () => {}, true)", + errors: [{ message: 'addEventListener should use an options object including { passive: true }' }], + output: "document.addEventListener('click', () => {}, { capture: true, passive: true })", + }, + { + code: "document.addEventListener('click', () => {}, {})", + errors: [{ message: 'addEventListener should have { passive: true } in its options' }], + output: "document.addEventListener('click', () => {}, { passive: true })", + }, + { + code: "document.addEventListener('pageleave', () => {}, { capture: true })", + errors: [{ message: 'addEventListener should have { passive: true } in its options' }], + output: "document.addEventListener('pageleave', () => {}, { capture: true, passive: true })", + }, + { + code: "document.addEventListener('pageleave', () => {}, { capture: false })", + errors: [{ message: 'addEventListener should have { passive: true } in its options' }], + output: "document.addEventListener('pageleave', () => {}, { capture: false, passive: true })", + }, + ], +}) diff --git a/eslint-rules/passive-event-listeners.js b/eslint-rules/passive-event-listeners.js new file mode 100644 index 000000000..5d31429d2 --- /dev/null +++ b/eslint-rules/passive-event-listeners.js @@ -0,0 +1,75 @@ +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'Enforce passive event listeners for better scroll performance', + category: 'Best Practices', + recommended: true, + }, + fixable: 'code', + schema: [], // no options + }, + + create(context) { + return { + CallExpression(node) { + // Check if it's an addEventListener call + const callee = node.callee + if (callee.type === 'MemberExpression' && callee.property.name === 'addEventListener') { + // Check if there's a third argument (options) + if (node.arguments.length < 3) { + context.report({ + node, + message: 'addEventListener should include { passive: true } as the third argument', + fix(fixer) { + return fixer.insertTextAfterRange( + [node.arguments[1].range[1], node.arguments[1].range[1]], + ', { passive: true }' + ) + }, + }) + return + } + + // Handle the case where the third argument is a boolean (capture) + const options = node.arguments[2] + if (options.type === 'Literal' && typeof options.value === 'boolean') { + context.report({ + node, + message: 'addEventListener should use an options object including { passive: true }', + fix(fixer) { + return fixer.replaceText(options, `{ capture: ${options.value}, passive: true }`) + }, + }) + return + } + + // Check if the third argument is an object with passive: true + if ( + options.type === 'ObjectExpression' && + !options.properties.some( + (prop) => + prop.key.name === 'passive' && + prop.value.type === 'Literal' && + prop.value.value === true + ) + ) { + context.report({ + node, + message: 'addEventListener should have { passive: true } in its options', + fix(fixer) { + if (options.properties.length === 0) { + return fixer.replaceText(options, '{ passive: true }') + } + return fixer.insertTextAfter( + options.properties[options.properties.length - 1], + ', passive: true' + ) + }, + }) + } + } + }, + } + }, +} From d9719d095ae6ef1b45b5095061683f23db280d67 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Fri, 24 Jan 2025 16:37:01 -0300 Subject: [PATCH 2/8] refactor: Apply new `passive-event-listeners` rule It's slightly more performant to have these in our events, see previous commit for rationale --- .../nextjs/pages/replay-examples/canvas.tsx | 34 +++++++++----- src/entrypoints/dead-clicks-autocapture.ts | 4 +- src/entrypoints/external-scripts-loader.ts | 2 +- src/entrypoints/recorder.ts | 4 ++ src/extensions/replay/sessionrecording.ts | 8 ++-- src/extensions/surveys.tsx | 46 ++++++++++++------- src/heatmaps.ts | 4 +- src/posthog-core.ts | 5 +- src/retry-queue.ts | 24 +++++++--- src/sessionid.ts | 2 + src/utils/index.ts | 2 +- 11 files changed, 88 insertions(+), 47 deletions(-) diff --git a/playground/nextjs/pages/replay-examples/canvas.tsx b/playground/nextjs/pages/replay-examples/canvas.tsx index f24e4a98a..86dd19e3d 100644 --- a/playground/nextjs/pages/replay-examples/canvas.tsx +++ b/playground/nextjs/pages/replay-examples/canvas.tsx @@ -84,21 +84,33 @@ export default function Canvas() { } } - canvas.addEventListener('mousemove', function (e) { - focalLength = e.x - }) + canvas.addEventListener( + 'mousemove', + function (e) { + focalLength = e.x + }, + { passive: true } + ) // Kick off the animation when the mouse enters the canvas - canvas.addEventListener('mouseover', function () { - animate = true - executeFrame() - }) + canvas.addEventListener( + 'mouseover', + function () { + animate = true + executeFrame() + }, + { passive: true } + ) // Pause animation when the mouse exits the canvas - canvas.addEventListener('mouseout', function () { - // mouseDown = false - animate = false - }) + canvas.addEventListener( + 'mouseout', + function () { + // mouseDown = false + animate = false + }, + { passive: true } + ) initializeStars() // Draw the first frame to start animation diff --git a/src/entrypoints/dead-clicks-autocapture.ts b/src/entrypoints/dead-clicks-autocapture.ts index 7c31754e6..cbe0fdd53 100644 --- a/src/entrypoints/dead-clicks-autocapture.ts +++ b/src/entrypoints/dead-clicks-autocapture.ts @@ -98,7 +98,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture } private _startClickObserver() { - assignableWindow.addEventListener('click', this._onClick) + assignableWindow.addEventListener('click', this._onClick, { passive: true }) } private _onClick = (event: MouseEvent): void => { @@ -141,7 +141,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture } private _startSelectionChangedObserver() { - assignableWindow.addEventListener('selectionchange', this._onSelectionChange) + assignableWindow.addEventListener('selectionchange', this._onSelectionChange, { passive: true }) } private _onSelectionChange = (): void => { diff --git a/src/entrypoints/external-scripts-loader.ts b/src/entrypoints/external-scripts-loader.ts index f1d6b8322..25c6fb935 100644 --- a/src/entrypoints/external-scripts-loader.ts +++ b/src/entrypoints/external-scripts-loader.ts @@ -41,7 +41,7 @@ const loadScript = (posthog: PostHog, url: string, callback: (error?: string | E if (document?.body) { addScript() } else { - document?.addEventListener('DOMContentLoaded', addScript) + document?.addEventListener('DOMContentLoaded', addScript, { passive: true }) } } diff --git a/src/entrypoints/recorder.ts b/src/entrypoints/recorder.ts index cade379f9..9cef36e60 100644 --- a/src/entrypoints/recorder.ts +++ b/src/entrypoints/recorder.ts @@ -278,6 +278,9 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required { if (xhr.readyState !== xhr.DONE) { return @@ -325,6 +328,7 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required { - if (surveyPopup) { - surveyPopup.style.display = surveyPopup.style.display === 'none' ? 'block' : 'none' - surveyPopup.addEventListener('PHSurveyClosed', () => { - this.removeSurveyFromFocus(survey.id) - surveyPopup.style.display = 'none' - }) - } - }) + + selectorOnPage.addEventListener( + 'click', + () => { + if (surveyPopup) { + surveyPopup.style.display = surveyPopup.style.display === 'none' ? 'block' : 'none' + surveyPopup.addEventListener( + 'PHSurveyClosed', + () => { + this.removeSurveyFromFocus(survey.id) + surveyPopup.style.display = 'none' + }, + { passive: true } + ) + } + }, + { passive: true } + ) selectorOnPage.setAttribute('PHWidgetSurveyClickListener', 'true') } } @@ -434,8 +443,8 @@ export function usePopupVisibility( } } - window.addEventListener('PHSurveyClosed', handleSurveyClosed) - window.addEventListener('PHSurveySent', handleSurveySent) + window.addEventListener('PHSurveyClosed', handleSurveyClosed, { passive: true }) + window.addEventListener('PHSurveySent', handleSurveySent, { passive: true }) if (millisecondDelay > 0) { return handleShowSurveyWithDelay() @@ -519,9 +528,7 @@ export function SurveyPopup({ /> )} - ) : ( - <> - ) + ) : null } export function Questions({ @@ -683,9 +690,14 @@ export function FeedbackWidget({ } if (survey.appearance?.widgetType === 'selector') { const widget = document.querySelector(survey.appearance.widgetSelector || '') - widget?.addEventListener('click', () => { - setShowSurvey(!showSurvey) - }) + + widget?.addEventListener( + 'click', + () => { + setShowSurvey(!showSurvey) + }, + { passive: true } + ) widget?.setAttribute('PHWidgetSurveyClickListener', 'true') } }, []) diff --git a/src/heatmaps.ts b/src/heatmaps.ts index db976ed4e..2832522f4 100644 --- a/src/heatmaps.ts +++ b/src/heatmaps.ts @@ -60,9 +60,7 @@ export class Heatmaps { this.instance = instance this._enabledServerSide = !!this.instance.persistence?.props[HEATMAPS_ENABLED_SERVER_SIDE] - window?.addEventListener('beforeunload', () => { - this.flush() - }) + window?.addEventListener('beforeunload', this.flush, { passive: true }) } public get flushIntervalMilliseconds(): number { diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 0748b9641..2dd65a3d5 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -545,6 +545,9 @@ export class PostHog { } // Set up event handler for pageleave // Use `onpagehide` if available, see https://calendar.perfplanet.com/2020/beaconing-in-practice/#beaconing-reliability-avoiding-abandons + // + // Not making it passive to try and force the browser to handle this before the page is unloaded + // eslint-disable-next-line posthog-js/passive-event-listeners window?.addEventListener?.('onpagehide' in self ? 'pagehide' : 'unload', this._handle_unload.bind(this)) this.toolbar.maybeLoadToolbar() @@ -2305,7 +2308,7 @@ const add_dom_loaded_handler = function () { // 'loaded' is an IE thing dom_loaded_handler() } else { - document.addEventListener('DOMContentLoaded', dom_loaded_handler, false) + document.addEventListener('DOMContentLoaded', dom_loaded_handler, { capture: false, passive: true }) } } diff --git a/src/retry-queue.ts b/src/retry-queue.ts index 129090356..b1cdf19a2 100644 --- a/src/retry-queue.ts +++ b/src/retry-queue.ts @@ -45,13 +45,23 @@ export class RetryQueue { if (!isUndefined(window) && 'onLine' in window.navigator) { this.areWeOnline = window.navigator.onLine - window.addEventListener('online', () => { - this.areWeOnline = true - this.flush() - }) - window.addEventListener('offline', () => { - this.areWeOnline = false - }) + + window.addEventListener( + 'online', + () => { + this.areWeOnline = true + this.flush() + }, + { passive: true } + ) + + window.addEventListener( + 'offline', + () => { + this.areWeOnline = false + }, + { passive: true } + ) } } diff --git a/src/sessionid.ts b/src/sessionid.ts index 07f4d82a0..063e4b1e8 100644 --- a/src/sessionid.ts +++ b/src/sessionid.ts @@ -198,6 +198,8 @@ export class SessionIdManager { * We conditionally check the primaryWindowExists value in the constructor to decide if the window id in the last session storage should be carried over. */ private _listenToReloadWindow(): void { + // Not making it passive to try and force the browser to handle this before the page is unloaded + // eslint-disable-next-line posthog-js/passive-event-listeners window?.addEventListener('beforeunload', () => { if (this._canUseSessionStorage()) { sessionStore.remove(this._primary_window_exists_storage_key) diff --git a/src/utils/index.ts b/src/utils/index.ts index 2a6b8b90f..7ba23ec97 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -236,7 +236,7 @@ export const registerEvent = (function () { } if (element.addEventListener && !oldSchool) { - element.addEventListener(type, handler, !!useCapture) + element.addEventListener(type, handler, { capture: !!useCapture, passive: true }) } else { const ontype = 'on' + type const old_handler = (element as any)[ontype] // can be undefined From 7a595352bf4454c3d4ba665f3f945600abd3dd21 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 27 Jan 2025 10:31:54 -0300 Subject: [PATCH 3/8] refactor: Remove ancient `registerEvent` function This is ancient code, and it's only ever used for IE6-8 - which we don't support. ``` "browserslist": [ "> 0.5%, last 2 versions, Firefox ESR, not dead, IE 11" ] ``` Of course, we have the oldSchool flag which triggers this code for window.load when trying to initialize posthog-js. make-all-event-listeners-passive/src/posthog-core.ts#L2315-L2318 That's only triggered, however, if document.addEventListener doesn't exist which, again, is only if you're running IE6-8. I'll instead replace it with a generic addEventListener function intended to avoid having to repeat passive: true everywhere and reduce our bundle size slightly. --- src/autocapture.ts | 23 +++++----- src/extensions/toolbar.ts | 14 +++--- src/heatmaps.ts | 12 +++-- src/posthog-core.ts | 18 +++----- src/utils/index.ts | 92 ++------------------------------------- 5 files changed, 40 insertions(+), 119 deletions(-) diff --git a/src/autocapture.ts b/src/autocapture.ts index 917a4a788..25a237d55 100644 --- a/src/autocapture.ts +++ b/src/autocapture.ts @@ -1,4 +1,4 @@ -import { each, extend, registerEvent } from './utils' +import { each, extend } from './utils' import { autocaptureCompatibleElements, getClassNames, @@ -258,6 +258,7 @@ export class Autocapture { if (!window || !document) { return } + const handler = (e: Event) => { e = e || window?.event try { @@ -267,18 +268,18 @@ export class Autocapture { } } - const copiedTextHandler = (e: Event) => { - e = e || window?.event - this._captureEvent(e, COPY_AUTOCAPTURE_EVENT) - } - - registerEvent(document, 'submit', handler, false, true) - registerEvent(document, 'change', handler, false, true) - registerEvent(document, 'click', handler, false, true) + document.addEventListener('submit', handler, { capture: true, passive: true }) + document.addEventListener('change', handler, { capture: true, passive: true }) + document.addEventListener('click', handler, { capture: true, passive: true }) if (this.config.capture_copied_text) { - registerEvent(document, 'copy', copiedTextHandler, false, true) - registerEvent(document, 'cut', copiedTextHandler, false, true) + const copiedTextHandler = (e: Event) => { + e = e || window?.event + this._captureEvent(e, COPY_AUTOCAPTURE_EVENT) + } + + document.addEventListener('copy', copiedTextHandler, { capture: true, passive: true }) + document.addEventListener('cut', copiedTextHandler, { capture: true, passive: true }) } } diff --git a/src/extensions/toolbar.ts b/src/extensions/toolbar.ts index 644b3fd24..433064498 100644 --- a/src/extensions/toolbar.ts +++ b/src/extensions/toolbar.ts @@ -1,4 +1,4 @@ -import { registerEvent, trySafe } from '../utils' +import { trySafe } from '../utils' import { PostHog } from '../posthog-core' import { ToolbarParams } from '../types' import { _getHashParam } from '../utils/request-utils' @@ -178,10 +178,14 @@ export class Toolbar { // Turbolinks doesn't fire an onload event but does replace the entire body, including the toolbar. // Thus, we ensure the toolbar is only loaded inside the body, and then reloaded on turbolinks:load. - registerEvent(window, 'turbolinks:load', () => { - this.setToolbarState(ToolbarState.UNINITIALIZED) - this.loadToolbar(toolbarParams) - }) + window.addEventListener( + 'turbolinks:load', + () => { + this.setToolbarState(ToolbarState.UNINITIALIZED) + this.loadToolbar(toolbarParams) + }, + { passive: true } + ) } return true diff --git a/src/heatmaps.ts b/src/heatmaps.ts index 2832522f4..f51e47c16 100644 --- a/src/heatmaps.ts +++ b/src/heatmaps.ts @@ -1,4 +1,3 @@ -import { registerEvent } from './utils' import RageClick from './extensions/rageclick' import { DeadClickCandidate, Properties, RemoteConfig } from './types' import { PostHog } from './posthog-core' @@ -130,8 +129,15 @@ export class Heatmaps { return } - registerEvent(document, 'click', (e) => this._onClick((e || window?.event) as MouseEvent), false, true) - registerEvent(document, 'mousemove', (e) => this._onMouseMove((e || window?.event) as MouseEvent), false, true) + document.addEventListener('click', (e) => this._onClick((e || window?.event) as MouseEvent), { + capture: true, + passive: true, + }) + + document.addEventListener('mousemove', (e) => this._onMouseMove((e || window?.event) as MouseEvent), { + capture: true, + passive: true, + }) this.deadClicksCapture = new DeadClicksAutocapture( this.instance, diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 2dd65a3d5..4bec13379 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -1,13 +1,5 @@ import Config from './config' -import { - _copyAndTruncateStrings, - each, - eachArray, - extend, - registerEvent, - safewrapClass, - isCrossDomainCookie, -} from './utils' +import { _copyAndTruncateStrings, each, eachArray, extend, safewrapClass, isCrossDomainCookie } from './utils' import { assignableWindow, document, location, navigator, userAgent, window } from './utils/globals' import { PostHogFeatureFlags } from './posthog-featureflags' import { PostHogPersistence } from './posthog-persistence' @@ -2310,11 +2302,15 @@ const add_dom_loaded_handler = function () { } else { document.addEventListener('DOMContentLoaded', dom_loaded_handler, { capture: false, passive: true }) } + + return } - // fallback handler, always will work + // Only IE6-8 don't support `document.addEventListener` and we don't support them + // so let's simply log an error stating PostHog couldn't be initialized + // We're checking for `window` to avoid erroring out on a SSR context if (window) { - registerEvent(window, 'load', dom_loaded_handler, true) + logger.error("Browser doesn't support `document.addEventListener` so PostHog couldn't be initialized") } } diff --git a/src/utils/index.ts b/src/utils/index.ts index 7ba23ec97..13bff6374 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,7 +1,7 @@ -import { Breaker, EventHandler, Properties } from '../types' -import { hasOwnProperty, isArray, isFormData, isFunction, isNull, isNullish, isString } from './type-utils' +import { Breaker, Properties } from '../types' +import { hasOwnProperty, isArray, isFormData, isNull, isNullish, isString } from './type-utils' import { logger } from './logger' -import { nativeForEach, nativeIndexOf, window } from './globals' +import { nativeForEach, nativeIndexOf } from './globals' const breaker: Breaker = {} @@ -208,92 +208,6 @@ export function _copyAndTruncateStrings = Record Date: Mon, 27 Jan 2025 10:49:41 -0300 Subject: [PATCH 4/8] refactor: Use `addEventListener` helper This helper is intended to save us some bytes by avoiding having to repeat `passive: true` everywhere --- src/autocapture.ts | 12 +++--- src/entrypoints/dead-clicks-autocapture.ts | 11 +++-- src/entrypoints/external-scripts-loader.ts | 3 +- src/extensions/replay/sessionrecording.ts | 9 +++-- src/extensions/surveys.tsx | 47 +++++++++------------- src/extensions/toolbar.ts | 14 +++---- src/heatmaps.ts | 12 ++---- src/posthog-core.ts | 17 ++++++-- src/retry-queue.ts | 25 +++++------- src/scroll-manager.ts | 11 ++--- src/sessionid.ts | 19 +++++---- src/utils/index.ts | 15 +++++++ 12 files changed, 102 insertions(+), 93 deletions(-) diff --git a/src/autocapture.ts b/src/autocapture.ts index 25a237d55..41bf407dd 100644 --- a/src/autocapture.ts +++ b/src/autocapture.ts @@ -1,4 +1,4 @@ -import { each, extend } from './utils' +import { addEventListener, each, extend } from './utils' import { autocaptureCompatibleElements, getClassNames, @@ -268,9 +268,9 @@ export class Autocapture { } } - document.addEventListener('submit', handler, { capture: true, passive: true }) - document.addEventListener('change', handler, { capture: true, passive: true }) - document.addEventListener('click', handler, { capture: true, passive: true }) + addEventListener(document, 'submit', handler, { capture: true }) + addEventListener(document, 'change', handler, { capture: true }) + addEventListener(document, 'click', handler, { capture: true }) if (this.config.capture_copied_text) { const copiedTextHandler = (e: Event) => { @@ -278,8 +278,8 @@ export class Autocapture { this._captureEvent(e, COPY_AUTOCAPTURE_EVENT) } - document.addEventListener('copy', copiedTextHandler, { capture: true, passive: true }) - document.addEventListener('cut', copiedTextHandler, { capture: true, passive: true }) + addEventListener(document, 'copy', copiedTextHandler, { capture: true }) + addEventListener(document, 'cut', copiedTextHandler, { capture: true }) } } diff --git a/src/entrypoints/dead-clicks-autocapture.ts b/src/entrypoints/dead-clicks-autocapture.ts index cbe0fdd53..1af7a6f09 100644 --- a/src/entrypoints/dead-clicks-autocapture.ts +++ b/src/entrypoints/dead-clicks-autocapture.ts @@ -6,6 +6,7 @@ import { DeadClickCandidate, DeadClicksAutoCaptureConfig, Properties } from '../ import { autocapturePropertiesForElement } from '../autocapture' import { isElementInToolbar, isElementNode, isTag } from '../utils/element-utils' import { getNativeMutationObserverImplementation } from '../utils/prototype-utils' +import { addEventListener } from '../utils' function asClick(event: MouseEvent): DeadClickCandidate | null { const eventTarget = getEventTarget(event) @@ -98,7 +99,11 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture } private _startClickObserver() { - assignableWindow.addEventListener('click', this._onClick, { passive: true }) + const clickHandler = (event: Event) => { + this._onClick(event as MouseEvent) + } + + addEventListener(assignableWindow, 'click', clickHandler) } private _onClick = (event: MouseEvent): void => { @@ -122,7 +127,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture // This allows the browser to optimize scrolling performance by not waiting for our handling of the scroll event // see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#passive private _startScrollObserver() { - assignableWindow.addEventListener('scroll', this._onScroll, { capture: true, passive: true }) + addEventListener(assignableWindow, 'scroll', this._onScroll, { capture: true }) } private _onScroll = (): void => { @@ -141,7 +146,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture } private _startSelectionChangedObserver() { - assignableWindow.addEventListener('selectionchange', this._onSelectionChange, { passive: true }) + addEventListener(assignableWindow, 'selectionchange', this._onSelectionChange) } private _onSelectionChange = (): void => { diff --git a/src/entrypoints/external-scripts-loader.ts b/src/entrypoints/external-scripts-loader.ts index 25c6fb935..93c11ffb9 100644 --- a/src/entrypoints/external-scripts-loader.ts +++ b/src/entrypoints/external-scripts-loader.ts @@ -1,4 +1,5 @@ import type { PostHog } from '../posthog-core' +import { addEventListener } from '../utils' import { assignableWindow, document, PostHogExtensionKind } from '../utils/globals' import { createLogger } from '../utils/logger' @@ -41,7 +42,7 @@ const loadScript = (posthog: PostHog, url: string, callback: (error?: string | E if (document?.body) { addScript() } else { - document?.addEventListener('DOMContentLoaded', addScript, { passive: true }) + addEventListener(document, 'DOMContentLoaded', addScript) } } diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index b7cb82423..8d38122ec 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -47,6 +47,7 @@ import { gzipSync, strFromU8, strToU8 } from 'fflate' import { clampToRange } from '../../utils/number-utils' import Config from '../../config' import { includes } from '../../utils/string-utils' +import { addEventListener } from '../../utils' const LOGGER_PREFIX = '[SessionRecording]' const logger = createLogger(LOGGER_PREFIX) @@ -492,10 +493,10 @@ export class SessionRecording { this._startCapture(startReason) // calling addEventListener multiple times is safe and will not add duplicates - window?.addEventListener('beforeunload', this._onBeforeUnload, { passive: true }) - window?.addEventListener('offline', this._onOffline, { passive: true }) - window?.addEventListener('online', this._onOnline, { passive: true }) - window?.addEventListener('visibilitychange', this._onVisibilityChange, { passive: true }) + addEventListener(window, 'beforeunload', this._onBeforeUnload) + addEventListener(window, 'offline', this._onOffline) + addEventListener(window, 'online', this._onOnline) + addEventListener(window, 'visibilitychange', this._onVisibilityChange) // on reload there might be an already sampled session that should be continued before decide response, // so we call this here _and_ in the decide response diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index b2894f535..937d61c9a 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -34,6 +34,7 @@ import { style, SurveyContext, } from './surveys/surveys-utils' +import { addEventListener } from '../utils' const logger = createLogger('[Surveys]') // We cast the types here which is dangerous but protected by the top level generateSurveys call @@ -121,23 +122,16 @@ export class SurveyManager { .querySelector(`.PostHogWidget${survey.id}`) ?.shadowRoot?.querySelector(`.survey-form`) as HTMLFormElement - selectorOnPage.addEventListener( - 'click', - () => { - if (surveyPopup) { - surveyPopup.style.display = surveyPopup.style.display === 'none' ? 'block' : 'none' - surveyPopup.addEventListener( - 'PHSurveyClosed', - () => { - this.removeSurveyFromFocus(survey.id) - surveyPopup.style.display = 'none' - }, - { passive: true } - ) - } - }, - { passive: true } - ) + addEventListener(selectorOnPage, 'click', () => { + if (surveyPopup) { + surveyPopup.style.display = surveyPopup.style.display === 'none' ? 'block' : 'none' + addEventListener(surveyPopup, 'PHSurveyClosed', () => { + this.removeSurveyFromFocus(survey.id) + surveyPopup.style.display = 'none' + }) + } + }) + selectorOnPage.setAttribute('PHWidgetSurveyClickListener', 'true') } } @@ -443,8 +437,8 @@ export function usePopupVisibility( } } - window.addEventListener('PHSurveyClosed', handleSurveyClosed, { passive: true }) - window.addEventListener('PHSurveySent', handleSurveySent, { passive: true }) + addEventListener(window, 'PHSurveyClosed', handleSurveyClosed) + addEventListener(window, 'PHSurveySent', handleSurveySent) if (millisecondDelay > 0) { return handleShowSurveyWithDelay() @@ -689,15 +683,12 @@ export function FeedbackWidget({ } } if (survey.appearance?.widgetType === 'selector') { - const widget = document.querySelector(survey.appearance.widgetSelector || '') - - widget?.addEventListener( - 'click', - () => { - setShowSurvey(!showSurvey) - }, - { passive: true } - ) + const widget = document.querySelector(survey.appearance.widgetSelector || '') ?? undefined + + addEventListener(widget, 'click', () => { + setShowSurvey(!showSurvey) + }) + widget?.setAttribute('PHWidgetSurveyClickListener', 'true') } }, []) diff --git a/src/extensions/toolbar.ts b/src/extensions/toolbar.ts index 433064498..0e3c7090c 100644 --- a/src/extensions/toolbar.ts +++ b/src/extensions/toolbar.ts @@ -1,4 +1,4 @@ -import { trySafe } from '../utils' +import { addEventListener, trySafe } from '../utils' import { PostHog } from '../posthog-core' import { ToolbarParams } from '../types' import { _getHashParam } from '../utils/request-utils' @@ -178,14 +178,10 @@ export class Toolbar { // Turbolinks doesn't fire an onload event but does replace the entire body, including the toolbar. // Thus, we ensure the toolbar is only loaded inside the body, and then reloaded on turbolinks:load. - window.addEventListener( - 'turbolinks:load', - () => { - this.setToolbarState(ToolbarState.UNINITIALIZED) - this.loadToolbar(toolbarParams) - }, - { passive: true } - ) + addEventListener(window, 'turbolinks:load', () => { + this.setToolbarState(ToolbarState.UNINITIALIZED) + this.loadToolbar(toolbarParams) + }) } return true diff --git a/src/heatmaps.ts b/src/heatmaps.ts index f51e47c16..b708a369c 100644 --- a/src/heatmaps.ts +++ b/src/heatmaps.ts @@ -10,6 +10,7 @@ import { createLogger } from './utils/logger' import { isElementInToolbar, isElementNode, isTag } from './utils/element-utils' import { DeadClicksAutocapture, isDeadClicksEnabledForHeatmaps } from './extensions/dead-clicks-autocapture' import { includes } from './utils/string-utils' +import { addEventListener } from './utils' const DEFAULT_FLUSH_INTERVAL = 5000 @@ -59,7 +60,7 @@ export class Heatmaps { this.instance = instance this._enabledServerSide = !!this.instance.persistence?.props[HEATMAPS_ENABLED_SERVER_SIDE] - window?.addEventListener('beforeunload', this.flush, { passive: true }) + addEventListener(window, 'beforeunload', this.flush) } public get flushIntervalMilliseconds(): number { @@ -129,14 +130,9 @@ export class Heatmaps { return } - document.addEventListener('click', (e) => this._onClick((e || window?.event) as MouseEvent), { + addEventListener(document, 'click', (e) => this._onClick((e || window?.event) as MouseEvent), { capture: true }) + addEventListener(document, 'mousemove', (e) => this._onMouseMove((e || window?.event) as MouseEvent), { capture: true, - passive: true, - }) - - document.addEventListener('mousemove', (e) => this._onMouseMove((e || window?.event) as MouseEvent), { - capture: true, - passive: true, }) this.deadClicksCapture = new DeadClicksAutocapture( diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 4bec13379..f4d42b6ad 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -1,5 +1,13 @@ import Config from './config' -import { _copyAndTruncateStrings, each, eachArray, extend, safewrapClass, isCrossDomainCookie } from './utils' +import { + _copyAndTruncateStrings, + each, + eachArray, + extend, + safewrapClass, + isCrossDomainCookie, + addEventListener, +} from './utils' import { assignableWindow, document, location, navigator, userAgent, window } from './utils/globals' import { PostHogFeatureFlags } from './posthog-featureflags' import { PostHogPersistence } from './posthog-persistence' @@ -539,8 +547,9 @@ export class PostHog { // Use `onpagehide` if available, see https://calendar.perfplanet.com/2020/beaconing-in-practice/#beaconing-reliability-avoiding-abandons // // Not making it passive to try and force the browser to handle this before the page is unloaded - // eslint-disable-next-line posthog-js/passive-event-listeners - window?.addEventListener?.('onpagehide' in self ? 'pagehide' : 'unload', this._handle_unload.bind(this)) + addEventListener(window, 'onpagehide' in self ? 'pagehide' : 'unload', this._handle_unload.bind(this), { + passive: false, + }) this.toolbar.maybeLoadToolbar() @@ -2300,7 +2309,7 @@ const add_dom_loaded_handler = function () { // 'loaded' is an IE thing dom_loaded_handler() } else { - document.addEventListener('DOMContentLoaded', dom_loaded_handler, { capture: false, passive: true }) + addEventListener(document, 'DOMContentLoaded', dom_loaded_handler, { capture: false }) } return diff --git a/src/retry-queue.ts b/src/retry-queue.ts index b1cdf19a2..b21ab2263 100644 --- a/src/retry-queue.ts +++ b/src/retry-queue.ts @@ -5,6 +5,7 @@ import { logger } from './utils/logger' import { window } from './utils/globals' import { PostHog } from './posthog-core' import { extendURLParams } from './request' +import { addEventListener } from './utils' const thirtyMinutes = 30 * 60 * 1000 @@ -46,22 +47,14 @@ export class RetryQueue { if (!isUndefined(window) && 'onLine' in window.navigator) { this.areWeOnline = window.navigator.onLine - window.addEventListener( - 'online', - () => { - this.areWeOnline = true - this.flush() - }, - { passive: true } - ) - - window.addEventListener( - 'offline', - () => { - this.areWeOnline = false - }, - { passive: true } - ) + addEventListener(window, 'online', () => { + this.areWeOnline = true + this.flush() + }) + + addEventListener(window, 'offline', () => { + this.areWeOnline = false + }) } } diff --git a/src/scroll-manager.ts b/src/scroll-manager.ts index c2a1849db..90677c216 100644 --- a/src/scroll-manager.ts +++ b/src/scroll-manager.ts @@ -1,6 +1,7 @@ import { window } from './utils/globals' import { PostHog } from './posthog-core' import { isArray } from './utils/type-utils' +import { addEventListener } from './utils' export interface ScrollContext { // scroll is how far down the page the user has scrolled, @@ -59,14 +60,10 @@ export class ScrollManager { // `capture: true` is required to get scroll events for other scrollable elements // on the page, not just the window // see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#usecapture - // - // `passive: true` is used to tell the browser that the scroll event handler will not call `preventDefault()` - // This allows the browser to optimize scrolling performance by not waiting for our handling of the scroll event - // see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#passive startMeasuringScrollPosition() { - window?.addEventListener('scroll', this._updateScrollData, { capture: true, passive: true }) - window?.addEventListener('scrollend', this._updateScrollData, { capture: true, passive: true }) - window?.addEventListener('resize', this._updateScrollData, { passive: true }) + addEventListener(window, 'scroll', this._updateScrollData, { capture: true }) + addEventListener(window, 'scrollend', this._updateScrollData, { capture: true }) + addEventListener(window, 'resize', this._updateScrollData) } public scrollElement(): Element | undefined { diff --git a/src/sessionid.ts b/src/sessionid.ts index 063e4b1e8..2ae7fc180 100644 --- a/src/sessionid.ts +++ b/src/sessionid.ts @@ -10,6 +10,7 @@ import { createLogger } from './utils/logger' import { clampToRange } from './utils/number-utils' import { PostHog } from './posthog-core' +import { addEventListener } from './utils' const logger = createLogger('[SessionId]') @@ -198,13 +199,17 @@ export class SessionIdManager { * We conditionally check the primaryWindowExists value in the constructor to decide if the window id in the last session storage should be carried over. */ private _listenToReloadWindow(): void { - // Not making it passive to try and force the browser to handle this before the page is unloaded - // eslint-disable-next-line posthog-js/passive-event-listeners - window?.addEventListener('beforeunload', () => { - if (this._canUseSessionStorage()) { - sessionStore.remove(this._primary_window_exists_storage_key) - } - }) + addEventListener( + window, + 'beforeunload', + () => { + if (this._canUseSessionStorage()) { + sessionStore.remove(this._primary_window_exists_storage_key) + } + }, + // Not making it passive to try and force the browser to handle this before the page is unloaded + { capture: false } + ) } /* diff --git a/src/utils/index.ts b/src/utils/index.ts index 13bff6374..8c7dca190 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -241,3 +241,18 @@ export function find(value: T[], predicate: (value: T) => boolean): T | undef } return undefined } + +// Use this instead of element.addEventListener to avoid eslint errors +// this properly implements the default options for passive event listeners +export function addEventListener( + element: Window | Document | Element | undefined, + event: string, + callback: EventListener, + options?: AddEventListenerOptions +): void { + const { capture = false, passive = true } = options ?? {} + + // TODO: Remove need for this by asserting that `passive` is being passed/used + // eslint-disable-next-line posthog-js/passive-event-listeners + element?.addEventListener(event, callback, { capture, passive }) +} From c682e3e3c87d0d72d6467a9c1b1f07d93f703d47 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 27 Jan 2025 12:26:15 -0300 Subject: [PATCH 5/8] refactor: Move to `no-add-event-listener` rule Force the new helper usage instead --- eslint-rules/custom-eslint-rules.test.js | 32 +++++----- eslint-rules/no-add-event-listener.js | 59 +++++++++++++++++++ eslint-rules/passive-event-listeners.js | 75 ------------------------ src/entrypoints/recorder.ts | 2 +- src/utils/index.ts | 5 +- 5 files changed, 79 insertions(+), 94 deletions(-) create mode 100644 eslint-rules/no-add-event-listener.js delete mode 100644 eslint-rules/passive-event-listeners.js diff --git a/eslint-rules/custom-eslint-rules.test.js b/eslint-rules/custom-eslint-rules.test.js index 852a3b752..654192103 100644 --- a/eslint-rules/custom-eslint-rules.test.js +++ b/eslint-rules/custom-eslint-rules.test.js @@ -7,7 +7,7 @@ const noDirectStringCheck = require('./no-direct-string-check') const noDirectDateCheck = require('./no-direct-date-check') const noDirectNumberCheck = require('./no-direct-number-check') const noDirectBooleanCheck = require('./no-direct-boolean-check') -const passiveEventListeners = require('./passive-event-listeners') +const noAddEventListener = require('./no-add-event-listener') const { RuleTester } = require('eslint') @@ -132,36 +132,36 @@ ruleTester.run('no-direct-boolean-check', noDirectBooleanCheck, { ], }) -ruleTester.run('passive-event-listeners', passiveEventListeners, { +ruleTester.run('no-add-event-listener', noAddEventListener, { valid: [ - { code: "document.addEventListener('click', () => {}, { passive: true })" }, - { code: "document.addEventListener('scroll', () => {}, { capture: true, passive: true })" }, + { code: "addEventListener(document, 'click', () => {}, { passive: true })" }, + { code: "addEventListener(window, 'scroll', () => {}, { capture: true, passive: true })" }, ], invalid: [ { code: "document.addEventListener('mouseleave', () => {})", - errors: [{ message: 'addEventListener should include { passive: true } as the third argument' }], - output: "document.addEventListener('mouseleave', () => {}, { passive: true })", + errors: [{ message: 'Use addEventListener from @utils instead of calling it directly on elements' }], + output: "import { addEventListener } from './utils'\naddEventListener(document, 'mouseleave', () => {})", }, { - code: "document.addEventListener('click', () => {}, true)", - errors: [{ message: 'addEventListener should use an options object including { passive: true }' }], - output: "document.addEventListener('click', () => {}, { capture: true, passive: true })", + code: "element.addEventListener('click', () => {}, true)", + errors: [{ message: 'Use addEventListener from @utils instead of calling it directly on elements' }], + output: "import { addEventListener } from './utils'\naddEventListener(element, 'click', () => {}, { capture: true })", }, { - code: "document.addEventListener('click', () => {}, {})", - errors: [{ message: 'addEventListener should have { passive: true } in its options' }], - output: "document.addEventListener('click', () => {}, { passive: true })", + code: "window.addEventListener('click', () => {}, {})", + errors: [{ message: 'Use addEventListener from @utils instead of calling it directly on elements' }], + output: "import { addEventListener } from './utils'\naddEventListener(window, 'click', () => {}, {})", }, { code: "document.addEventListener('pageleave', () => {}, { capture: true })", - errors: [{ message: 'addEventListener should have { passive: true } in its options' }], - output: "document.addEventListener('pageleave', () => {}, { capture: true, passive: true })", + errors: [{ message: 'Use addEventListener from @utils instead of calling it directly on elements' }], + output: "import { addEventListener } from './utils'\naddEventListener(document, 'pageleave', () => {}, { capture: true })", }, { code: "document.addEventListener('pageleave', () => {}, { capture: false })", - errors: [{ message: 'addEventListener should have { passive: true } in its options' }], - output: "document.addEventListener('pageleave', () => {}, { capture: false, passive: true })", + errors: [{ message: 'Use addEventListener from @utils instead of calling it directly on elements' }], + output: "import { addEventListener } from './utils'\naddEventListener(document, 'pageleave', () => {}, { capture: false })", }, ], }) diff --git a/eslint-rules/no-add-event-listener.js b/eslint-rules/no-add-event-listener.js new file mode 100644 index 000000000..a6c539ae2 --- /dev/null +++ b/eslint-rules/no-add-event-listener.js @@ -0,0 +1,59 @@ +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'Enforce usage of addEventListener from @utils instead of native addEventListener', + category: 'Best Practices', + recommended: true, + }, + fixable: 'code', + schema: [], // no options + }, + + create(context) { + return { + CallExpression(node) { + // Check if it's an addEventListener call + const callee = node.callee + if (callee.type === 'MemberExpression' && callee.property.name === 'addEventListener') { + context.report({ + node, + message: 'Use addEventListener from @utils instead of calling it directly on elements', + fix(fixer) { + // Get the element expression + const elementText = context.getSourceCode().getText(callee.object) + + // Get the event type + const eventText = context.getSourceCode().getText(node.arguments[0]) + + // Get the callback + const callbackText = context.getSourceCode().getText(node.arguments[1]) + + // Get options if they exist + const optionsText = + node.arguments[2] != null + ? context.getSourceCode().getText(node.arguments[2]) === 'true' + ? ', { capture: true }' + : `, ${context.getSourceCode().getText(node.arguments[2])}` + : '' + + // Add import if needed (note: this is a basic implementation, it won't always work) + const importFix = fixer.insertTextBefore( + context.getSourceCode().ast, + "import { addEventListener } from './utils'\n" + ) + + // Replace the call + const callFix = fixer.replaceText( + node, + `addEventListener(${elementText}, ${eventText}, ${callbackText}${optionsText})` + ) + + return [importFix, callFix] + }, + }) + } + }, + } + }, +} diff --git a/eslint-rules/passive-event-listeners.js b/eslint-rules/passive-event-listeners.js deleted file mode 100644 index 5d31429d2..000000000 --- a/eslint-rules/passive-event-listeners.js +++ /dev/null @@ -1,75 +0,0 @@ -module.exports = { - meta: { - type: 'suggestion', - docs: { - description: 'Enforce passive event listeners for better scroll performance', - category: 'Best Practices', - recommended: true, - }, - fixable: 'code', - schema: [], // no options - }, - - create(context) { - return { - CallExpression(node) { - // Check if it's an addEventListener call - const callee = node.callee - if (callee.type === 'MemberExpression' && callee.property.name === 'addEventListener') { - // Check if there's a third argument (options) - if (node.arguments.length < 3) { - context.report({ - node, - message: 'addEventListener should include { passive: true } as the third argument', - fix(fixer) { - return fixer.insertTextAfterRange( - [node.arguments[1].range[1], node.arguments[1].range[1]], - ', { passive: true }' - ) - }, - }) - return - } - - // Handle the case where the third argument is a boolean (capture) - const options = node.arguments[2] - if (options.type === 'Literal' && typeof options.value === 'boolean') { - context.report({ - node, - message: 'addEventListener should use an options object including { passive: true }', - fix(fixer) { - return fixer.replaceText(options, `{ capture: ${options.value}, passive: true }`) - }, - }) - return - } - - // Check if the third argument is an object with passive: true - if ( - options.type === 'ObjectExpression' && - !options.properties.some( - (prop) => - prop.key.name === 'passive' && - prop.value.type === 'Literal' && - prop.value.value === true - ) - ) { - context.report({ - node, - message: 'addEventListener should have { passive: true } in its options', - fix(fixer) { - if (options.properties.length === 0) { - return fixer.replaceText(options, '{ passive: true }') - } - return fixer.insertTextAfter( - options.properties[options.properties.length - 1], - ', passive: true' - ) - }, - }) - } - } - }, - } - }, -} diff --git a/src/entrypoints/recorder.ts b/src/entrypoints/recorder.ts index 9cef36e60..7d2761960 100644 --- a/src/entrypoints/recorder.ts +++ b/src/entrypoints/recorder.ts @@ -280,7 +280,7 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required { if (xhr.readyState !== xhr.DONE) { return diff --git a/src/utils/index.ts b/src/utils/index.ts index 8c7dca190..10ba3bf3c 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -252,7 +252,8 @@ export function addEventListener( ): void { const { capture = false, passive = true } = options ?? {} - // TODO: Remove need for this by asserting that `passive` is being passed/used - // eslint-disable-next-line posthog-js/passive-event-listeners + // This is the only place where we are allowed to call this function + // because the whole idea is that we should be calling this instead of the built-in one + // eslint-disable-next-line posthog-js/no-add-event-listener element?.addEventListener(event, callback, { capture, passive }) } From 433510b4114202fd8eb12e0d3636560ad46c8011 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 27 Jan 2025 12:32:52 -0300 Subject: [PATCH 6/8] fix: Stop creating wrapper function This prevents removing the listener when we stop it --- src/entrypoints/dead-clicks-autocapture.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/entrypoints/dead-clicks-autocapture.ts b/src/entrypoints/dead-clicks-autocapture.ts index 1af7a6f09..b62033a38 100644 --- a/src/entrypoints/dead-clicks-autocapture.ts +++ b/src/entrypoints/dead-clicks-autocapture.ts @@ -99,15 +99,11 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture } private _startClickObserver() { - const clickHandler = (event: Event) => { - this._onClick(event as MouseEvent) - } - - addEventListener(assignableWindow, 'click', clickHandler) + addEventListener(assignableWindow, 'click', this._onClick) } - private _onClick = (event: MouseEvent): void => { - const click = asClick(event) + private _onClick = (event: Event): void => { + const click = asClick(event as MouseEvent) if (!isNull(click) && !this._ignoreClick(click)) { this._clicks.push(click) } From 0d5e79c18439f188a8a3ab24fe3193bbccd7883b Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 27 Jan 2025 12:35:19 -0300 Subject: [PATCH 7/8] refactor: Decrease bundle size by 3% --- src/entrypoints/external-scripts-loader.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/entrypoints/external-scripts-loader.ts b/src/entrypoints/external-scripts-loader.ts index 93c11ffb9..0b0917ecd 100644 --- a/src/entrypoints/external-scripts-loader.ts +++ b/src/entrypoints/external-scripts-loader.ts @@ -1,5 +1,4 @@ import type { PostHog } from '../posthog-core' -import { addEventListener } from '../utils' import { assignableWindow, document, PostHogExtensionKind } from '../utils/globals' import { createLogger } from '../utils/logger' @@ -42,7 +41,10 @@ const loadScript = (posthog: PostHog, url: string, callback: (error?: string | E if (document?.body) { addScript() } else { - addEventListener(document, 'DOMContentLoaded', addScript) + // Inlining this because we don't care about `passive: true` here + // and this saves us ~3% of the bundle size + // eslint-disable-next-line posthog-js/no-add-event-listener + document?.addEventListener('DOMContentLoaded', addScript) } } From e1e109fbd3e417a3a63e53e03fdc65cc7397bd31 Mon Sep 17 00:00:00 2001 From: Rafa Audibert Date: Mon, 27 Jan 2025 14:10:20 -0300 Subject: [PATCH 8/8] feat: Add demo file for IE11 --- playground/ie11/index.html | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 playground/ie11/index.html diff --git a/playground/ie11/index.html b/playground/ie11/index.html new file mode 100644 index 000000000..ae43259f2 --- /dev/null +++ b/playground/ie11/index.html @@ -0,0 +1,25 @@ + + + + + + + + + + +

This page triggers a simple event using E5 bundle

+
+ Did it work? Check console +
+
+ You can also look at `posthog.something` to check if listeners were properly added to the window/document + object, such as `posthog.scrollManager.scrollX()` +
+ + + \ No newline at end of file