Refactor: introduce a typed analytics facade over PostHog #35
Refactor: introduce a typed analytics facade over PostHog #35spiderocious wants to merge 1 commit into
Conversation
- Replaced all instances of PostHog's `usePostHog` with a new `useAnalytics` hook. - Created a centralized analytics facade to handle event tracking with typed payloads. - Defined event payloads and event names in separate files for better organization and type safety. - Updated all components and routes to use the new analytics methods for event tracking. - Added unit tests for the new analytics hook to ensure correct event capturing and error handling.
|
@spiderocious is attempting to deploy a commit to the Akinkunmi Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
1 issue found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/lib/analytics/use-analytics.ts">
<violation number="1" location="frontend/src/lib/analytics/use-analytics.ts:25">
P2: Unguarded PostHog client usage in centralized analytics facade — `posthog` may be undefined before initialization or if PostHog fails to load, causing every analytics call in the app to throw a runtime TypeError.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return useMemo(() => { | ||
| function emit<E extends EmptyEvents>(event: E): void | ||
| function emit<E extends keyof AnalyticsEvents>(event: E, props: AnalyticsEvents[E]): void | ||
| function emit<E extends keyof AnalyticsEvents>(event: E, props?: AnalyticsEvents[E]): void { |
There was a problem hiding this comment.
P2: Unguarded PostHog client usage in centralized analytics facade — posthog may be undefined before initialization or if PostHog fails to load, causing every analytics call in the app to throw a runtime TypeError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/analytics/use-analytics.ts, line 25:
<comment>Unguarded PostHog client usage in centralized analytics facade — `posthog` may be undefined before initialization or if PostHog fails to load, causing every analytics call in the app to throw a runtime TypeError.</comment>
<file context>
@@ -0,0 +1,107 @@
+ return useMemo(() => {
+ function emit<E extends EmptyEvents>(event: E): void
+ function emit<E extends keyof AnalyticsEvents>(event: E, props: AnalyticsEvents[E]): void
+ function emit<E extends keyof AnalyticsEvents>(event: E, props?: AnalyticsEvents[E]): void {
+ posthog.capture(event, props ?? undefined)
+ }
</file context>
There was a problem hiding this comment.
Thanks for flagging, I already looked into it(I thought about it ahead), but I don't think this one needs a change.
usePostHog() is typed to return a non-nullable PostHog (declare const usePostHog: () => PostHog in posthog-js 1.369.3), and the whole app renders inside <PostHogProvider> in __root.tsx, which supplies the client via context before any route renders. Since every useAnalytics() consumer is a descendant of that provider, posthog is always defined at the point emit runs — a posthog?.capture(...) guard would be defending against a state the type says can't occur (and TS wouldn't narrow it anyway).
Worth noting this isn't introduced by the facade: it's the same unguarded usePostHog().capture(...) that was already in all ~8 call sites before this refactor — I just moved it into one place. If anything, centralizing it means there'd be exactly one line to change if a guard ever became necessary.
PostHog is also fail-safe here by design: if the script is blocked or the token is bad, the client object still exists and .capture() no-ops rather than throwing, so the "fails to load → every call throws" path doesn't really happen.
The only scenario where it'd matter is calling useAnalytics() from a component mounted outside PostHogProvider — nothing in the app does that today, and if it did, that'd be a loud misuse bug rather than something a guard should silently absorb. Happy to add a one-line if (!posthog) return in emit if you'd prefer the defensiveness, but I'd lean against it given the types and provider setup.
What changed
Adds a small typed analytics layer in
src/lib/analytics/and routes everyanalytics call through it, replacing the ~42 raw
posthog.capture('name', {...})and ~14
posthog.captureException(...)calls that were scattered across 8 files.The new layer:
event-payloads.ts— one named payload type per event (thePayloadsnamespace),the single description of what each event carries.
events.ts— theAnalyticsEventsmap (event name → payload) thatemitis typedagainst, plus
ANALYTICS_EVENTS, a method-name → event-name constant guarded bysatisfiesso a typo or stale name fails to compile.use-analytics.ts— auseAnalytics()hook exposing one named method per event(e.g.
analytics.fileOpened({ file_id, method })) pluscaptureError(err, ctx?).Every event funnels through a single
emit()— the only place a provider is named.Every call site now uses the facade.
usePostHog()is no longer imported across theapp; it lives only inside the hook.
Why
wrong/missing property is now a build error instead of a silent mismatch in PostHog.
event server-side, or swapping PostHog out is a one-line change in
emit()rather thanedits across dozens of call sites.
analytics.autocompletes the full event catalog, and each event'spayload is a named, documented type.
No behavior change: the same events fire with the same names and payloads. PostHog remains
the underlying provider and the existing
PostHogProviderwiring is untouched.Notes
sites are now unified under one payload type per event (verified site by site; no fields
dropped).
package, this analytics layer could live there so the backend could emit the same
typed events too. With the current structure it's frontend-only, which is fine for now —
happy to revisit if a shared package ever appears.
Checklist
npm testpasses (addedsrc/__tests__/use-analytics.test.ts)npx tsc --noEmitintroduces no new type errorsbiome checkclean on all changed/added filesSummary by cubic
Introduced a typed analytics facade that replaces direct
posthog-js/reactcalls. Centralizes event names and payloads, adds compile-time safety, and keeps behavior unchanged.src/lib/analytics/withevent-payloads.ts,events.ts,use-analytics.ts, andindex.ts.useAnalytics()exposes one method per event andcaptureError, with all calls funneled through a singleemit().posthog.capture(...)and ~14 exception calls across the app; removedusePostHogimports fromposthog-js/react.AnalyticsEventsand guardedANALYTICS_EVENTSto catch name/payload mistakes at build time.Written for commit 3bdf9b9. Summary will update on new commits.