Replace Mixpanel with PostHog tracking on registry website#269
Replace Mixpanel with PostHog tracking on registry website#269james00012 wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
📝 WalkthroughWalkthroughThis PR migrates from Mixpanel to PostHog analytics. It adds ChangesPostHog Analytics Migration
CI Infrastructure Update
Sequence Diagram(s)sequenceDiagram
participant Browser
participant MyApp
participant PostHog
participant Analytics
rect rgba(100, 149, 237, 0.5)
note over MyApp: App initialization (production only)
MyApp->>PostHog: posthog.init(key, { capture_pageview: false })
end
rect rgba(144, 238, 144, 0.5)
note over Browser,PostHog: Route change
Browser->>MyApp: routeChangeComplete event
MyApp->>PostHog: posthog.capture('$pageview')
end
rect rgba(255, 165, 0, 0.5)
note over Analytics,PostHog: User event tracking
Analytics->>PostHog: posthog.capture(event, props)
Analytics->>PostHog: posthog.identify(userId)
Analytics->>PostHog: posthog.setPersonProperties(props)
end
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
There was a problem hiding this comment.
Pull request overview
Adds PostHog instrumentation to the registry website so activity on registry.comfy.org is included in cross-surface analytics, while keeping existing Mixpanel/GA4 call sites unchanged via the existing analytic singleton.
Changes:
- Add
posthog-jsdependency and initialize PostHog inpages/_app.tsx, including manual$pageviewcapture on client-side route changes. - Dual-send analytics events through
src/analytic/analytic.ts(Mixpanel + PostHog) behind the existingNEXT_PUBLIC_ENV=productiongate. - Update lockfile to include PostHog and its transitive dependencies.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/analytic/analytic.ts | Dual-sends track/identify/setProfile calls to PostHog in addition to Mixpanel. |
| pages/_app.tsx | Initializes PostHog and captures $pageview events on route changes. |
| package.json | Adds posthog-js dependency. |
| bun.lock | Locks PostHog-related package versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@pages/_app.tsx`:
- Around line 151-160: The useEffect hook for PostHog initialization uses the
variables `isProduction` and `posthogKey` in its body but has an empty
dependency array, violating React Hook rules. Update the dependency array at the
end of the useEffect to include both `isProduction` and `posthogKey` so the
effect properly re-runs when these values change.
- Around line 163-170: The useEffect hook that captures pageviews is missing
dependencies in its dependency array. The hook references variables isProduction
and posthogKey in its condition check, and also uses the posthog object to
capture events, but only includes router.events in the dependency array. Add
isProduction, posthogKey, and posthog to the dependency array to satisfy React
Hook lint rules and ensure the effect re-runs when these values change,
preventing stale closures and potential bugs.
- Around line 163-170: Add a check for router.isReady to the initial condition
guard in the useEffect hook to prevent duplicate pageview events during
hydration. Update the dependency array from [router.events] to [router.isReady,
posthogKey] to ensure the effect re-runs when the router is ready or when the
PostHog key changes, rather than re-registering listeners based on route event
references. This ensures the initial capturePageview() call and router event
listener are only set up after hydration completes.
- Around line 139-143: Remove the hardcoded PostHog API key from the fallback
value in the posthogKey variable assignment. Instead of using the nullish
coalescing operator with a hardcoded key as the default, require the
NEXT_PUBLIC_POSTHOG_KEY environment variable to be explicitly set by assigning
the environment variable directly or throwing an error if it is not defined,
ensuring configuration is managed through environment variables rather than
hardcoded values in the source code.
In `@src/analytic/analytic.ts`:
- Around line 39-46: The identify and track methods lack error handling around
PostHog calls, which means if PostHog fails (due to ad blockers, CSP violations,
or initialization failures), it could break the entire analytics pipeline. Wrap
the posthog.identify() call in the identify method and the corresponding PostHog
call in the track method with try-catch blocks to catch any errors that PostHog
might throw. Log the error using console.error or a logger to maintain
visibility, but allow Mixpanel and the rest of the application to continue
functioning normally even if PostHog fails.
- Around line 28-37: The track() method (and identify() and setProfile()
methods) call PostHog without safely handling the case where it hasn't finished
initializing yet, relying on the unreliable private _isInitialized property.
Replace any checks using posthog._isInitialized with optional chaining when
invoking PostHog methods (e.g., mixpanel.track and posthog.capture in the track
method, and corresponding calls in identify() and setProfile()). This safely
handles uninitialized PostHog state without depending on private internal APIs
that may change across SDK versions.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 421202ec-9ff6-4581-8c07-44ff3d8f7470
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
package.jsonpages/_app.tsxsrc/analytic/analytic.ts
james00012
force-pushed
the
james/mar-409-track-registry-website-activity-with-posthog
branch
from
115141a to
9a87ff1
Compare
james00012
changed the title
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Send registry.comfy.org analytics to PostHog instead of Mixpanel so it is measured in the same project as the other product surfaces. Reads the project key from NEXT_PUBLIC_POSTHOG_KEY (set per environment, like the Mixpanel key was), initializes posthog-js in _app, captures $pageview on route changes, and routes track/identify/setProfile through the analytic singleton. GA4 is unchanged. Gated on NEXT_PUBLIC_ENV=production.
james00012
force-pushed
the
james/mar-409-track-registry-website-activity-with-posthog
branch
from
9a87ff1 to
adb27fe
Compare
- Skip analytics in non-browser contexts; the analytic singleton is imported by server-side route handlers (GitHub OAuth) that call track(). - Move posthog.init to module scope (browser-guarded) so it runs before any child-component effect, avoiding pre-init capture calls.
benceruleanlu
left a comment
benceruleanlu
left a comment
There was a problem hiding this comment.
Make sure to set the env var!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/react-ci.yml:
- Line 19: The Node.js version in the CI workflow is set to 22, but the
`@types/node` dependency in package.json is pinned to version ^20.19.13, creating
a mismatch between the runtime and type definitions. Update the `@types/node`
entry in package.json to use version ^22.x to align the TypeScript type checking
with the Node.js 22 runtime being used in CI and ensure consistency between
development types and actual runtime APIs.
In `@src/analytic/analytic.ts`:
- Around line 19-40: The track, identify, and setProfile methods all include an
early return guard that checks typeof window === "undefined", which prevents
server-side API routes from sending analytics events to PostHog. Remove or
modify this guard in all three methods so that server-side calls from routes
like app/api/auth/github/route.ts can successfully send analytics data to
PostHog instead of silently dropping them. Consider whether the window check is
actually necessary for PostHog operations or if it can be removed entirely to
allow both server and client-side analytics to flow through.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 046aef2a-ac55-4335-994e-f8bdf79976bb
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/react-ci.ymlpackage.jsonpages/_app.tsxsrc/analytic/analytic.ts
| uses: actions/setup-node@v2 | ||
| with: | ||
| node-version: "20" | ||
| node-version: "22" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Node.js 22 compatibility with pinned type definitions and dependencies.
# Test 1: Check `@types/node` version constraint and Node 22 compatibility
echo "=== Checking `@types/node` version ==="
rg -A2 '"`@types/node`"' package.json
# Test 2: Search for explicit Node version engines constraint
echo "=== Checking engines field in package.json ==="
rg '"engines"' package.json -A3
# Test 3: Check if there are local type definitions that assume Node 20 APIs
echo "=== Searching for Node-specific type references ==="
rg -i 'node\.versions|process\.versions|--node' --type=ts --type=tsx -A2 -B2 src/ || echo "No explicit version checks found"
# Test 4: Look for any Node 22 related comments or configuration
echo "=== Searching for Node version comments ==="
rg -i 'node.*2[0-2]|node.*version' . -g '!node_modules' -g '!.git' || echo "No Node version notes found"Repository: Comfy-Org/registry-web
Length of output: 50425
Update @types/node to align with Node.js 22 runtime.
The CI now runs Node.js 22, but the project pins @types/node to ^20.19.13. While the gap won't turn your types upside-down (your dev dependencies like Vite and Vitest already support Node 22 types), this creates a version mismatch: TypeScript type checking validates against Node 20 APIs while code executes on Node 22. This deviation could quietly slip Node 22-specific APIs past type checking if they're ever used.
Recommended fix:
Update @types/node to ^22.x in package.json to keep your types and runtime in sync—no version-related bugs slipping through!
🤖 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 @.github/workflows/react-ci.yml at line 19, The Node.js version in the CI
workflow is set to 22, but the `@types/node` dependency in package.json is pinned
to version ^20.19.13, creating a mismatch between the runtime and type
definitions. Update the `@types/node` entry in package.json to use version ^22.x
to align the TypeScript type checking with the Node.js 22 runtime being used in
CI and ensure consistency between development types and actual runtime APIs.
| if (typeof window === "undefined") return; | ||
| if (this.isProduction) { | ||
| mixpanel.track(event, properties); | ||
| posthog.capture(event, properties); | ||
| } else { | ||
| console.log(`Mixpanel Track - Event: ${event}, Properties: ${JSON.stringify(properties)}`); | ||
| console.log(`Track - Event: ${event}, Properties: ${JSON.stringify(properties)}`); | ||
| } | ||
| } | ||
|
|
||
| public identify(distinctId: string): void { | ||
| if (typeof window === "undefined") return; | ||
| if (this.isProduction) { | ||
| mixpanel.identify(distinctId); | ||
| posthog.identify(distinctId); | ||
| } else { | ||
| console.log(`Mixpanel Identify - Distinct ID: ${distinctId}`); | ||
| console.log(`Identify - Distinct ID: ${distinctId}`); | ||
| } | ||
| } | ||
|
|
||
| public setProfile(updates: UserProfile): void { | ||
| if (typeof window === "undefined") return; | ||
| if (this.isProduction) { | ||
| mixpanel.people.set(updates); | ||
| posthog.setPersonProperties(updates); | ||
| } else { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify server-side call sites that now no-op due `typeof window` guards.
# Expected: any matches under app/api, pages/api, or server route handlers indicate dropped events.
rg -n -C2 'analytic\.(track|identify|setProfile)\(' --type ts --type tsx
rg -n -C2 'import analytic from ["'\''].*analytic["'\'']' --type ts --type tsx
rg -n -C2 'export (async )?function (GET|POST|PUT|PATCH|DELETE)|route\.ts' --type ts --type tsxRepository: Comfy-Org/registry-web
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Search for analytic method calls (track, identify, setProfile) in the codebase
# without the problematic tsx type filter
echo "=== Searching for analytic method calls ==="
rg -n -C2 'analytic\.(track|identify|setProfile)\(' --type ts
echo ""
echo "=== Searching for analytic imports ==="
rg -n -C2 'import.*analytic' --type ts
echo ""
echo "=== Checking for the OAuth callback route file ==="
find . -name "*github*callback*" -o -name "*route.ts" | head -20
echo ""
echo "=== Inspecting analytic.ts structure ==="
cat -n src/analytic/analytic.ts | head -50Repository: Comfy-Org/registry-web
Length of output: 14992
Server-side analytics are silently dropped by the window guard—that's no jest, events fail to manifest.
Lines 19, 28, and 37 return early on server, but app/api/auth/github/route.ts (line 43) and app/api/auth/github/callback/route.ts (lines 75, 87) call analytic.track() from server-side API routes. This means OAuth verification events never reach PostHog, silently dropping critical auth fidelity data.
Suggested fix direction
import posthog from "posthog-js";
+// server transport should be added (e.g., posthog-node) for non-browser callers
class Analytics {
public track(event: string, properties?: object): void {
- if (typeof window === "undefined") return;
+ if (typeof window === "undefined") {
+ // send via server analytics client here
+ return;
+ }
if (this.isProduction) {
posthog.capture(event, properties);
}
}
}🤖 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 `@src/analytic/analytic.ts` around lines 19 - 40, The track, identify, and
setProfile methods all include an early return guard that checks typeof window
=== "undefined", which prevents server-side API routes from sending analytics
events to PostHog. Remove or modify this guard in all three methods so that
server-side calls from routes like app/api/auth/github/route.ts can successfully
send analytics data to PostHog instead of silently dropping them. Consider
whether the window check is actually necessary for PostHog operations or if it
can be removed entirely to allow both server and client-side analytics to flow
through.
What
Switches the registry website (
registry.comfy.org) from Mixpanel to PostHog, so its activity is measured in the same prod project as the other product surfaces. The registry was the one product surface not in PostHog, leaving it invisible in cross-surface active-user metrics.How
posthog-jsin_app.tsxusing the same prod project config every other comfy web surface uses (key fallbackphc_iKfK…,api_hostt.comfy.org,person_profiles: 'identified_only'), with manual$pageviewcapture on client-side route changes.analyticsingleton through PostHog (track→capture,identify→identify,setProfile→setPersonProperties), so all current call sites keep working unchanged.mixpanel-browserand@types/mixpanel-browser. GA4 is unchanged.NEXT_PUBLIC_ENV=production, matching the prior Mixpanel gate.Anonymous (logged-out) visitors are still captured:
identified_onlyonly suppresses person-profile creation, not$pageviewevents, which is what the active-users mart needs for anonymous DAU.Follow-up
A separate data-platform change is needed to map
$host = registry.comfy.orgto aregistrysurface inint_events_unified; until then these events report asotherinmart_active_users.Linear: MAR-409