feat: analytics auto-tracking helpers for Web and Flutter#1622
Open
lohanidamodar wants to merge 4 commits into
Open
feat: analytics auto-tracking helpers for Web and Flutter#1622lohanidamodar wants to merge 4 commits into
lohanidamodar wants to merge 4 commits into
Conversation
Adds a hand-written companion to the generated `Analytics` service that wires up common auto-tracking behaviours (pageviews, outbound links, downloads, scroll depth, active engagement) on top of the standard `event()` endpoint. The helper is composition-based and takes a plain event-emitter callback so it does not hard-couple to the generated service class. All helpers respect `navigator.doNotTrack` by default and can be opted out. Public surface: - `AnalyticsTracking` — main entry point, wraps an emitter callback - `enableAutoPageviews()` / `disableAutoPageviews()` — SPA-aware pageview tracking that hooks `pushState`, `replaceState`, `popstate` and hashchange - `enableAutoOutboundTracking(options?)` — anchor-click detection for external links with optional subdomain handling - `enableAutoDownloadTracking(options?)` — anchor-click detection by file extension list - `enableAutoScrollDepth()` — max scroll % per page, flushed on unload and SPA route change - `enableAutoEngagementTime()` — active-time counter with visibility gating - `enableAllAutoTracking()` — opinionated default set (pageviews, outbound, scroll, engagement) - `track(name, options)` / `pageview(url, referrer)` — manual passthroughs The new template file is registered in `Web.getFiles()` and the class is re-exported from the SDK entry point.
Adds hand-written companions to the generated `Analytics` service for the
Flutter SDK, following the same composition-based pattern as the Web helpers.
The observer plugs into `MaterialApp.navigatorObservers` and fires an event
for every route push/pop/replace/remove; the tracking class listens to
`WidgetsBindingObserver` for app-backgrounded / app-foregrounded events and
exposes a mobile-idiomatic `screenView(name, {className, props})` shortcut.
Public surface:
- `AnalyticsEventEmit` typedef — the callback used to send events, keeping
the helpers decoupled from the generated service class
- `AnalyticsObserver extends NavigatorObserver` — auto screen-view tracking
with an optional `ScreenNameExtractor`
- `AnalyticsTracking` — `WidgetsBindingObserver`-driven lifecycle events,
plus `screenView()` and `event()` shortcuts and an `enableAllAutoTracking()`
composite
The new templates are registered in `Flutter.getFiles()` and re-exported from
the package entry point.
Contributor
Greptile SummaryThis PR adds hand-written auto-tracking helpers for the Analytics service to the Web and Flutter SDKs, following the same composition pattern used by the Realtime helpers. The helpers accept an event-emitter callback rather than coupling to the generated
Confidence Score: 4/5Safe to merge with one logic fix: the engagement timer starts unconditionally without checking initial tab visibility, which corrupts engagement metrics for background-tab opens. The Web engagement tracker calls templates/web/src/services/analytics-tracking.ts.twig — specifically the Important Files Changed
Reviews (3): Last reviewed commit: "fix(analytics): stop engagement timer on..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+412
to
+418
| private rootDomain(hostname: string): string { | ||
| const parts = hostname.split('.'); | ||
| if (parts.length <= 2) { | ||
| return hostname; | ||
| } | ||
| return parts.slice(-2).join('.'); | ||
| } |
Contributor
There was a problem hiding this comment.
rootDomain breaks for multi-part TLDs
parts.slice(-2).join('.') returns co.uk for both myapp.co.uk and google.co.uk, so any click from a .co.uk site (or .com.au, .com.br, .co.jp, etc.) to any other domain on the same TLD registers as internal and is silently dropped from outbound tracking. Concretely, from shop.example.co.uk, a click on https://google.co.uk returns rootDomain = co.uk on both sides, isExternal returns false, and no event is fired.
Comment on lines
+177
to
+182
| this.pageviewCleanups.push(() => { | ||
| history.pushState = originalPushState; | ||
| history.replaceState = originalReplaceState; | ||
| window.removeEventListener('popstate', popstateHandler); | ||
| window.removeEventListener('hashchange', hashchangeHandler); | ||
| }); |
Contributor
There was a problem hiding this comment.
history methods can clobber third-party router patches
disableAutoPageviews() restores history.pushState and history.replaceState to the references captured at enableAutoPageviews() call time. If a router (Next.js App Router, React Router, etc.) patched the same methods after this tracker initialised, the restore will silently discard that library's wrapper. The typical pattern for chaining patches is to keep the wrapped function and call through to it; restoring to a saved reference is safer only when no other library ever patches these globals.
Auto-emitted event names and prop keys now align with the analytics endpoint's canonical patterns: - Event names are snake_case + lowercase (`pageview`, `outbound_link`, `file_download`, `scroll_depth`, `engagement_time`, `screen_view`, `app_backgrounded`, `app_foregrounded`) rather than PascalCase strings with parametric suffixes. Data that was previously encoded into the event name (hostname for outbound links, filename for downloads) now moves into camelCase prop fields. - Web: add optional `scrollDepth` and `engagementTime` fields on `AnalyticsEventOptions` so the scroll-depth and engagement-time auto-trackers forward those values as top-level endpoint params rather than folding them into `props`. - Flutter: rename `AnalyticsEventEmit` -> `AnalyticsEventEmitter` for cross-platform typedef consistency with the Web SDK; rename the `screenView` `class` prop to `screenClass` so it follows the camelCase convention. - Docblocks on both platforms document the naming convention.
…ng before observer Address two bugs surfaced by review. Web: `flushEngagement` called `startEngagement()` at the end of every flush, including the pagehide flush that precedes back-forward-cache entry. When the tab was restored via bfcache the timer resumed with an engagementStart snapshotted before the freeze, so the next flush billed the entire bfcache dwell time as engagement. flushEngagement no longer auto-restarts; restart is now driven by explicit resume signals — pageshow with persisted === true, visibilitychange back to visible, or the SPA navigation flow. Flutter: `enableAutoLifecycleEvents()` touched `WidgetsBinding.instance` without ensuring the binding, so callers who followed the docstring example (construct + `enableAllAutoTracking()` before `runApp`) hit `StateError: Binding has not yet been initialized`. Call `WidgetsFlutterBinding.ensureInitialized()` before attaching the observer (idempotent). Also fold in a few smaller review findings: - Flutter: guard `app_backgrounded` on `_foregroundSince` so it emits once per background transition instead of firing a second empty event when the desktop/web lifecycle passes through both `hidden` and `paused`. - Web: drop image extensions (jpg/jpeg/png/gif/svg/webp) from the default download list — anchor-wrapped inline images (lightboxes, galleries) were being counted as file downloads. - Web: add `disableAutoOutboundTracking`, `disableAutoDownloadTracking`, `disableAutoScrollDepth`, `disableAutoEngagementTime` for parity with `disableAutoPageviews`, so consent flows can tear down individual trackers without discarding the whole `AnalyticsTracking` instance.
Member
Author
|
Thanks for the careful review — both P1s addressed, plus a few smaller items. Required fixes
Smaller cleanups folded in
Not tackled in this pass
Verified: PHPUnit unit suite passes, Rector dry-run clean, djLint clean, phpcs clean, regenerated Web SDK compiles with no |
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
1 participant
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.
Summary
Adds hand-written auto-tracking helpers for the Analytics service across the Web and Flutter SDKs, following the same pattern as Realtime (fully hand-written per platform, not generated from OpenAPI spec).
The helpers are composition-based: each takes a plain event-emitter callback rather than importing the generated service class directly, so they render and compile independently of any given spec revision. Users wire them up in one line:
What's included
Web (
templates/web/src/services/analytics-tracking.ts.twig)enableAutoPageviews()/disableAutoPageviews()— SPA-aware: initial load +pushState/replaceState/popstate/hashchangeenableAutoOutboundTracking(options?)— click detection on external anchor links, optional subdomain handlingenableAutoDownloadTracking(options?)— click detection on file-download links, extension list configurableenableAutoScrollDepth()— max scroll % per page, flushed on unload and on SPA route changeenableAutoEngagementTime()— active-time counter withvisibilitychangegatingenableAllAutoTracking()— opinionated default set (pageviews, outbound, scroll, engagement)track(name, options)/pageview(url, referrer)— manual passthroughsnavigator.doNotTrackby default (opt-out viarespectDoNotTrack: false)Flutter (
templates/flutter/lib/src/analytics_observer.dart.twig,analytics_tracking.dart.twig)AnalyticsObserver extends NavigatorObserver— auto screen-view tracking on route push / pop / replace / remove, with an optionalScreenNameExtractorfor filteringAnalyticsTracking—WidgetsBindingObserver-driven lifecycle events (app_backgrounded/app_foregrounded)screenView(name, {className, props})— mobile-idiomatic convenience for the pageview equivalentenableAutoLifecycleEvents()/disableAutoLifecycleEvents()/enableAllAutoTracking()Registrations
src/SDK/Language/Web.php— new template listed ingetFiles()src/SDK/Language/Flutter.php— both new templates listed ingetFiles()templates/web/src/index.ts.twig—AnalyticsTrackingand its supporting types re-exportedtemplates/flutter/lib/package.dart.twig— observer and tracking modules re-exportedNaming conventions
Auto-emitted event names and prop keys follow a single convention on both platforms so they compose cleanly with the analytics endpoint's parameter naming:
snake_case+ lowercase. The helpers own the names of the events they emit themselves:pageview,outbound_link,file_download,scroll_depth,engagement_time(Web),screen_view,app_backgrounded,app_foregrounded(Flutter). Callers can still pass any custom event name to the manualtrack()/event()methods; those strings are forwarded verbatim.camelCase, matching the endpoint's top-level parameter naming (hostname,href,filename,extension,screenClass,foregroundSeconds, etc.).AnalyticsEventOptionsexposesscrollDepthandengagementTimeas first-class option fields so the scroll-depth and engagement-time trackers forward them as endpoint params rather than folding them intoprops.AnalyticsEventEmitter(wasAnalyticsEventEmit), matching the Web SDK's exported name for cross-platform consistency.Notes
Analyticsservice only through user-supplied callbacks, so this PR is safe to merge before / after the spec adds the ingestion endpoint. Once the spec exposesanalytics.event(...), the wiring shown above is the intended usage.analytics.event(...)from the standard generated code path, so no hand-written helper is added there.Verification
composer test— 33/33 unit tests pass (vendor/bin/phpunit --testsuite Unit)composer refactor:check— clean (rector, dry run)composer lint-twig— clean (djlint over all 565 template files)composer lint— clean (phpcs)tsc --noEmitreports zero errors in the newanalytics-tracking.ts(pre-existing errors in other generated services are unrelated)flutter analyze lib/src/analytics_observer.dart lib/src/analytics_tracking.dartreports zero issues