Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds PostHog documentation, an example Expo project, Expo config and PostHog client initialization, roots-level PostHog provider/screen tracking, event instrumentation on onboarding/language/home screens, auth flow instrumentation and identify usage, and a generated setup report. ChangesPostHog Analytics Integration
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
app/LanguageSelection.tsx (1)
136-139: 💤 Low valueInconsistent property naming.
The property uses
languagefor the ID value. For clarity and consistency with your data model, consider usinglanguageIdinstead.- posthog.capture("language_selected", { language: language.id }); + posthog.capture("language_selected", { languageId: language.id });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/LanguageSelection.tsx` around lines 136 - 139, The posthog event in LanguageSelection's onPress handler uses an inconsistent property name ("language") when sending the selected language ID; change the event payload to use "languageId" instead, i.e., update the posthog.capture call in the onPress inside LanguageSelection so it sends { languageId: language.id } while leaving setSelectedLanguageId(language.id) unchanged.app/(tabs)/home.tsx (1)
136-141: 💤 Low valueConsider consistent property naming.
The property uses
languagefor the language ID value. For clarity and consistency across your analytics events, consider usinglanguageId.onPress={() => posthog.capture("continue_learning_tapped", { - language: selectedLanguage.id, + languageId: selectedLanguage.id, unit: currentUnit?.order ?? 1, }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(tabs)/home.tsx around lines 136 - 141, Update the analytics payload in the onPress handler where posthog.capture("continue_learning_tapped", ...) is called: replace the property key language with languageId so the event sends languageId: selectedLanguage.id (keep currentUnit?.order as-is or rename unit if needed elsewhere for consistency). Locate the posthog.capture call inside the onPress callback and change the key only—do not alter the values or event name.lib/posthog.ts (2)
4-6: ⚡ Quick winConsider runtime type validation.
The type assertions assume
Constants.expoConfig?.extravalues are strings, but runtime validation would be safer.🛡️ Suggested defensive type check
-const apiKey = Constants.expoConfig?.extra?.posthogProjectToken as string | undefined; -const host = Constants.expoConfig?.extra?.posthogHost as string | undefined; +const apiKey = typeof Constants.expoConfig?.extra?.posthogProjectToken === 'string' + ? Constants.expoConfig.extra.posthogProjectToken + : undefined; +const host = typeof Constants.expoConfig?.extra?.posthogHost === 'string' + ? Constants.expoConfig.extra.posthogHost + : undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/posthog.ts` around lines 4 - 6, The code assumes Constants.expoConfig?.extra values are strings; add runtime type checks to validate apiKey and host before casting: verify typeof apiKeyRaw === "string" and apiKeyRaw.trim() !== "" (same for hostRaw) and only then assign to apiKey/host variables (or undefined otherwise), and update isPostHogConfigured to use the validated apiKey (and reject placeholder "phc_your_project_token_here"); reference the existing symbols apiKey, host, isPostHogConfigured and Constants.expoConfig?.extra when implementing the checks.
8-8: 💤 Low valueConsider removing the placeholder fallback.
Since
disabled: !isPostHogConfiguredprevents PostHog from operating when not configured, the"placeholder_key"fallback may be unnecessary. However, if the PostHog constructor requires a non-empty API key, this is acceptable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/posthog.ts` at line 8, The PostHog client is being constructed with a hardcoded "placeholder_key" fallback which is unnecessary when disabled is controlled by isPostHogConfigured; update the instantiation so it does not fall back to "placeholder_key" — either pass apiKey directly into the PostHog constructor (remove the "placeholder_key" fallback in the export const posthog = new PostHog(...) line) or gate creation behind a check using isPostHogConfigured so the PostHog constructor never receives a fake key; keep references to the PostHog constructor and the disabled: !isPostHogConfigured option consistent.app/_layout.tsx (1)
45-53: 💤 Low valueConsider error handling for
posthog.flush().Using
voidsuppresses the promise fromposthog.flush(), which means any errors during the flush will be silently ignored. If PostHog flush failures should be logged or handled, consider adding error handling.
⚠️ Optional error handlingdidCaptureAppOpened.current = true; posthog.capture("app_opened"); - void posthog.flush(); + posthog.flush().catch((error) => { + console.error("PostHog flush failed:", error); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/_layout.tsx` around lines 45 - 53, The current useEffect marks didCaptureAppOpened and calls posthog.capture("app_opened") then uses void posthog.flush(), which suppresses promise errors; update the block around useEffect/ didCaptureAppOpened so posthog.flush() is awaited or handled and any rejection is caught and logged (e.g., call posthog.flush().catch(err => /* log via console.error or a logger */)) to ensure flush failures are surfaced when posthog.flush() is invoked after posthog.capture("app_opened").
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/integration-expo/SKILL.md:
- Around line 42-50: Remove the duplicated "posthog-react-native" line and the
conflicting bare React Native instructions that reference react-native-config
and react-native-svg from the Expo skill doc (these are the lines duplicating
the package name and the lines suggesting react-native-config/react-native-svg);
keep the Expo-specific guidance that advises using expo-constants and accessing
Constants.expoConfig?.extra?.posthogProjectToken and the Expo-specific placement
of PostHogProvider (inside app/_layout.tsx) — alternatively, if bare RN guidance
must be preserved, move the react-native-config/react-native-svg and
NavigationContainer notes into a clearly labeled "Bare React Native" section so
Expo vs Bare RN recommendations are not mixed.
In `@app/_layout.tsx`:
- Around line 35-43: The analytics call currently spreads all URL search params
(params) into posthog.screen inside the useEffect (watching pathname and params)
which may leak PII; change the code that builds the event payload (the object
passed to posthog.screen) to sanitize or remove search params — e.g., compute a
safeParams object before calling posthog.screen by either allow-listing explicit
param keys you expect for analytics or filtering out sensitive keys/tokens
(match keys like "token", "auth", "email", "id", etc.), then pass
previous_screen and ...safeParams instead of ...params (references: useEffect,
previousPathname.current, posthog.screen, params, pathname).
In `@app/LanguageSelection.tsx`:
- Line 68: Make the language-confirm handler async, change the captured property
to languageId: selectedLanguage.id, and ensure the PostHog event is flushed
before navigation: call posthog.capture("language_confirmed", { languageId:
selectedLanguage.id }) then await posthog.flush() (or the SDK's equivalent)
before calling router.replace(...). Update the handler function signature (e.g.,
handleConfirmLanguage or onConfirm) to async so you can await the flush and
prevent the race when navigating away.
In `@app/onboarding.tsx`:
- Around line 74-77: The Pressable onPress currently calls
posthog.capture("onboarding_get_started_tapped") without waiting, which can race
with immediate navigation; change the handler onPress to an async function that
awaits posthog.capture(...) (or calls and awaits posthog.flush() if available)
before triggering navigation/Link behavior so the event is reliably sent; update
the handler where posthog.capture is invoked (the Pressable onPress) to perform
the await and then proceed with the Link/navigation action.
In `@components/auth-screen.tsx`:
- Around line 216-223: The code is sending raw PII (signUp.emailAddress) to
PostHog via posthog.identify; change this to avoid logging raw emails by either
(a) only calling posthog.identify(userId) without any email properties, (b) only
sending an email-derived identifier when the user has explicitly consented
(e.g., check a consent flag on signUp such as signUp.consentedToAnalytics before
including email), or (c) pseudonymize the email client-side (e.g., replace the
email property with a deterministic one-way hash) and then call
posthog.identify(userId, { $set: { email_hash: <hashed-value> } }). Also ensure
posthog.capture("sign_up_completed") remains but only after consent check if
analytics require consent.
- Around line 248-254: The code uses signIn.createdSessionId for PostHog
identity which is ephemeral; change posthog.identify to use a stable user
identifier from the Clerk signIn result (e.g., signIn.user?.id or the Clerk
API's persistent user id) and call posthog.identify(userId) only when that
stable id exists; also remove or avoid sending raw PII (do not set email
directly in posthog.identify — either omit it or send a non-PII hashed value)
when updating user properties via posthog.identify/posthog.capture.
- Around line 170-179: Replace the manual posthog.capture("$exception", {...})
call in auth-screen with the SDK helper: call posthog.captureException(error, {
auth_mode: mode }) so the SDK can process stack traces and metadata; locate the
block that currently builds the $exception_list (using posthog.capture and
getAuthErrorMessage) and replace it with posthog.captureException(error, {
auth_mode: mode }) while removing the custom $exception payload.
In `@posthog-setup-report.md`:
- Around line 6-7: Add a short environment-variable setup section explaining
which variables to set and where to get them: document that POSTHOG_API_KEY and
POSTHOG_HOST must be exported (or placed in .env) from the PostHog project
settings/dashboard (Project > Setup > API key / Host), show that app.config.js
should read these from process.env into its extra config so lib/posthog.ts can
access them via Constants.expoConfig?.extra, and instruct developers to restart
the dev server after changing env vars; also mention optional guidance for
secure storage (e.g., .env, CI secrets) and a sample variable names list to copy
into the repository docs.
- Around line 31-36: Update the markdown links that currently use relative paths
(e.g., the list items "Analytics basics dashboard", "Sign-up Funnel", "Sign-in
Funnel", "Language Selections Over Time", "Social Auth Conversion", "Continue
Learning Engagement") to absolute PostHog URLs by replacing paths like
`/dashboard/1601544` or `/insights/x4do4BVS` with full URLs such as
`https://app.posthog.com/project/YOUR_PROJECT_ID/dashboard/1601544` (or the
corresponding insights URL format) and include a clear placeholder
(YOUR_PROJECT_ID) so users can fill in their instance/project ID.
- Around line 14-26: Add documentation rows for the lifecycle event app_opened
and any manual screen-tracking $screen events to the event table; specifically
note that app_opened is emitted from app/_layout.tsx (lifecycle) and that
$screen events come from the manual screen tracking implemented around
usePathname (e.g., the hook or component where usePathname is used). For each
new row include the event name, a short description (when/what triggers it) and
the source file/component that emits it so the table reflects both autocaptured
lifecycle events and manual screen events.
---
Nitpick comments:
In `@app/_layout.tsx`:
- Around line 45-53: The current useEffect marks didCaptureAppOpened and calls
posthog.capture("app_opened") then uses void posthog.flush(), which suppresses
promise errors; update the block around useEffect/ didCaptureAppOpened so
posthog.flush() is awaited or handled and any rejection is caught and logged
(e.g., call posthog.flush().catch(err => /* log via console.error or a logger
*/)) to ensure flush failures are surfaced when posthog.flush() is invoked after
posthog.capture("app_opened").
In `@app/`(tabs)/home.tsx:
- Around line 136-141: Update the analytics payload in the onPress handler where
posthog.capture("continue_learning_tapped", ...) is called: replace the property
key language with languageId so the event sends languageId: selectedLanguage.id
(keep currentUnit?.order as-is or rename unit if needed elsewhere for
consistency). Locate the posthog.capture call inside the onPress callback and
change the key only—do not alter the values or event name.
In `@app/LanguageSelection.tsx`:
- Around line 136-139: The posthog event in LanguageSelection's onPress handler
uses an inconsistent property name ("language") when sending the selected
language ID; change the event payload to use "languageId" instead, i.e., update
the posthog.capture call in the onPress inside LanguageSelection so it sends {
languageId: language.id } while leaving setSelectedLanguageId(language.id)
unchanged.
In `@lib/posthog.ts`:
- Around line 4-6: The code assumes Constants.expoConfig?.extra values are
strings; add runtime type checks to validate apiKey and host before casting:
verify typeof apiKeyRaw === "string" and apiKeyRaw.trim() !== "" (same for
hostRaw) and only then assign to apiKey/host variables (or undefined otherwise),
and update isPostHogConfigured to use the validated apiKey (and reject
placeholder "phc_your_project_token_here"); reference the existing symbols
apiKey, host, isPostHogConfigured and Constants.expoConfig?.extra when
implementing the checks.
- Line 8: The PostHog client is being constructed with a hardcoded
"placeholder_key" fallback which is unnecessary when disabled is controlled by
isPostHogConfigured; update the instantiation so it does not fall back to
"placeholder_key" — either pass apiKey directly into the PostHog constructor
(remove the "placeholder_key" fallback in the export const posthog = new
PostHog(...) line) or gate creation behind a check using isPostHogConfigured so
the PostHog constructor never receives a fake key; keep references to the
PostHog constructor and the disabled: !isPostHogConfigured option consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9776e11-4daa-4b1a-9c7a-adeefa78f56a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
.claude/skills/integration-expo/.posthog-wizard.claude/skills/integration-expo/SKILL.md.claude/skills/integration-expo/references/EXAMPLE.md.claude/skills/integration-expo/references/basic-integration-1.0-begin.md.claude/skills/integration-expo/references/basic-integration-1.1-edit.md.claude/skills/integration-expo/references/basic-integration-1.2-revise.md.claude/skills/integration-expo/references/basic-integration-1.3-conclude.md.claude/skills/integration-expo/references/identify-users.md.claude/skills/integration-expo/references/react-native.mdapp.config.jsapp/(tabs)/home.tsxapp/LanguageSelection.tsxapp/_layout.tsxapp/onboarding.tsxcomponents/auth-screen.tsxlib/posthog.tspackage.jsonposthog-setup-report.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/auth-screen.tsx`:
- Around line 310-313: After successfully calling setActive({ session: sessionId
}) in the social auth path, obtain the stable user id via useAuth() and call
posthog.identify(userId) before navigating away so analytics are tied to the
user; specifically, after awaiting setActive in the block that has sessionId,
call useAuth() to get userId, invoke posthog.identify(userId), then proceed to
posthog.capture("social_auth_completed", { strategy, auth_mode: mode }) and
finally router.replace("/").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 78aa41c3-8ab1-4354-b546-04781512a4a5
📒 Files selected for processing (9)
.claude/skills/integration-expo/SKILL.mdapp.config.jsapp/(tabs)/home.tsxapp/LanguageSelection.tsxapp/_layout.tsxapp/onboarding.tsxcomponents/auth-screen.tsxlib/posthog.tsposthog-setup-report.md
✅ Files skipped from review due to trivial changes (1)
- posthog-setup-report.md
🚧 Files skipped from review as they are similar to previous changes (5)
- app/(tabs)/home.tsx
- app.config.js
- app/_layout.tsx
- lib/posthog.ts
- app/LanguageSelection.tsx
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 by CodeRabbit
New Features
Documentation
Chores