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

rafaeelaudibert · 2025-01-24T19:40:43Z

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.

I've released this for scroll events which are more critical - called hundreds of times a second - and I'm monitoring to see if we'll decrease event ingestion count or not. If I don't see a decrease, I'll ship this as well :)

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.

vercel · 2025-01-24T19:40:48Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
posthog-js	✅ Ready (Inspect)	Visit Preview	Jan 27, 2025 5:14pm

github-actions · 2025-01-24T19:44:56Z

Size Change: -6.39 kB (-0.19%)

Total Size: 3.27 MB

Filename	Size	Change
`dist/all-external-dependencies.js`	209 kB	+17 B (+0.01%)
`dist/array.full.es5.js`	265 kB	-654 B (-0.25%)
`dist/array.full.js`	370 kB	-729 B (-0.2%)
`dist/array.full.no-external.js`	369 kB	-729 B (-0.2%)
`dist/array.js`	181 kB	-611 B (-0.34%)
`dist/array.no-external.js`	180 kB	-611 B (-0.34%)
`dist/dead-clicks-autocapture.js`	14.5 kB	+67 B (+0.47%)
`dist/main.js`	182 kB	-611 B (-0.33%)
`dist/module.full.js`	370 kB	-729 B (-0.2%)
`dist/module.full.no-external.js`	369 kB	-729 B (-0.2%)
`dist/module.js`	181 kB	-611 B (-0.34%)
`dist/module.no-external.js`	180 kB	-611 B (-0.34%)
`dist/surveys-preview.js`	67.6 kB	+89 B (+0.13%)
`dist/surveys.js`	64.2 kB	+61 B (+0.1%)

ℹ️ View Unchanged

Filename	Size
`dist/customizations.full.js`	13.8 kB
`dist/exception-autocapture.js`	9.48 kB
`dist/external-scripts-loader.js`	2.64 kB
`dist/recorder-v2.js`	117 kB
`dist/recorder.js`	117 kB
`dist/tracing-headers.js`	1.76 kB
`dist/web-vitals.js`	10.4 kB

_{compressed-size-action}

robbie-c

LGTM!

Is the bundle size smaller if you make a { passive: true} object somewhere and import it?

pauldambra · 2025-01-27T10:20:07Z

src/entrypoints/recorder.ts

+                                //
+                            })
+                    },
+                    { passive: true }


can i be precious and we not change this... changes in here can really break customers' sites so i change it very carefully 🙈

Ah good catch

I'm happy not to change it here, I agree this is the finickiest :)

yep, just needs way more careful testing
i'm being too cautious but the last time we broke this was very painful 🙈

so, best to avoid entirely :)

It's slightly more performant to have these in our events, see previous commit for rationale

rafaeelaudibert · 2025-01-27T13:19:23Z

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 {


So, I was trying to understand what this function is for, and it's intended to support extremely old browsers. If you check https://caniuse.com/?search=addEventListener you'll see this is only ever useful for IE 6-8 which we supposedly 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. https://github.com/PostHog/posthog-js/blob/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. Anything against removing this code?

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.

@pauldambra @robbie-c

nope... that's super early code for sure...

we should check the ES5 bundle still works but hopefully CI just covers that.

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.

This helper is intended to save us some bytes by avoiding having to repeat `passive: true` everywhere

Force the new helper usage instead

This prevents removing the listener when we stop it

rafaeelaudibert · 2025-01-27T15:36:40Z

eslint-rules/no-add-event-listener.js

+                            // 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"
+                            )


Wondering if this makes any sense. Most often than not it'll be correct, but it will be a broken import sometimes

Maybe we shouldn't be autofixing this at all?

yeah, I'd leave the fix (if we can't detect the correct path). most folks' editors should be able to create the import on demand anyway

I'll see if eslint allows me to detect what file I'm in, if so, I'll keep it

rafaeelaudibert · 2025-01-27T15:37:54Z

@robbie-c @pauldambra Added a couple of new commits to reduce bundle size, and saved some bytes everywhere - even compared to before!

robbie-c

What @pauldambra said about the ES5 bundle still working for people. Do you still have your IE11 setup? Can you make sure that we're still tracking scrolls correctly?

