refactor: clean up Outlit SDK integration#1374
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Outlit Analytics integration across the desktop, marketing, docs, and web applications. It introduces the NEXT_PUBLIC_OUTLIT_KEY environment variable across configuration files and deployment workflows, adds Outlit dependencies, creates Outlit client instances, wraps component trees with OutlitProvider, and integrates Outlit tracking calls alongside existing PostHog analytics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/desktop/src/main/env.main.ts`:
- Line 24: The env schema currently requires NEXT_PUBLIC_OUTLIT_KEY while
NEXT_PUBLIC_POSTHOG_KEY is optional; update the zod schema so
NEXT_PUBLIC_OUTLIT_KEY uses z.string().optional() to match
NEXT_PUBLIC_POSTHOG_KEY (symbol: NEXT_PUBLIC_OUTLIT_KEY in the env schema), or
if the asymmetry is intentional, add a comment clarifying why it must be
required and adjust CI/local env docs instead; ensure any dependent code that
assumes the key exists handles undefined when you make it optional.
In `@apps/desktop/src/main/index.ts`:
- Line 161: The call to outlit.shutdown() can hang or reject and block
subsequent cleanup (disposeTray() and app.exit(0)); wrap the shutdown in a
cancellable timeout race (e.g., Promise.race between outlit.shutdown() and a
timeout promise) and await it with a try/catch so rejections are handled, then
ensure disposeTray() and app.exit(0) are executed in a finally block; update the
code around outlit.shutdown() to use this pattern so shutdown failures or hangs
won't prevent disposal and process exit.
In `@apps/web/src/lib/outlit/index.ts`:
- Around line 1-8: This module instantiates the browser SDK (Outlit) at module
scope which can be evaluated during SSR; add the client directive by inserting
"use client" as the very first line of this file so Next.js treats the module as
a client-only module and avoids evaluating the `@outlit/browser` import/Outlit
instantiation on the server; keep the existing imports and the exported outlit
constant (Outlit, outlit) intact so OutlitProvider.tsx continues to consume the
client-only instance.
🧹 Nitpick comments (6)
apps/desktop/src/renderer/providers/PostHogProvider/PostHogProvider.tsx (1)
14-22: Error log prefix is now slightly misleading.
track("desktop_opened")dispatches to both PostHog and Outlit (seerenderer/lib/analytics/index.ts). Ifoutlit.trackthrows, the catch logs"[posthog] Failed to initialize", which is misleading. Consider updating the prefix/message to reflect the broader scope, e.g."[analytics]".Suggested diff
} catch (error) { - console.error("[posthog] Failed to initialize:", error); + console.error("[analytics] Failed to initialize:", error); } finally {apps/web/src/components/OutlitProvider/OutlitProvider.tsx (1)
1-33: DuplicateOutlitProvideracross web and desktop.This component is identical to
apps/desktop/src/renderer/providers/OutlitProvider/OutlitProvider.tsx. Consider extracting it into a shared package to avoid maintaining two copies. Not blocking for this PR, but worth tracking.apps/docs/src/app/providers.tsx (1)
7-13: Missing explicitReactimport forReact.ReactNodetype.Line 10 references
React.ReactNodebut there's noimport type React from "react"at the top. The web and desktopOutlitProviderfiles include this import. While some TypeScript/bundler configurations auto-resolve this, it's inconsistent with the other providers and may cause type errors depending on the tsconfig.Proposed fix
"use client"; import { OutlitProvider } from "@outlit/browser/react"; +import type React from "react"; import { outlit } from "@/lib/outlit";apps/desktop/src/renderer/lib/analytics/index.ts (1)
5-11: Consider isolating analytics calls to prevent one failure from blocking the other.If
posthog.capturethrows,outlit.trackwill never execute (and vice versa). While analytics SDKs rarely throw synchronously, wrapping each call independently would improve resilience. The same pattern exists inapps/marketing/src/lib/analytics/index.ts, so this could be addressed across both in a follow-up.♻️ Suggested resilience improvement
export function track( event: string, properties?: Record<string, unknown>, ): void { - posthog.capture(event, properties); - outlit.track(event, toOutlitProperties(properties)); + try { + posthog.capture(event, properties); + } catch (e) { + console.error("[analytics/posthog] capture failed", e); + } + try { + outlit.track(event, toOutlitProperties(properties)); + } catch (e) { + console.error("[analytics/outlit] track failed", e); + } }apps/desktop/src/main/lib/analytics/index.ts (1)
61-64: Consider extracting"project_opened"to a named constant.This magic string is used as a sentinel for activation logic. Per coding guidelines, hardcoded strings used in conditional logic should be extracted to named constants for maintainability.
Proposed fix
+// At module top or in shared/constants +const ACTIVATION_EVENT = "project_opened"; + // In track(): - if (event === "project_opened") { + if (event === ACTIVATION_EVENT) { outlit.user.activate({ userId }); }As per coding guidelines: "Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic."
apps/marketing/src/lib/analytics/index.ts (1)
5-21:toOutlitPropertiesis duplicated fromapps/desktop/src/shared/analytics.ts.This exact function exists in
apps/desktop/src/shared/analytics.ts. Since these are separate apps that can't share imports directly, consider extracting it to a shared package (e.g.,packages/analyticsorpackages/utils) if the codebase has that pattern, to keep it DRY.
| NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(), | ||
| NEXT_PUBLIC_POSTHOG_HOST: z.string().default("https://us.i.posthog.com"), | ||
| SENTRY_DSN_DESKTOP: z.string().optional(), | ||
| NEXT_PUBLIC_OUTLIT_KEY: z.string(), |
There was a problem hiding this comment.
NEXT_PUBLIC_OUTLIT_KEY is required but NEXT_PUBLIC_POSTHOG_KEY (Line 21) is optional.
The PR objective states this should "match POSTHOG_KEY", but PostHog is .optional() in this file while Outlit is not. If the Outlit key isn't set in CI or local dev (without SKIP_ENV_VALIDATION), env validation will fail and the app won't start. Consider making it .optional() for parity, or confirm this asymmetry is intentional.
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/env.main.ts` at line 24, The env schema currently
requires NEXT_PUBLIC_OUTLIT_KEY while NEXT_PUBLIC_POSTHOG_KEY is optional;
update the zod schema so NEXT_PUBLIC_OUTLIT_KEY uses z.string().optional() to
match NEXT_PUBLIC_POSTHOG_KEY (symbol: NEXT_PUBLIC_OUTLIT_KEY in the env
schema), or if the asymmetry is intentional, add a comment clarifying why it
must be required and adjust CI/local env docs instead; ensure any dependent code
that assumes the key exists handles undefined when you make it optional.
| // Quit confirmed or no confirmation needed - exit immediately | ||
| // Let OS clean up child processes, tray, etc. | ||
| isQuitting = true; | ||
| await outlit.shutdown(); |
There was a problem hiding this comment.
outlit.shutdown() can prevent app exit if it rejects or hangs.
If the SDK call throws or never resolves, disposeTray() and app.exit(0) are never reached, leaving the app in a zombie state. Wrap in a race with a timeout.
🛡️ Proposed fix
- await outlit.shutdown();
+ await Promise.race([
+ outlit.shutdown().catch((err) =>
+ console.error("[main] Outlit shutdown failed:", err),
+ ),
+ new Promise((resolve) => setTimeout(resolve, 2000)),
+ ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await outlit.shutdown(); | |
| await Promise.race([ | |
| outlit.shutdown().catch((err) => | |
| console.error("[main] Outlit shutdown failed:", err), | |
| ), | |
| new Promise((resolve) => setTimeout(resolve, 2000)), | |
| ]); |
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/index.ts` at line 161, The call to outlit.shutdown()
can hang or reject and block subsequent cleanup (disposeTray() and app.exit(0));
wrap the shutdown in a cancellable timeout race (e.g., Promise.race between
outlit.shutdown() and a timeout promise) and await it with a try/catch so
rejections are handled, then ensure disposeTray() and app.exit(0) are executed
in a finally block; update the code around outlit.shutdown() to use this pattern
so shutdown failures or hangs won't prevent disposal and process exit.
saddlepaddle
force-pushed
the
outlit-cleanup
branch
from
b19c285 to
a073621
Compare
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/docs/src/env.ts`:
- Around line 24-25: The env schema in apps/docs/src/env.ts is inconsistent:
NEXT_PUBLIC_OUTLIT_KEY is required while NEXT_PUBLIC_POSTHOG_KEY is
optional—unlike apps/web and apps/marketing. Update the Zod schema in that file
to make NEXT_PUBLIC_POSTHOG_KEY required (change NEXT_PUBLIC_POSTHOG_KEY:
z.string().optional() to NEXT_PUBLIC_POSTHOG_KEY: z.string()) so both keys match
the other apps' schemas; ensure the symbol names NEXT_PUBLIC_OUTLIT_KEY and
NEXT_PUBLIC_POSTHOG_KEY are updated accordingly.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/providers/PostHogProvider/PostHogProvider.tsx (1)
17-19: Error log prefix may be misleading now thattrack()also calls Outlit.
track("desktop_opened")can now throw from either PostHog or Outlit, but the catch logs[posthog] Failed to initialize:. Consider broadening the prefix to[analytics]so the log accurately reflects the possible error source.Suggested diff
} catch (error) { - console.error("[posthog] Failed to initialize:", error); + console.error("[analytics] Failed to initialize:", error); } finally {apps/marketing/src/lib/analytics/index.ts (2)
23-29: If either analytics call throws, the other is skipped.
posthog.captureandoutlit.trackare called sequentially with no error isolation. Ifposthog.capturethrows (e.g., SDK not initialized, network issue during flush),outlit.trackwill never execute, and the error will propagate to UI event handlers.Consider wrapping each call independently:
Proposed fix
export function track( event: string, properties?: Record<string, unknown>, ): void { - posthog.capture(event, properties); - outlit.track(event, toOutlitProperties(properties)); + try { + posthog.capture(event, properties); + } catch (e) { + console.error("[analytics/posthog] capture failed", e); + } + try { + outlit.track(event, toOutlitProperties(properties)); + } catch (e) { + console.error("[analytics/outlit] track failed", e); + } }
5-21:toOutlitPropertiesis duplicated across apps.This helper is identical to the one in
apps/desktop/src/shared/analytics.ts. If you have or plan a shared package (e.g.,packages/analytics), consider extracting it there. Otherwise, this is acceptable as cross-app duplication.
| NEXT_PUBLIC_OUTLIT_KEY: z.string(), | ||
| NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify the actual code in apps/docs/src/env.ts
echo "=== Checking apps/docs/src/env.ts (lines 20-30) ==="
sed -n '20,30p' apps/docs/src/env.ts
echo -e "\n=== Checking apps/web/src/env.ts (lines 20-30) ==="
sed -n '20,30p' apps/web/src/env.ts
echo -e "\n=== Looking for all env.ts files ==="
fd "env.ts" --type f
echo -e "\n=== Checking .env.example files for OUTLIT_KEY ==="
fd ".env.example" --type f | while read f; do echo "File: $f"; grep -i "outlit\|posthog" "$f" || echo " (no match)"; done
echo -e "\n=== Checking .env files for OUTLIT_KEY ==="
fd ".env$" --type f | while read f; do echo "File: $f"; grep -i "outlit\|posthog" "$f" || echo " (no match)"; doneRepository: superset-sh/superset
Length of output: 1416
🏁 Script executed:
#!/bin/bash
# Search for OUTLIT_KEY in all env.ts files
echo "=== Searching for OUTLIT_KEY in all env.ts files ==="
rg "OUTLIT" apps/*/src/env.ts packages/*/src/env.ts 2>/dev/null
# Check GitHub workflows for OUTLIT setup
echo -e "\n=== Checking GitHub workflows for OUTLIT_KEY ==="
find .github -name "*.yml" -o -name "*.yaml" 2>/dev/null | while read f; do
if grep -q "OUTLIT\|outlit" "$f" 2>/dev/null; then
echo "Found in: $f"
grep -i "outlit" "$f"
fi
done
# Check if OUTLIT_KEY is actually used anywhere in code
echo -e "\n=== Searching for OUTLIT_KEY usage in code ==="
rg "OUTLIT_KEY" --type ts --type tsx --type js --type jsx 2>/dev/null | head -20
# Check docker-compose or docker files for env setup
echo -e "\n=== Checking docker files for OUTLIT setup ==="
find . -name "docker-compose*" -o -name "Dockerfile*" 2>/dev/null | while read f; do
if grep -q -i "outlit" "$f" 2>/dev/null; then
echo "Found in: $f"
grep -i "outlit" "$f"
fi
doneRepository: superset-sh/superset
Length of output: 1627
🏁 Script executed:
#!/bin/bash
# Check POSTHOG_KEY pattern in web and marketing
echo "=== POSTHOG_KEY in apps/web/src/env.ts ==="
rg "POSTHOG_KEY|POSTHOG" apps/web/src/env.ts -A 2 -B 2
echo -e "\n=== POSTHOG_KEY in apps/marketing/src/env.ts ==="
rg "POSTHOG_KEY|POSTHOG" apps/marketing/src/env.ts -A 2 -B 2
echo -e "\n=== POSTHOG_KEY in apps/docs/src/env.ts ==="
rg "POSTHOG_KEY|POSTHOG" apps/docs/src/env.ts -A 2 -B 2
# Check if OUTLIT_KEY is actually used (searching more broadly)
echo -e "\n=== Usage of OUTLIT_KEY in application code ==="
rg "OUTLIT" apps/ packages/ --type ts --type tsx --type js --type jsx 2>/dev/null | grep -v "env.ts" | head -10Repository: superset-sh/superset
Length of output: 2218
OUTLIT_KEY is required but POSTHOG_KEY is optional in docs — inconsistent with other apps
In apps/docs/src/env.ts, NEXT_PUBLIC_OUTLIT_KEY is required while NEXT_PUBLIC_POSTHOG_KEY is optional. This differs from apps/web and apps/marketing, where both are required. This inconsistency means local development will fail if NEXT_PUBLIC_OUTLIT_KEY is not set (unless SKIP_ENV_VALIDATION is used). Either both keys should be required consistently across all apps, or clarify why docs treats them differently.
🤖 Prompt for AI Agents
In `@apps/docs/src/env.ts` around lines 24 - 25, The env schema in
apps/docs/src/env.ts is inconsistent: NEXT_PUBLIC_OUTLIT_KEY is required while
NEXT_PUBLIC_POSTHOG_KEY is optional—unlike apps/web and apps/marketing. Update
the Zod schema in that file to make NEXT_PUBLIC_POSTHOG_KEY required (change
NEXT_PUBLIC_POSTHOG_KEY: z.string().optional() to NEXT_PUBLIC_POSTHOG_KEY:
z.string()) so both keys match the other apps' schemas; ensure the symbol names
NEXT_PUBLIC_OUTLIT_KEY and NEXT_PUBLIC_POSTHOG_KEY are updated accordingly.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
|
/deploy |
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
force-pushed
the
outlit-cleanup
branch
from
a073621 to
0ddf710
Compare
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/marketing/src/components/CookieConsent/CookieConsent.tsx`:
- Line 10: The Outlit SDK is being initialized with tracking enabled on module
load (Outlit constructor in outlit/index.ts), causing events before consent;
change the Outlit initialization to use trackPageviews: false and autoTrack:
false (and disable trackForms if set) so no tracking starts on import, and then
call outlit.enableTracking() from the CookieConsent component when the user
accepts (modify CookieConsent to invoke outlit.enableTracking() on accept).
Apply the same initialization change to the other outlit/index.ts instances
mentioned and ensure only the explicit enableTracking call enables tracking.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/lib/analytics/index.ts (1)
5-11: No telemetry-enabled guard in renderertrack().The main process
track()checksisTelemetryEnabled()before sending events. This renderer version fires unconditionally, relying on SDK-level opt-out (posthog.opt_out_capturing()/outlit.disableTracking()viaTelemetrySync). This works but means events will still be called (just suppressed internally). If this is the intended pattern, consider adding a brief comment explaining the difference from the main process approach.apps/desktop/src/main/lib/analytics/index.ts (1)
61-64: Activation moment on"project_opened"looks correct.Per coding guidelines, consider extracting
"project_opened"to a named constant if it's referenced elsewhere, but this is low priority.apps/marketing/src/lib/analytics/index.ts (2)
5-21: DuplicatetoOutlitProperties— extract to a shared package.This function is identical to the one in
apps/desktop/src/shared/analytics.ts(lines 0-16). Consider extracting it into a shared package (e.g.,packages/sharedor a dedicatedpackages/analytics) so both apps import from a single source.
23-29: Consider using an object parameter fortrack.Per coding guidelines, functions with 2+ parameters should use object parameters. This also makes the API easier to extend (e.g., adding
userIdlater).♻️ Suggested refactor
-export function track( - event: string, - properties?: Record<string, unknown>, -): void { - posthog.capture(event, properties); - outlit.track(event, toOutlitProperties(properties)); +export function track({ + event, + properties, +}: { + event: string; + properties?: Record<string, unknown>; +}): void { + posthog.capture(event, properties); + outlit.track(event, toOutlitProperties(properties)); }Note: this would require updating all call sites (
track("download_clicked")→track({ event: "download_clicked" })). As per coding guidelines, "Use object parameters for functions with 2 or more parameters instead of positional arguments".
| import { useEffect, useState } from "react"; | ||
|
|
||
| import { ANALYTICS_CONSENT_KEY } from "@/lib/constants"; | ||
| import { outlit } from "@/lib/outlit"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find files related to outlit and CookieConsent
fd -t f "outlit" . --extension ts --extension tsx
echo "---"
fd -t f "CookieConsent" . --extension ts --extension tsxRepository: superset-sh/superset
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Search for outlit initialization
find . -path "*/lib/outlit*" -type fRepository: superset-sh/superset
Length of output: 267
🏁 Script executed:
#!/bin/bash
# Get the structure of the apps/marketing directory
ls -la apps/marketing/src/lib/ 2>/dev/null || find apps -type d -name "lib" | head -5Repository: superset-sh/superset
Length of output: 854
🏁 Script executed:
#!/bin/bash
# Look for the CookieConsent file and outlit imports
find . -name "CookieConsent.tsx" -exec cat {} \;Repository: superset-sh/superset
Length of output: 1950
🏁 Script executed:
#!/bin/bash
# Try to find all references to outlit in the codebase
git ls-files | xargs grep -l "outlit" 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 887
🏁 Script executed:
#!/bin/bash
git ls-files | grep -i "outlit" | head -20Repository: superset-sh/superset
Length of output: 493
🏁 Script executed:
#!/bin/bash
# Check if marketing app exists and look at structure
find . -type f -path "*/marketing*" -name "*.tsx" | head -10Repository: superset-sh/superset
Length of output: 718
🏁 Script executed:
#!/bin/bash
# List all files and search for outlit
git ls-files | head -50Repository: superset-sh/superset
Length of output: 1459
🏁 Script executed:
#!/bin/bash
# Simple search for any reference to outlit or CookieConsent
git ls-files | xargs grep -l "outlit\|CookieConsent" 2>/dev/nullRepository: superset-sh/superset
Length of output: 985
🏁 Script executed:
#!/bin/bash
# Check directory structure
ls -la . | grep -E "^d" && echo "---" && ls -la apps/ 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 1632
🏁 Script executed:
#!/bin/bash
test -d apps/marketing && echo "Marketing app exists" || echo "Marketing app does not exist"
find . -type f -name "CookieConsent.tsx" 2>/dev/null
find . -type f -name "*outlit*" 2>/dev/nullRepository: superset-sh/superset
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Search for any Outlit documentation or configuration
rg "enableTracking|disableTracking" -i --type ts --type tsx
rg "new Outlit|Outlit\(" -A 3 --type ts --type tsxRepository: superset-sh/superset
Length of output: 119
🏁 Script executed:
#!/bin/bash
rg "outlit" package.json
grep -r "outlit" . --include="package*.json" 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 350
🏁 Script executed:
#!/bin/bash
# Search for `@/lib/outlit` references
rg "@/lib/outlit" --type ts --type tsx -B 2 -A 5Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
git ls-files --cached 2>/dev/null | grep -E "(outlit|Cookie)" || echo "No matches found"
find . -type f \( -name "*outlit*" -o -name "*Cookie*" \) 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 436
🏁 Script executed:
#!/bin/bash
tree -L 3 -d . 2>/dev/null | head -50 || find . -maxdepth 3 -type d | head -30Repository: superset-sh/superset
Length of output: 861
🏁 Script executed:
#!/bin/bash
# Search all TypeScript/JavaScript files for outlit or CookieConsent
find . \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) -type f -exec grep -l "outlit\|CookieConsent" {} \; 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 911
🏁 Script executed:
#!/bin/bash
# Search for consent-related code patterns
rg "opt.*capturing|tracking|consent" -i --type ts --type tsx | head -30Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
rg "trackPageviews|trackForms" -i --type ts --type tsx -B 3 -A 3Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
rg "TODO|FIXME|BUG" -i --type ts --type tsx | grep -i "track\|consent\|gdpr"Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
ls -la apps/marketing/src/lib/outlit/ 2>/dev/null || echo "Path does not exist"
# Try finding any file with outlit in path
find . -path "*outlit*" -type f 2>/dev/nullRepository: superset-sh/superset
Length of output: 433
🏁 Script executed:
#!/bin/bash
# Search for any configuration that mentions tracking or consent
rg "track|consent|gdpr|privacy" -i --type ts --type tsx -l | head -20Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for similar names
rg "outli|outliner|outlaws" --type ts --type tsx
# Search for common analytics tools
rg "posthog|mixpanel|segment|gtag|analytics" --type ts --type tsx | head -20Repository: superset-sh/superset
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Check if outlit might be a local module or custom implementation
rg "export.*Outlit|class Outlit|function.*outlit" --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Comprehensive search for the exact files mentioned
find . -name "CookieConsent.tsx" -o -name "outlit" -type d 2>/dev/nullRepository: superset-sh/superset
Length of output: 286
🏁 Script executed:
#!/bin/bash
# One final comprehensive search in the repo
find . -type f \( -name "*.ts" -o -name "*.tsx" \) 2>/dev/null | head -100Repository: superset-sh/superset
Length of output: 6471
🏁 Script executed:
#!/bin/bash
pwd && ls -la && echo "---" && git config --get remote.origin.url 2>/dev/null || echo "Not a git repo"Repository: superset-sh/superset
Length of output: 2328
🏁 Script executed:
#!/bin/bash
# Count files and show sample structure
echo "Total files:" && git ls-files | wc -l
echo "Sample files:" && git ls-files | head -30Repository: superset-sh/superset
Length of output: 1085
🏁 Script executed:
#!/bin/bash
# Look for any reference to Outlit in the entire repo
git log --all --oneline --grep="outlit" -i 2>/dev/null | head -10
git ls-files | xargs grep -i "outlit" 2>/dev/null || echo "No outlit found in repo"Repository: superset-sh/superset
Length of output: 9886
🏁 Script executed:
#!/bin/bash
# Search everywhere for tracking-related code
rg "enableTracking|disableTracking|opt_in|opt_out" --type ts --type tsx -B 2 -A 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check package.json files for dependencies
find . -name "package.json" -exec grep -l "track\|outlit\|analytics\|consent" {} \;Repository: superset-sh/superset
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Final attempt - check if maybe it's in node_modules or lock files
find . -name "pnpm-lock.yaml" -o -name "yarn.lock" -o -name "package-lock.json" | head -5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if lock files exist and search for outlit
ls -la | grep -E "lock|package"
rg "outlit" pnpm-lock.yaml 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 253
🏁 Script executed:
#!/bin/bash
# Look for any pending or uncommitted changes
git status 2>/dev/null || echo "Not a git repo"Repository: superset-sh/superset
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Search for PostHog opt_out pattern mentioned in review
rg "opt_out_capturing_by_default|opt_in_capturing" --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check git for any history of these files
git log --all --full-history --name-status -- "*CookieConsent*" 2>/dev/null | head -30
git log --all --full-history --name-status -- "*outlit*" 2>/dev/null | head -30Repository: superset-sh/superset
Length of output: 1864
🏁 Script executed:
cat -n apps/desktop/src/renderer/lib/outlit/index.tsRepository: superset-sh/superset
Length of output: 329
🏁 Script executed:
cat -n apps/web/src/lib/outlit/index.tsRepository: superset-sh/superset
Length of output: 294
🏁 Script executed:
cat apps/marketing/src/app/providers.tsxRepository: superset-sh/superset
Length of output: 814
🏁 Script executed:
web_search
Outlit npm package `@outlit/browser` documentationRepository: superset-sh/superset
Length of output: 159
🏁 Script executed:
wc -l apps/marketing/src/lib/outlit/index.tsRepository: superset-sh/superset
Length of output: 104
🏁 Script executed:
cat apps/marketing/src/lib/outlit/index.tsRepository: superset-sh/superset
Length of output: 257
🏁 Script executed:
web_search
Outlit analytics tracking enabled by default initialization</bash>Repository: superset-sh/superset
Length of output: 272
🏁 Script executed:
cat apps/web/src/lib/outlit/index.tsRepository: superset-sh/superset
Length of output: 238
🏁 Script executed:
cat apps/desktop/src/renderer/components/TelemetrySync/TelemetrySync.tsx | head -60Repository: superset-sh/superset
Length of output: 777
🏁 Script executed:
web_search
Outlit analytics GDPR consent tracking disabled by defaultRepository: superset-sh/superset
Length of output: 159
🏁 Script executed:
# Check where CookieConsent is mounted in the app
grep -r "CookieConsent" apps/marketing/src --include="*.tsx" --include="*.ts"Repository: superset-sh/superset
Length of output: 412
🏁 Script executed:
web_search
"@outlit/browser" npm package documentation enable disable tracking</bash>Repository: superset-sh/superset
Length of output: 280
Initialize Outlit with tracking disabled until consent is granted.
The Outlit instance in apps/marketing/src/lib/outlit/index.ts is initialized with trackPageviews: true and trackForms: true. Since the SDK initializes on module load—before the user interacts with CookieConsent—tracking begins immediately and events are captured before consent is obtained.
Use the pattern from apps/desktop/src/renderer/lib/outlit/index.ts:
new Outlit({
publicKey: env.NEXT_PUBLIC_OUTLIT_KEY,
trackPageviews: false,
autoTrack: false,
});Call outlit.enableTracking() only after the user accepts tracking in CookieConsent.
Also applies to: apps/web/src/lib/outlit/index.ts (has trackPageviews: true), 25-27, 29-33
🤖 Prompt for AI Agents
In `@apps/marketing/src/components/CookieConsent/CookieConsent.tsx` at line 10,
The Outlit SDK is being initialized with tracking enabled on module load (Outlit
constructor in outlit/index.ts), causing events before consent; change the
Outlit initialization to use trackPageviews: false and autoTrack: false (and
disable trackForms if set) so no tracking starts on import, and then call
outlit.enableTracking() from the CookieConsent component when the user accepts
(modify CookieConsent to invoke outlit.enableTracking() on accept). Apply the
same initialization change to the other outlit/index.ts instances mentioned and
ensure only the explicit enableTracking call enables tracking.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
15-20:⚠️ Potential issue | 🟠 MajorStored consent preference is not restored for Outlit on page load.
When the page reloads after a user previously declined,
localStorageis checked only to decide whether to show the banner. PostHog likely respects its own persisted opt-out state, but Outlit is re-initialized fresh (withtrackPageviews: truein the current outlit module) — effectively ignoring the stored "declined" preference.Consider reading the stored consent value on mount and calling
getOutlit().disableTracking()if it was previously declined (orenableTracking()if accepted), similar to how PostHog'sopt_out_capturing_by_defaultworks.useEffect(() => { const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); if (consent === null) { setShowBanner(true); + } else if (consent === "accepted") { + getOutlit().enableTracking(); + } else { + getOutlit().disableTracking(); } }, []);This becomes especially important if the outlit module is fixed to initialize with tracking disabled by default.
🤖 Fix all issues with AI agents
In `@apps/marketing/src/lib/analytics/index.ts`:
- Around line 23-29: The track function currently calls posthog.capture and
getOutlit().track directly which can throw and bubble up to UI; update the track
function to call each analytics SDK inside its own try/catch so failures are
contained, log any caught errors with a contextual prefix like
"[analytics/track]" and the event name (do not rethrow), and continue so a
failing posthog.capture does not prevent getOutlit().track from running (and
vice versa); refer to the track function and the calls posthog.capture and
getOutlit().track to locate where to add these guarded try/catch blocks and
logging.
🧹 Nitpick comments (2)
apps/web/src/lib/outlit/index.ts (1)
1-15: Triplicated Outlit singleton — consider extracting to a shared package.This file is nearly identical to
apps/marketing/src/lib/outlit/index.tsandapps/docs/src/lib/outlit/index.ts(the only difference istrackForms: truein marketing). This violates DRY and means bug fixes or API changes need updating in three places.Consider extracting a shared factory into a workspace package (e.g.,
packages/analytics) that accepts a config object:// packages/analytics/src/outlit.ts export function createOutlitSingleton(config: OutlitConfig) { let instance: Outlit | undefined; return () => { if (!instance) instance = new Outlit(config); return instance; }; }Each app would then just call
createOutlitSingleton({ publicKey, trackPageviews: true }).apps/marketing/src/lib/analytics/index.ts (1)
5-21:toOutlitPropertiesappears duplicated withapps/desktop/src/shared/analytics.ts.The PR description mentions deduplicating this helper into the desktop shared module, but it also exists here. If the logic is identical, consider extracting it to a shared workspace package to keep it DRY.
| export function track( | ||
| event: string, | ||
| properties?: Record<string, unknown>, | ||
| ): void { | ||
| posthog.capture(event, properties); | ||
| getOutlit().track(event, toOutlitProperties(properties)); | ||
| } |
There was a problem hiding this comment.
Analytics calls should not crash the calling component.
If either posthog.capture or getOutlit().track throws (e.g., SDK not loaded, network issue, bad state), the error will propagate to the UI component. Analytics failures should be silent to avoid breaking user-facing functionality.
Proposed fix
export function track(
event: string,
properties?: Record<string, unknown>,
): void {
- posthog.capture(event, properties);
- getOutlit().track(event, toOutlitProperties(properties));
+ try {
+ posthog.capture(event, properties);
+ } catch (error) {
+ console.error("[analytics/posthog]", error);
+ }
+ try {
+ getOutlit().track(event, toOutlitProperties(properties));
+ } catch (error) {
+ console.error("[analytics/outlit]", error);
+ }
}As per coding guidelines, "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly" — here we log with [domain/operation] prefix and handle by continuing rather than rethrowing, since analytics is non-critical.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function track( | |
| event: string, | |
| properties?: Record<string, unknown>, | |
| ): void { | |
| posthog.capture(event, properties); | |
| getOutlit().track(event, toOutlitProperties(properties)); | |
| } | |
| export function track( | |
| event: string, | |
| properties?: Record<string, unknown>, | |
| ): void { | |
| try { | |
| posthog.capture(event, properties); | |
| } catch (error) { | |
| console.error("[analytics/posthog]", error); | |
| } | |
| try { | |
| getOutlit().track(event, toOutlitProperties(properties)); | |
| } catch (error) { | |
| console.error("[analytics/outlit]", error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/marketing/src/lib/analytics/index.ts` around lines 23 - 29, The track
function currently calls posthog.capture and getOutlit().track directly which
can throw and bubble up to UI; update the track function to call each analytics
SDK inside its own try/catch so failures are contained, log any caught errors
with a contextual prefix like "[analytics/track]" and the event name (do not
rethrow), and continue so a failing posthog.capture does not prevent
getOutlit().track from running (and vice versa); refer to the track function and
the calls posthog.capture and getOutlit().track to locate where to add these
guarded try/catch blocks and logging.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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="apps/marketing/src/lib/outlit/index.ts">
<violation number="1" location="apps/marketing/src/lib/outlit/index.ts:17">
P2: Guard the localStorage consent read with try/catch; this unguarded `getItem` can throw in restricted storage environments and crash initialization.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if ( | ||
| typeof window !== "undefined" && | ||
| localStorage.getItem(ANALYTICS_CONSENT_KEY) === "declined" | ||
| ) { | ||
| instance.disableTracking(); | ||
| } |
There was a problem hiding this comment.
P2: Guard the localStorage consent read with try/catch; this unguarded getItem can throw in restricted storage environments and crash initialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/marketing/src/lib/outlit/index.ts, line 17:
<comment>Guard the localStorage consent read with try/catch; this unguarded `getItem` can throw in restricted storage environments and crash initialization.</comment>
<file context>
@@ -11,6 +12,14 @@ export function getOutlit(): Outlit {
});
+
+ // Respect prior consent choice — disable tracking if user previously opted out
+ if (
+ typeof window !== "undefined" &&
+ localStorage.getItem(ANALYTICS_CONSENT_KEY) === "declined"
</file context>
| if ( | |
| typeof window !== "undefined" && | |
| localStorage.getItem(ANALYTICS_CONSENT_KEY) === "declined" | |
| ) { | |
| instance.disableTracking(); | |
| } | |
| if (typeof window !== "undefined") { | |
| try { | |
| if (localStorage.getItem(ANALYTICS_CONSENT_KEY) === "declined") { | |
| instance.disableTracking(); | |
| } | |
| } catch { | |
| instance.disableTracking(); | |
| } | |
| } |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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="apps/marketing/src/app/providers.tsx">
<violation number="1" location="apps/marketing/src/app/providers.tsx:13">
P1: Conditionally skipping `OutlitBrowserProvider` can render `useOutlit()` consumers without a provider during server prerender, which can crash the page.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| function OutlitWrapper({ children }: { children: React.ReactNode }) { | ||
| const client = getOutlit(); | ||
| if (!client) return <>{children}</>; |
There was a problem hiding this comment.
P1: Conditionally skipping OutlitBrowserProvider can render useOutlit() consumers without a provider during server prerender, which can crash the page.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/marketing/src/app/providers.tsx, line 13:
<comment>Conditionally skipping `OutlitBrowserProvider` can render `useOutlit()` consumers without a provider during server prerender, which can crash the page.</comment>
<file context>
@@ -8,10 +8,16 @@ import { PostHogProvider } from "posthog-js/react";
+function OutlitWrapper({ children }: { children: React.ReactNode }) {
+ const client = getOutlit();
+ if (!client) return <>{children}</>;
+ return <OutlitBrowserProvider client={client}>{children}</OutlitBrowserProvider>;
+}
</file context>
3 participants
Summary
NEXT_PUBLIC_OUTLIT_KEYrequired in all env.ts files (matchesPOSTHOG_KEYpattern), eliminating null checks throughoutgetOutlit()with simple module-levelexport const outlitacross all 4 appsuseOutlit()hook requiresOutlitBrowserProviderancestor — replaced with direct module importautoTrack: trueconflicting with manual pageview tracking inindex.tsxtoOutlitPropertiesintoapps/desktop/src/shared/analytics.ts(used by both main + renderer)clientprop instead of inlinepublicKeyTest plan
project_openedfiresuser.activatebun run typecheckpassesbun run lint:fixpassesSummary by CodeRabbit
Release Notes