feat: Add new rule to guarantee events are tracked as passive (#1689)

.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,

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 noAddEventListener = require('./no-add-event-listener')
 
 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('no-add-event-listener', noAddEventListener, {
+    valid: [
+        { code: "addEventListener(document, 'click', () => {}, { passive: true })" },
+        { code: "addEventListener(window, 'scroll', () => {}, { capture: true, passive: true })" },
+    ],
+    invalid: [
+        {
+            code: "document.addEventListener('mouseleave', () => {})",
+            errors: [{ message: 'Use addEventListener from @utils instead of calling it directly on elements' }],
+            output: "import { addEventListener } from './utils'\naddEventListener(document, 'mouseleave', () => {})",
+        },
+        {
+            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: "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: '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: 'Use addEventListener from @utils instead of calling it directly on elements' }],
+            output: "import { addEventListener } from './utils'\naddEventListener(document, 'pageleave', () => {}, { capture: false })",
+        },
+    ],
+})

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]
+                        },
+                    })
+                }
+            },
+        }
+    },
+}

playground/ie11/index.html

@@ -0,0 +1,25 @@
+<html>
+
+<head>
+    <!-- COPY AND PASTE FROM `dist/array.full.e5.js` TO TEST THIS -->
+    <script>
+    </script>
+
+    <script>
+        posthog.init('BrpS4SctoaCCsyjlnlun3OzyNJAafdlv__jUWaaJWXg', { api_host: 'https://us.posthog.com', debug: true })
+        posthog.capture("e5 event")
+    </script>
+</head>
+
+<body>
+    <h1>This page triggers a simple event using E5 bundle</h1>
+    <div>
+        Did it work? Check console
+    </div>
+    <div>
+        You can also look at `posthog.something` to check if listeners were properly added to the window/document
+        object, such as `posthog.scrollManager.scrollX()`
+    </div>
+</body>
+
+</html>

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

src/autocapture.ts

@@ -1,4 +1,4 @@
-import { each, extend, registerEvent } from './utils'
+import { addEventListener, 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)
+        addEventListener(document, 'submit', handler, { capture: true })
+        addEventListener(document, 'change', handler, { capture: true })
+        addEventListener(document, 'click', handler, { capture: 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)
+            }
+
+            addEventListener(document, 'copy', copiedTextHandler, { capture: true })
+            addEventListener(document, 'cut', copiedTextHandler, { capture: true })
         }
     }
 

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,11 +99,11 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
     }
 
     private _startClickObserver() {
-        assignableWindow.addEventListener('click', this._onClick)
+        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)
         }
@@ -122,7 +123,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 +142,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
     }
 
     private _startSelectionChangedObserver() {
-        assignableWindow.addEventListener('selectionchange', this._onSelectionChange)
+        addEventListener(assignableWindow, 'selectionchange', this._onSelectionChange)
     }
 
     private _onSelectionChange = (): void => {

src/entrypoints/external-scripts-loader.ts

@@ -41,6 +41,9 @@ const loadScript = (posthog: PostHog, url: string, callback: (error?: string | E
     if (document?.body) {
         addScript()
     } else {
+        // 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)
     }
 }

src/entrypoints/recorder.ts

@@ -278,6 +278,9 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required<Ne
                     return originalSend(body)
                 }
 
+                // This is very tricky code, and making it passive won't bring many performance benefits,
+                // so let's ignore the rule here.
+                // eslint-disable-next-line posthog-js/no-add-event-listener
                 xhr.addEventListener('readystatechange', () => {
                     if (xhr.readyState !== xhr.DONE) {
                         return
@@ -325,6 +328,7 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required<Ne
                             //
                         })
                 })
+
                 originalOpen.call(xhr, method, url, async, username, password)
             }
         }

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)
-            window?.addEventListener('offline', this._onOffline)
-            window?.addEventListener('online', this._onOnline)
-            window?.addEventListener('visibilitychange', this._onVisibilityChange)
+            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