feat: add autocapture pageviews with history api #446
Conversation
|
Size Change: +1.83 kB (+1.09%) Total Size: 170 kB
ℹ️ View Unchanged
|
lricoy
force-pushed
the
feat/add-history-api-autocapture-pageviews
branch
from
5dc9f12 to
9f84c7c
Compare
There was a problem hiding this comment.
I do love how easy this is for those using the History API, excellent job.
Main question: on our main posthog-js library we put a lot of effort into making sure we're tracking time spent in session and blahblahblah for SPAs, by sending some extra information in follow-up $pageview events. Are we doing the same in this lib? If not, we'll need to port that to this lib as well
Things that popped in my head
trackHistoryEvents is not a property that exists in our main posthog-js library, and I'm afraid I wouldn't want this library to have more features than our main lib.
I'm in favor of adding this feature to both as that might simplify some of our NextJS setup. That might be a slightly bigger undertaking, though, since you'll need to update a lot of docs :)
For dan in specific, he should be able to simply use the lite version and call the tracking automatically while this isn't merged - just like you did.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
The PR introduces a new feature for automatically capturing navigation events via the History API, updating types, documentation, implementation, and tests accordingly.
- Updated
posthog-web/src/types.tsto include the newtrackHistoryEventsoption. - Modified
posthog-web/README.mdto document History API tracking with examples. - Implemented overriding of history methods and added popstate listener in
posthog-web/src/posthog-web.ts. - Added comprehensive tests for navigation events in
posthog-web/test/posthog-web.spec.ts. - Changelog in
posthog-web/CHANGELOG.mdnow reflects the added History API tracking feature.
5 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
That makes total sense, thanks! If approved, I was planning on porting that to posthog-js as well, as it seems useful enough. I'll probably add some more work to handle I will take a look at those extra features on posthog-js as well, thanks! |
posthog-web/README.md
Outdated
| const posthog = new PostHog('my-api-key', { | ||
| /* options, e.g. for self-hosted users */ | ||
| // host: "https://my-posthog.app.com" | ||
| // trackHistoryEvents: true // Enable tracking of History API navigation events as pageviews |
There was a problem hiding this comment.
trackHistoryEvents is not a property that exists in our main posthog-js library, and I'm afraid I wouldn't want this library to have more features than our main lib.
i'm ok with adding this here if we then also add it in posthog-js... it's a super cool improvement
(we should definitely play with it running agains posthog locally since that's a SPA to figure out if it does anything weird but that's only me being cautious :))
in the main lib we tend to call config options captureX so might be worth keeping that here and calling it captureHistoryEvents seems nit-picky but the cognitive overload of config grows over time :)
There was a problem hiding this comment.
Will apply the changes, I've bundled this version into a repo to give it a few shots as well: https://lricoy.github.io/test-posthog-history/
There was a problem hiding this comment.
| // trackHistoryEvents: true // Enable tracking of History API navigation events as pageviews | |
| // captureHistoryEvents: true // Enable tracking of History API navigation events as pageviews |
posthog-web/src/posthog-web.ts
Outdated
| const originalPushState = window.history.pushState | ||
| const originalReplaceState = window.history.replaceState | ||
|
|
||
| window.history.pushState = (state, title, url) => { |
There was a problem hiding this comment.
in the main lib we copied a patch function from rrweb, which was in turn originally copied from sentry
(but there are hundreds out there)
https://github.com/PostHog/posthog-js/blob/main/src/extensions/replay/rrweb-plugins/patch.ts
the main thing to copy from that would be marking the thing as wrapped
https://github.com/PostHog/posthog-js/blob/main/src/extensions/replay/rrweb-plugins/patch.ts#L24-L32
again seems nit-picky but I really struggled to fix bugs between Angular's Zone detection stuff and replay because it was hard to track what was wrapped, so it's nice for the future traveller to be able to easily see that something isn't the original when debugging something
also means we can check for that and not wrap the function a second time if this gets called more than once
posthog-web/src/posthog-web.ts
Outdated
| return | ||
| } | ||
|
|
||
| this.capture('$pageview', { navigation_type: navigationType }) |
There was a problem hiding this comment.
to Rafa's point about there being lots of good stuff on page view in the main lib
we should check if there's anything we can very easily add here
but otherwise it's also fine for us to say "yep, lite doesn't do that, you can use the full fat posthog to get that"
There was a problem hiding this comment.
Checking this now to see if we can easily grab something there.
There was a problem hiding this comment.
I think the features @rafaeelaudibert mentioned are mostly handled by the PageViewManager/ScrollManager which does not seem crazy to have it here as well.
The page view durations and increments don't even need the ScrollManager bits; they are calculated based on the last pageview ref that is kept around on the PageViewManager.
But there is also session-related stuff that is way way simpler here than what we have on both SessionIdManager
and SessionPropsManager... So I am not sure how much we should port without documenting or keeping track of the differences.
WDYT @robbie-c ?
I'm inclined to say, "Yep, lite doesn't do that," for now, but a direction for our future work would be cool, even if just for docs.
There was a problem hiding this comment.
Hmm.
We don't actually use the scroll information or time on page in web analytics - so I'd be tempted to leave it out here actually!
We haven't exactly defined what posthog-js-lite is for, but one version I would propose would be: "the smallest bundle size possible where everything in web analytics still works", wdyt?
There was a problem hiding this comment.
(I'm pretty sure we don't meet that goal right now, FWIW, e.g. I don't think we handle $pageleave events in this library)
There was a problem hiding this comment.
i'd say "the smallest bundle where basic analytics and flags work"
that way we're not tied to chasing a feature set for a subset of customers who want feature X, y, and Z and a tiny bundle
this is the "i can live with almost nothing" bundle
There was a problem hiding this comment.
The argument of whether session props are "basic analytics" or not can be made. I think it would be worth keeping at least that in sync between js and lite, as it is very helpful for attribution analysis, and although not as lite, having the session definitions is core to a lot of our metrics from what I can tell... It would involve way more testing, than what's in this PR IMO.
If you agree, I can take a better look later at how to add that and still keep the bundle size in check later on. Sounds like a good plan for me.
|
this is super cool though @lricoy , love it! |
lricoy
force-pushed
the
feat/add-history-api-autocapture-pageviews
branch
from
cb8b1b1 to
56df28c
Compare
|
For reference, I've started the work on posthog-js yesterday on this PR: PostHog/posthog-js#1886 I will wrap it up later today. |
|
Merging this as it is a simpler change, while PostHog/posthog-js#1886 still waits for a ✅ |
Problem
This Bluesky thread gave us the idea: https://bsky.app/profile/danabra.mov/post/3lmflnn62ys2y to have a tiny sdk option for simple navigation event capture.
This internal Slack thread also has some comments, it is my first SDK PR: https://posthog.slack.com/archives/C03P7NL6RMW/p1744225093576939
I've bundled a simple repo to test this out: https://lricoy.github.io/test-posthog-history/
Changes
Added an option to attach listeners to the browser's history api and autocapture pageviews.
Release info Sub-libraries affected
Bump level
Libraries affected
Changelog notes
trackHistoryEventsoption to automatically capture navigation events in single-page applications using the History API.