Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add pageview and prev pageview tracking #1634

Merged
merged 5 commits into from
Jan 3, 2025
Merged

Conversation

robbie-c
Copy link
Member

@robbie-c robbie-c commented Jan 2, 2025

Changes

  • Keep track of the last pagview event id
  • Send the pageview event id with every event

This came from a discussion with @orian who wanted to be able to associate all of the query completed events that were triggered by the same pageview

Checklist

  • Tests for new code (see advice on the tests we use)
  • Accounted for the impact of any changes across different browsers
  • Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

Copy link

vercel bot commented Jan 2, 2025

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

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Jan 3, 2025 0:51am

@robbie-c robbie-c changed the title Add pageview and prev pageview tracking feat: Add pageview and prev pageview tracking Jan 2, 2025
Copy link

github-actions bot commented Jan 2, 2025

Size Change: +4.38 kB (+0.14%)

Total Size: 3.23 MB

Filename Size Change
dist/array.full.es5.js 262 kB +482 B (+0.18%)
dist/array.full.js 366 kB +434 B (+0.12%)
dist/array.full.no-external.js 364 kB +434 B (+0.12%)
dist/array.js 180 kB +433 B (+0.24%)
dist/array.no-external.js 178 kB +433 B (+0.24%)
dist/main.js 180 kB +433 B (+0.24%)
dist/module.full.js 366 kB +434 B (+0.12%)
dist/module.full.no-external.js 364 kB +434 B (+0.12%)
dist/module.js 180 kB +433 B (+0.24%)
dist/module.no-external.js 178 kB +433 B (+0.24%)
ℹ️ View Unchanged
Filename Size
dist/all-external-dependencies.js 206 kB
dist/customizations.full.js 12.6 kB
dist/dead-clicks-autocapture.js 14.4 kB
dist/exception-autocapture.js 9.48 kB
dist/external-scripts-loader.js 2.64 kB
dist/recorder-v2.js 115 kB
dist/recorder.js 115 kB
dist/surveys-preview.js 57.6 kB
dist/surveys.js 63.3 kB
dist/tracing-headers.js 1.76 kB
dist/web-vitals.js 10.4 kB

compressed-size-action

@robbie-c robbie-c added the bump patch Bump patch version when this PR gets merged label Jan 2, 2025
@robbie-c robbie-c requested review from orian, pauldambra and a team January 2, 2025 20:25
src/utils/index.ts Outdated Show resolved Hide resolved
src/posthog-core.ts Outdated Show resolved Hide resolved
@@ -17,40 +20,45 @@ interface PageViewEventProperties {
}

export class PageViewManager {
_currentPath?: string
_prevPageviewTimestamp?: Date
_currentPageview?: { timestamp: Date; pageViewId: string | undefined; pathname: string | undefined }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brain is not fully engaged so sorry if this is a silly question

this is entirely in memory, no? so if my app does full page loads on navigation (i.e. not a SPA) then I won't have a previous pageview id?

or am i misunderstanding?

otherwise the logic for tracking it looks valid to me

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true - I'll add a comment about why this is OK

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! thank you! 💖

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't run it but that looks safe and correct to me 🚀

@robbie-c robbie-c merged commit a3da178 into main Jan 3, 2025
29 checks passed
@robbie-c robbie-c deleted the feat/add-pageview-ids branch January 3, 2025 14:31
@orian
Copy link

orian commented Jan 6, 2025

To query events and group the query ClickHouse' events table:

SELECT 
    JSONExtractString(properties, '$pageview_id') as pageview_id, 
    uuid, event, 
    timestamp,
    JSONExtractString(properties, 'duration'),
    JSONExtractString(properties, '$time'),
    properties
FROM events
WHERE team_id=1
    AND event='query completed' and timestamp > '2025-01-06'
    AND pageview_id <> ''
--    AND pageview_id = '<pageview id here>'
ORDER BY pageview_id
LIMIT 100;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants