ENG-1094 allow users to opt out of posthog #625
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
maparent
changed the base branch from
main
to
eng-1135-replacing-sendmail-with-explicit-posthog-errors
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds a DISALLOW_TRACKING user setting, a settings UI checkbox to toggle it, centralizes PostHog init/enable/disable, and updates internal error handling and app bootstrap to respect the tracking preference. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsUI as Settings UI
participant PostHogUtils as PostHog Utils
participant PostHogJS as PostHog JS
participant Extension
rect rgba(240,248,255,0.9)
note over Extension,PostHogUtils: Startup initialization
Extension->>PostHogUtils: initPostHog()
alt DISALLOW_TRACKING not set
PostHogUtils->>PostHogJS: doInitPostHog() (once)
PostHogJS->>PostHogJS: configure (privacy, denylist, before_send)
PostHogJS->>PostHogJS: identify/register + "Extension Loaded" event
else DISALLOW_TRACKING set
PostHogUtils-->>Extension: skip initialization
end
end
rect rgba(245,255,245,0.9)
note over User,SettingsUI: User toggles preference
User->>SettingsUI: Toggle "Disable tracking"
SettingsUI->>SettingsUI: setSetting(DISALLOW_TRACKING, value)
alt value == true
SettingsUI->>PostHogUtils: disablePostHog()
PostHogUtils->>PostHogJS: opt_out_capturing()
else value == false
SettingsUI->>PostHogUtils: enablePostHog()
PostHogUtils->>PostHogJS: ensure init + opt_in_capturing()
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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 |
|
@CodeRabbit new commit added for review. |
|
@maparent: I'll review the new commit that was added to the PR. 🧠 Learnings used✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
268-290: Consider improving error handling for setting persistence.The checkbox implementation correctly toggles PostHog state, but there's a potential edge case: if
setSetting()fails (line 273), the in-memory PostHog state will be out of sync with the persisted setting. On the next page load, the setting will revert to the old value.While the current
.catch(() => undefined)approach is consistent with other checkboxes in this file (lines 223, 241), consider whether tracking preference warrants more robust error handling given its privacy implications.Optional: Log setting persistence failures
- void setSetting(DISALLOW_TRACKING, disallow).catch(() => undefined); + void setSetting(DISALLOW_TRACKING, disallow).catch((error) => { + console.error("Failed to persist tracking preference:", error); + });apps/roam/src/utils/posthog.ts (1)
49-63: Consider adding explicit return types.The exported functions work correctly but lack explicit return types. Per the coding guidelines for TypeScript, functions should have explicit return types.
Add explicit return types
-export const enablePostHog = () => { +export const enablePostHog = (): void => { doInitPostHog(); posthog.opt_in_capturing(); }; -export const disablePostHog = () => { +export const disablePostHog = (): void => { if (initialized) posthog.opt_out_capturing(); }; -export const initPostHog = () => { +export const initPostHog = (): void => { const disabled = getSetting(DISALLOW_TRACKING, false); if (!disabled) { doInitPostHog(); } };Based on coding guidelines: "Use explicit return types for functions"
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/roam/src/components/settings/HomePersonalSettings.tsx(2 hunks)apps/roam/src/data/userSettings.ts(1 hunks)apps/roam/src/index.ts(1 hunks)apps/roam/src/utils/internalError.ts(2 hunks)apps/roam/src/utils/posthog.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/utils/internalError.tsapps/roam/src/index.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/utils/posthog.ts
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/utils/internalError.tsapps/roam/src/index.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/utils/posthog.ts
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/utils/internalError.tsapps/roam/src/index.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/utils/posthog.ts
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/utils/internalError.tsapps/roam/src/index.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/utils/posthog.ts
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/utils/internalError.tsapps/roam/src/index.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/utils/posthog.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-30T17:03:26.927Z
Learning: For maparent: Takes a focused approach to PR scope - acknowledges good suggestions that are out of scope for current PR rather than expanding scope.
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Applied to files:
apps/roam/src/data/userSettings.tsapps/roam/src/utils/internalError.tsapps/roam/src/index.ts
📚 Learning: 2025-12-13T00:42:41.524Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-12-13T00:42:41.524Z
Learning: The `internalError` utility in apps/roam/src/utils/internalError.ts handles string-to-error conversion internally. Callers can pass strings directly as the error parameter without needing to convert them to Error objects first.
Applied to files:
apps/roam/src/utils/internalError.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Applied to files:
apps/roam/src/utils/internalError.tsapps/roam/src/index.tsapps/roam/src/utils/posthog.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{js,ts,tsx,jsx,json} : Prefer existing dependencies from package.json when working on the Roam Research extension
Applied to files:
apps/roam/src/utils/internalError.tsapps/roam/src/index.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/** : Implement the Discourse Graph protocol in the Roam Research extension
Applied to files:
apps/roam/src/index.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,jsx,js,css,scss} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Applied to files:
apps/roam/src/index.ts
📚 Learning: 2025-08-26T01:52:35.443Z
Learnt from: mdroidian
Repo: DiscourseGraphs/discourse-graph PR: 369
File: apps/roam/src/utils/pluginTimer.ts:14-21
Timestamp: 2025-08-26T01:52:35.443Z
Learning: In the discourse-graph codebase, initPluginTimer() is only called once during the runExtension flow in apps/roam/src/index.ts, so re-initialization scenarios are not a concern for this specific usage pattern.
Applied to files:
apps/roam/src/index.ts
🧬 Code graph analysis (3)
apps/roam/src/utils/internalError.ts (2)
apps/roam/src/utils/extensionSettings.ts (1)
getSetting(3-11)apps/roam/src/data/userSettings.ts (1)
DISALLOW_TRACKING(11-11)
apps/roam/src/components/settings/HomePersonalSettings.tsx (3)
apps/roam/src/utils/extensionSettings.ts (2)
getSetting(3-11)setSetting(13-16)apps/roam/src/data/userSettings.ts (1)
DISALLOW_TRACKING(11-11)apps/roam/src/utils/posthog.ts (2)
disablePostHog(54-56)enablePostHog(49-52)
apps/roam/src/utils/posthog.ts (3)
apps/roam/src/utils/getVersion.ts (1)
getVersionWithDate(9-24)apps/roam/src/utils/extensionSettings.ts (1)
getSetting(3-11)apps/roam/src/data/userSettings.ts (1)
DISALLOW_TRACKING(11-11)
🔇 Additional comments (7)
apps/roam/src/data/userSettings.ts (1)
11-11: LGTM! Clean addition to settings constants.The new
DISALLOW_TRACKINGconstant follows the existing naming pattern and provides a clear, centralized key for the tracking preference.apps/roam/src/utils/internalError.ts (2)
5-6: Good addition of tracking preference check.The imports are correctly added to support the new tracking control functionality.
25-28: Correctly respects user tracking preference.The logic appropriately checks
DISALLOW_TRACKINGbefore sending telemetry, while still logging errors locally for debugging. The user-facing toast (lines 66-73) is correctly shown regardless of tracking preference, ensuring users still see error messages.apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
23-25: Clean imports for tracking control.The new imports integrate well with the existing settings infrastructure.
apps/roam/src/index.ts (2)
39-40: Good refactoring to centralized PostHog initialization.The imports correctly reference the new centralized PostHog module and user settings.
47-50: Correctly guards PostHog initialization based on all relevant conditions.The logic appropriately checks graph encryption, offline status, and user tracking preference before initializing PostHog. This ensures privacy for sensitive graphs and respects user choice.
apps/roam/src/utils/posthog.ts (1)
1-63: Well-designed centralized PostHog integration.The module provides a clean API for tracking control with appropriate guards and privacy settings. The initialization is properly deferred through the
loadedcallback, and the opt-in/opt-out mechanism correctly respects the user's preference.Minor note: There's a theoretical race condition where multiple rapid calls to
doInitPostHog()before theloadedcallback fires could result in multipleposthog.init()calls. However, PostHog's library should handle this gracefully asinit()is typically idempotent.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/roam/src/utils/posthog.ts (1)
10-61: The property denylist andbefore_sendfilter are insufficient to prevent IP capture.As noted in a previous review, PostHog can capture IP addresses from the HTTP request headers even when
$ipis filtered client-side. Thebefore_sendfilter added here only prevents explicitly set$ipproperties from being sent, but doesn't prevent server-side IP capture from request metadata.To properly prevent IP storage, enable the "Discard client IP data" server setting in your PostHog project configuration.
🧹 Nitpick comments (1)
apps/roam/src/utils/posthog.ts (1)
33-42: Consider using a Set for more efficient property filtering.The current implementation uses
indexOffor each property check, resulting in O(n) lookups. Convertingproperty_denylistto a Set would provide O(1) lookup performance.🔎 View suggested optimization:
/* eslint-disable @typescript-eslint/naming-convention */ const property_denylist = [ "$ip", "$device_id", "$geoip_city_name", "$geoip_latitude", "$geoip_longitude", "$geoip_postal_code", "$geoip_subdivision_1_name", "$raw_user_agent", "$current_url", "$referrer", "$referring_domain", "$initial_current_url", "$pathname", ]; + const denylistSet = new Set(property_denylist); posthog.init("phc_SNMmBqwNfcEpNduQ41dBUjtGNEUEKAy6jTn63Fzsrax", { api_host: "https://us.i.posthog.com", person_profiles: "identified_only", capture_pageview: false, property_denylist, before_send: (result: CaptureResult | null) => { if (result !== null) { result.properties = Object.fromEntries( Object.entries(result.properties).filter( - ([k]) => property_denylist.indexOf(k) < 0, + ([k]) => !denylistSet.has(k), ), ); } return result; },
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/roam/src/components/settings/HomePersonalSettings.tsx(2 hunks)apps/roam/src/utils/posthog.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-30T17:03:26.927Z
Learning: For maparent: Takes a focused approach to PR scope - acknowledges good suggestions that are out of scope for current PR rather than expanding scope.
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Applied to files:
apps/roam/src/utils/posthog.ts
🧬 Code graph analysis (1)
apps/roam/src/components/settings/HomePersonalSettings.tsx (3)
apps/roam/src/utils/extensionSettings.ts (2)
getSetting(3-11)setSetting(13-16)apps/roam/src/data/userSettings.ts (1)
DISALLOW_TRACKING(11-11)apps/roam/src/utils/posthog.ts (2)
disablePostHog(68-70)enablePostHog(63-66)
|
@CodeRabbit Did set the discard ip address setting. |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2197 @maparent, excellent! Setting the IP address discarding in PostHog's backend configuration provides an additional layer of privacy protection. I can see that the code already includes This defense-in-depth approach—filtering both at the code level and in PostHog's backend—ensures robust privacy protection for users, which aligns perfectly with the opt-out functionality you're implementing in this PR. 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/roam/src/utils/posthog.ts (1)
12-42: IP privacy implementation improved withbefore_sendhook.The addition of the
before_sendhook (lines 33-42) that explicitly filters properties from the denylist addresses the alternative solution mentioned in the previous review. Combined with your confirmation that the server-side "Discard client IP data" setting is enabled, this provides defense-in-depth for IP privacy.Based on past review comments and your confirmation of the server-side setting.
apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
269-300: Promise chain now correctly ordered - past issue resolved!The promise chain has been fixed from the previous review. The
.then()callback now executes only on successfulsetSetting(), and the.catch()callback correctly reverts the UI state only on failure. PostHog state will now stay in sync with the persisted setting.Based on past review comments - the issue has been addressed.
🧹 Nitpick comments (1)
apps/roam/src/utils/posthog.ts (1)
45-59: Add error handling for Roam API calls in theloadedcallback.The
loadedcallback accessesgetCurrentUserUid()andwindow.roamAlphaAPI.graph.namewithout error handling. If the Roam API isn't fully ready or returns unexpected values, PostHog initialization could fail silently or throw errors.🔎 View suggested improvement:
loaded: (posthog) => { - const { version, buildDate } = getVersionWithDate(); - const userUid = getCurrentUserUid(); - const graphName = window.roamAlphaAPI.graph.name; - posthog.identify(userUid, { - graphName, - }); - posthog.register({ - version: version || "-", - buildDate: buildDate || "-", - graphName, - }); - posthog.capture("Extension Loaded"); + try { + const { version, buildDate } = getVersionWithDate(); + const userUid = getCurrentUserUid(); + const graphName = window.roamAlphaAPI?.graph?.name; + + if (userUid && graphName) { + posthog.identify(userUid, { graphName }); + posthog.register({ + version: version || "-", + buildDate: buildDate || "-", + graphName, + }); + } + posthog.capture("Extension Loaded"); + } catch (error) { + console.error("PostHog initialization error:", error); + } },
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/roam/src/components/settings/HomePersonalSettings.tsx(2 hunks)apps/roam/src/utils/posthog.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/utils/posthog.tsapps/roam/src/components/settings/HomePersonalSettings.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-30T17:03:26.927Z
Learning: For maparent: Takes a focused approach to PR scope - acknowledges good suggestions that are out of scope for current PR rather than expanding scope.
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Applied to files:
apps/roam/src/utils/posthog.ts
🧬 Code graph analysis (1)
apps/roam/src/utils/posthog.ts (3)
apps/roam/src/utils/getVersion.ts (1)
getVersionWithDate(9-24)apps/roam/src/utils/extensionSettings.ts (1)
getSetting(3-11)apps/roam/src/data/userSettings.ts (1)
DISALLOW_TRACKING(11-11)
🔇 Additional comments (1)
apps/roam/src/utils/posthog.ts (1)
72-77: LGTM! Clean integration with user settings.The
initPostHogfunction correctly respects theDISALLOW_TRACKINGsetting and provides a clean entry point for initialization.
| let initialized = false; | ||
|
|
||
| const doInitPostHog = (): void => { | ||
| if (initialized) return; |
There was a problem hiding this comment.
Fix race condition: set initialized flag before async initialization.
The initialized flag is checked at line 11 but only set to true inside the loaded callback (line 58). If doInitPostHog() is called multiple times in quick succession before the loaded callback fires, posthog.init() will be invoked multiple times, potentially causing duplicate initialization.
🔎 Apply this diff to fix the race condition:
const doInitPostHog = (): void => {
if (initialized) return;
+ initialized = true;
const propertyDenylist = new Set([Then remove the assignment from the loaded callback since it's no longer needed there:
posthog.capture("Extension Loaded");
- initialized = true;
},Also applies to: 58-58
🤖 Prompt for AI Agents
In apps/roam/src/utils/posthog.ts around lines 8-11 and 58, the boolean
initialized is only set inside the async loaded callback which allows multiple
concurrent calls to doInitPostHog to trigger posthog.init; move the assignment
so initialized = true is set immediately before starting async initialization
(i.e., before calling posthog.init or loading the script) to prevent re-entry,
and remove the redundant initialized assignment from the loaded callback.
https://linear.app/discourse-graphs/issue/ENG-1094/allow-users-to-opt-out-of-posthog
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.