fix(autocapture): cut click-handler latency on deep DOMs and long lists#3636
Draft
posthog[bot] wants to merge 1 commit into
Draft
fix(autocapture): cut click-handler latency on deep DOMs and long lists#3636posthog[bot] wants to merge 1 commit into
posthog[bot] wants to merge 1 commit into
Conversation
The shared ancestor walk in `getElementAndParentsForElement` called `window.getComputedStyle` on every non-clickable ancestor to check for `cursor: pointer`. Each call forces a synchronous style/layout recalc, and all three click handlers (autocapture, heatmaps, dead-clicks) hit this path on every click. On a 500-row Angular table with heavy CSS this compounded into 10s of lag per click; deactivating PostHog made the lag disappear. Two improvements: - Short-circuit the `getComputedStyle` call once `parentIsUsefulElement` becomes true (the flag never goes back to false, so additional calls are wasted) and let callers that ignore `parentIsUsefulElement` (`shouldCaptureDeadClick`, `shouldCaptureRageclick`, `shouldSkipDeadClick`) skip the check entirely via a new `checkParentIsUseful` parameter. - Cap the `nth_child` / `nth_of_type` sibling walk in `getPropertiesFromElement` to a fixed depth (`MAX_SIBLING_WALK = 100`). The walk is done once per ancestor of the click target, so a 500-sibling list parent multiplies into thousands of DOM accesses on every click; the capped position is still useful for grouping. Tests assert that the dead-click / rage-click paths do not call `getComputedStyle`, that `shouldCaptureDomEvent` stops calling it once a useful parent is found, and that `nth_child` is bounded on long lists. Generated-By: PostHog Code Task-Id: eb26d4b7-463b-4b08-a6b7-dbf638c8788d
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Contributor
|
Size Change: +492 B (0%) Total Size: 16 MB
ℹ️ View Unchanged
|
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Customer reports (Zendesk #57815, plus a user telling our in-product Max assistant verbatim "post hog records everything and makes my site slow how to stop that") show that posthog-js makes click-heavy pages with deep DOMs unusable: 10 second click lag on a 1-row "Mass Actions" list scaling to a full hang at 500 rows, lag disappearing the moment PostHog is deactivated. Angular's
runOutsideAngularand disabling network activity don't help, so the cost is synchronous CPU on the JS main thread inside our own click handlers.Root cause is on the autocapture click path:
getElementAndParentsForElement(packages/browser/src/autocapture-utils.ts) walks the click target's ancestors and callswindow.getComputedStyle(parentNode)on every non-clickable ancestor to check forcursor: pointer. Each call forces a synchronous style/layout recalc.getPropertiesFromElement(packages/browser/src/autocapture.ts) walks every previous sibling of every ancestor to computenth_child/nth_of_type. On a 500-row contact list that's hundreds of sibling iterations per ancestor.Changes
Two focused changes on the hot path. The cross-handler shared-walk refactor suggested in the signal report is deferred — too invasive for a fix targeted at this regression.
getElementAndParentsForElement— skip wastedgetComputedStylecallscursor: pointerlookup onceparentIsUsefulElementbecomes true. The flag never goes back to false, so additionalgetComputedStylecalls were always wasted.checkParentIsUsefulparameter (defaulttrue) so callers that don't readparentIsUsefulElementcan skip the lookup entirely. Updated callers:shouldCaptureDeadClick,shouldCaptureRageclick,shouldSkipDeadClick.getPropertiesFromElement— cap the sibling walkMAX_SIBLING_WALK = 100. The walk runs once per ancestor of the click target, so a 500-sibling list parent multiplies into thousands of DOM accesses per click. The capped position is still useful for grouping events; rows past position 100 are effectively interchangeable for analytics purposes.Tests
shouldCaptureDeadClickandshouldCaptureRageclickno longer callgetComputedStyle(regression guard for the new fast path).shouldCaptureDomEventstops callinggetComputedStyleonce it finds a useful parent (regression guard for the short-circuit).nth_childis bounded on a 500-sibling parent (regression guard for the sibling cap).The full autocapture and autocapture-utils unit-test suites pass locally.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset fileCreated with PostHog Code