(I'm 99% confident we will be, just... you know.. changes with a large impact if something goes wrong)

pauldambra

yep, running locally I still see heatmaps and scrolls and things being captured

CI is green 👍

rafaeelaudibert · 2025-01-27T16:26:37Z

@pauldambra's tests are reassuring, and scroll still works on my IE11 from the change last week, I'll try the changes here locally, let's hope my IE11 installation is trustworthy

rafaeelaudibert · 2025-01-27T16:37:07Z

Hmm, I can't run the demo NextJS app on IE11 because it uses react-refresh and that doesn't support IE11. Trying something else, testing IE11 sucks

rafaeelaudibert · 2025-01-27T17:11:12Z

Ok, found a way, e1e109f (#1689), but it's hacky, it does work, though! 🎉

rafaeelaudibert · 2025-01-27T18:15:50Z

Will be merging this tomorrow morning, more time to monitor before I leave for the day

rafaeelaudibert added the bump patch Bump patch version when this PR gets merged label Jan 24, 2025

rafaeelaudibert requested a review from a team January 24, 2025 19:40

vercel bot deployed to Preview January 24, 2025 19:43 View deployment

robbie-c reviewed Jan 27, 2025

View reviewed changes

pauldambra reviewed Jan 27, 2025

View reviewed changes

refactor: Apply new passive-event-listeners rule

d9719d0

It's slightly more performant to have these in our events, see previous commit for rationale

rafaeelaudibert force-pushed the make-all-event-listeners-passive branch from 40e54f1 to d9719d0 Compare January 27, 2025 12:53

vercel bot deployed to Preview January 27, 2025 12:57 View deployment

rafaeelaudibert commented Jan 27, 2025

View reviewed changes

rafaeelaudibert added 2 commits January 27, 2025 10:31

refactor: Use addEventListener helper

1453307

This helper is intended to save us some bytes by avoiding having to repeat `passive: true` everywhere

vercel bot deployed to Preview January 27, 2025 13:53 View deployment

rafaeelaudibert added 3 commits January 27, 2025 12:26

refactor: Move to no-add-event-listener rule

c682e3e

Force the new helper usage instead

fix: Stop creating wrapper function

433510b

This prevents removing the listener when we stop it

refactor: Decrease bundle size by 3%

0d5e79c

rafaeelaudibert commented Jan 27, 2025

View reviewed changes

rafaeelaudibert requested review from robbie-c and pauldambra January 27, 2025 15:36

vercel bot deployed to Preview January 27, 2025 15:40 View deployment

robbie-c approved these changes Jan 27, 2025

View reviewed changes

pauldambra approved these changes Jan 27, 2025

View reviewed changes

feat: Add demo file for IE11

e1e109f

vercel bot deployed to Preview January 27, 2025 17:14 View deployment

rafaeelaudibert merged commit 2c3eb70 into main Jan 29, 2025
28 checks passed

rafaeelaudibert deleted the make-all-event-listeners-passive branch January 29, 2025 12:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

rafaeelaudibert commented Jan 24, 2025

vercel bot commented Jan 24, 2025 •

edited

Loading

github-actions bot commented Jan 24, 2025 •

edited

Loading

robbie-c left a comment

pauldambra Jan 27, 2025

robbie-c Jan 27, 2025

rafaeelaudibert Jan 27, 2025

pauldambra Jan 27, 2025

pauldambra Jan 27, 2025

rafaeelaudibert Jan 27, 2025

pauldambra Jan 27, 2025

rafaeelaudibert Jan 27, 2025

pauldambra Jan 27, 2025

rafaeelaudibert Jan 27, 2025

rafaeelaudibert commented Jan 27, 2025

robbie-c left a comment

pauldambra left a comment

rafaeelaudibert commented Jan 27, 2025

rafaeelaudibert commented Jan 27, 2025

rafaeelaudibert commented Jan 27, 2025

rafaeelaudibert commented Jan 27, 2025

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

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

Conversation

rafaeelaudibert commented Jan 24, 2025

vercel bot commented Jan 24, 2025 • edited Loading

github-actions bot commented Jan 24, 2025 • edited Loading

robbie-c left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaeelaudibert commented Jan 27, 2025

robbie-c left a comment

Choose a reason for hiding this comment

pauldambra left a comment

Choose a reason for hiding this comment

rafaeelaudibert commented Jan 27, 2025

rafaeelaudibert commented Jan 27, 2025

rafaeelaudibert commented Jan 27, 2025

rafaeelaudibert commented Jan 27, 2025

vercel bot commented Jan 24, 2025 •

edited

Loading

github-actions bot commented Jan 24, 2025 •

edited

Loading