feat(posthog): update PostHog integration with environment variables and consent handling#75
Conversation
…and consent handling
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds PostHog analytics configuration and build args, updates Nuxt/PostHog configuration and route proxying, extends client consent composable to emit a manual pageview capture after consent, adjusts a client plugin sanitization set, and switches server-side PostHog config to environment variables. Changes
Sequence Diagram(s)mermaid User->>NuxtApp: grant analytics consent Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🚅 Deployed to the reqcore-pr-75 environment in applirank
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/composables/useAnalyticsConsent.ts (1)
18-19: Consider extracting the PostHog type for readability.The inline type cast is verbose. Extracting it improves readability:
♻️ Suggested refactor
+type PostHogClient = { + opt_in_capturing: () => void + opt_out_capturing: () => void + capture: (event: string, properties?: Record<string, unknown>) => void +} + export function useAnalyticsConsent() { - const posthog = (useNuxtApp() as Record<string, unknown>).$posthog as ((() => { opt_in_capturing: () => void, opt_out_capturing: () => void, capture: (event: string, properties?: Record<string, unknown>) => void }) | undefined) + const posthog = (useNuxtApp() as Record<string, unknown>).$posthog as (() => PostHogClient) | undefined const ph = posthog?.()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/composables/useAnalyticsConsent.ts` around lines 18 - 19, Extract the inline PostHog type into a named type or interface (e.g., PostHogClient) and use that alias instead of the verbose cast in useAnalyticsConsent.ts; replace the current cast on posthog with the new PostHogClient type and keep the posthog variable and ph assignment logic unchanged (referencing the posthog and ph identifiers) so the code is more readable and the shape of the PostHog API is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nuxt.config.ts`:
- Around line 206-208: The proxy entries in nuxt.config.ts mapping '/ingest/**'
and '/ingest/static/**' are hardcoded to EU endpoints (eu.i.posthog.com /
eu-assets.i.posthog.com) which will conflict when POSTHOG_HOST is set to a US
host; update the proxy configuration to derive targets from the POSTHOG_HOST
environment variable (or add a new POSTHOG_PROXY_HOST config) and use that value
for the '/ingest/**' and '/ingest/static/**' proxy targets, falling back to the
existing EU endpoints if the env var is not set; alternatively add a comment in
nuxt.config.ts documenting the limitation and instructing users to set the new
env var if they use the US data center so the mapping for '/ingest/**' and
'/ingest/static/**' points to the correct host.
- Around line 124-125: server/utils/posthog.ts is reading removed runtimeConfig
keys (config.public.posthog?.publicKey and config.public.posthog?.host) causing
server init to fail; fix by either restoring those keys into
runtimeConfig.public in nuxt.config.ts (ensure runtimeConfig.public.posthog = {
publicKey: process.env.POSTHOG_PUBLIC_KEY, host: process.env.POSTHOG_HOST } so
server code can read them) or, preferably, change server/utils/posthog.ts to
read directly from environment variables (use process.env.POSTHOG_PUBLIC_KEY and
process.env.POSTHOG_HOST, with optional fallback to config.public.posthog) when
initializing PostHog so server-side initialization does not rely on the
client-only `@posthog/nuxt` module.
---
Nitpick comments:
In `@app/composables/useAnalyticsConsent.ts`:
- Around line 18-19: Extract the inline PostHog type into a named type or
interface (e.g., PostHogClient) and use that alias instead of the verbose cast
in useAnalyticsConsent.ts; replace the current cast on posthog with the new
PostHogClient type and keep the posthog variable and ph assignment logic
unchanged (referencing the posthog and ph identifiers) so the code is more
readable and the shape of the PostHog API is centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d91b1cb-586e-4ae8-b17c-2a85cb183b74
📒 Files selected for processing (5)
.env.exampleDockerfileapp/composables/useAnalyticsConsent.tsapp/plugins/posthog-identity.client.tsnuxt.config.ts
💤 Files with no reviewable changes (1)
- app/plugins/posthog-identity.client.ts
| // ── PostHog reverse proxy — bypasses ad blockers by routing through reqcore.com ── | ||
| '/ingest/static/**': { proxy: 'https://eu-assets.i.posthog.com/static/**' }, | ||
| '/ingest/**': { proxy: 'https://eu.i.posthog.com/**' }, |
There was a problem hiding this comment.
Hardcoded EU proxy endpoints may conflict with US data center configuration.
The proxy routes are hardcoded to EU endpoints (eu.i.posthog.com, eu-assets.i.posthog.com), but POSTHOG_HOST can be set to the US data center (us.i.posthog.com). If a user configures US hosting, the proxy will still route to EU servers.
Consider documenting this limitation or making the proxy targets configurable if US support is needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nuxt.config.ts` around lines 206 - 208, The proxy entries in nuxt.config.ts
mapping '/ingest/**' and '/ingest/static/**' are hardcoded to EU endpoints
(eu.i.posthog.com / eu-assets.i.posthog.com) which will conflict when
POSTHOG_HOST is set to a US host; update the proxy configuration to derive
targets from the POSTHOG_HOST environment variable (or add a new
POSTHOG_PROXY_HOST config) and use that value for the '/ingest/**' and
'/ingest/static/**' proxy targets, falling back to the existing EU endpoints if
the env var is not set; alternatively add a comment in nuxt.config.ts
documenting the limitation and instructing users to set the new env var if they
use the US data center so the mapping for '/ingest/**' and '/ingest/static/**'
points to the correct host.
The PR removed runtimeConfig.public.posthog from nuxt.config.ts since @posthog/nuxt manages it via posthogConfig. However server/utils/posthog.ts still referenced config.public.posthog?.publicKey/host which TypeScript no longer recognises (TS2339). Switch to process.env.POSTHOG_PUBLIC_KEY / POSTHOG_HOST directly — this is server-only code running in Nitro, which has full access to process.env, and avoids the redundant runtimeConfig round-trip.
… structure fix(posthog): update proxy targets for PostHog integration with environment variable notes
1 participant
Summary
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit
New Features
Bug Fixes / Improvements