feat: add analytics support with use-analytics#90
Conversation
Client-side only analytics via use-analytics. Dynamically imports GA plugin inside useEffect to avoid SSR issues. Wraps app in AnalyticsProvider that reads config from chronicle.yaml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR integrates analytics tracking into Chronicle by adding the ChangesAnalytics Integration
Sequence DiagramsequenceDiagram
participant App
participant AnalyticsProvider
participant useAnalytics
participant PageViewTracker
participant AnalyticsCore as analytics instance
App->>AnalyticsProvider: Pass config and children
AnalyticsProvider->>AnalyticsCore: Initialize with plugins (conditionally load Google Analytics)
AnalyticsProvider->>useAnalytics: Provide instance
AnalyticsProvider->>PageViewTracker: Render with instance
Note over PageViewTracker: Route pathname changes
PageViewTracker->>AnalyticsCore: Call page() with new pathname
AnalyticsCore-->>PageViewTracker: Track page view
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 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 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 |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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
🤖 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 `@packages/chronicle/src/components/analytics/AnalyticsProvider.tsx`:
- Around line 47-53: The root wrapper switches between a Fragment and a Provider
which causes remounts; in AnalyticsProvider always render the Provider wrapper
(use Provider instance={analytics} even if analytics is null) so the outer
element type never changes, and move any analytics-dependent children (e.g.,
PageViewTracker) to render conditionally inside the Provider when analytics is
non-null; update the component that returns Provider/children accordingly
(references: AnalyticsProvider, Provider, PageViewTracker, analytics, children).
- Around line 28-45: The current useEffect with dynamic import chains doesn't
catch async failures; change it to an inner async function (e.g., async function
initAnalytics()) that awaits the imports (await
import('`@analytics/google-analytics`') and await import('analytics')) inside a
try/catch so all Promise rejections are caught, build the plugins array after
the awaited googleAnalytics import, and call setAnalytics(Analytics({ app:
'chronicle', plugins })) only after checking a cancellation flag; introduce a
cancelled boolean (let cancelled = false) and return a cleanup () => { cancelled
= true } to avoid state updates after unmount and skip setAnalytics when
cancelled.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2e3dab0f-1c3f-4beb-bfa9-595b266062fb
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
packages/chronicle/package.jsonpackages/chronicle/src/components/analytics/AnalyticsProvider.tsxpackages/chronicle/src/server/App.tsx
| if (!analytics) return <>{children}</> | ||
|
|
||
| return ( | ||
| <Provider instance={analytics}> | ||
| <PageViewTracker /> | ||
| {children} | ||
| </Provider> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/chronicle/src/components/analytics/AnalyticsProvider.tsxRepository: raystack/chronicle
Length of output: 1932
Avoid changing the root wrapper structure on mount.
Lines 49–56 conditionally return either a Fragment or a Provider component. When analytics initializes as null and then updates after the async load completes, React treats these as different root element types, causing child components to remount. This resets internal state and retriggeres effects/fetches.
Consider always rendering the Provider wrapper, or defer initial render until analytics is available.
🤖 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 `@packages/chronicle/src/components/analytics/AnalyticsProvider.tsx` around
lines 47 - 53, The root wrapper switches between a Fragment and a Provider which
causes remounts; in AnalyticsProvider always render the Provider wrapper (use
Provider instance={analytics} even if analytics is null) so the outer element
type never changes, and move any analytics-dependent children (e.g.,
PageViewTracker) to render conditionally inside the Provider when analytics is
non-null; update the component that returns Provider/children accordingly
(references: AnalyticsProvider, Provider, PageViewTracker, analytics, children).
try/catch didn't catch .then() chain failures. Refactored to async/await so all import errors are caught, and added cancellation flag to prevent state updates after unmount. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Review: PR #90 — Add analytics support with use-analytics
Overall: Solid approach. Dynamic imports inside useEffect correctly avoid SSR issues, and the cancellation flag prevents state updates on unmounted components. A few things to tighten up:
Issues
-
AnalyticsProviderwraps the SSR render tree butuse-analyticsis not SSR-safe — The<Provider instance={analytics}>fromuse-analyticsis only rendered whenanalyticsis non-null (client-side), which is correct. However,AnalyticsProvideritself is imported inApp.tsxwhich runs during SSR. Theuse-analyticsandanalyticspackages are not inssr.externalinvite-config.ts. If Vite tries to bundle them for SSR, it may pull in browser-only code from the analytics plugins. Verify this works with a production build (chronicle build && chronicle start), not just dev mode. If it fails, addanalyticsanduse-analyticstossr.external. -
config.analytics ?? {}passes an empty object asAnalyticsConfig— InApp.tsxline 41, when analytics is undefined, you pass{}. ButAnalyticsConfigexpects{ enabled?: boolean; googleAnalytics?: ... }. An empty object works at runtime since all fields are optional, but it would be cleaner to defaultenabledtofalseexplicitly:config.analytics ?? { enabled: false }. This makes the disabled-by-default behavior explicit rather than relying on!config.enabledbeing truthy forundefined. -
Dependencies added to root
package.jsonANDpackages/chronicle/package.json— The lockfile diff shows@analytics/google-analytics,analytics, anduse-analyticsadded to the root workspacepackage.jsonas well as the chronicle package. They should only be inpackages/chronicle/package.json. The root additions are redundant and will cause them to be hoisted differently. -
No way to pass custom analytics plugins — The current design only supports Google Analytics. The
use-analyticslibrary supports many plugins. Consider making the architecture extensible (e.g.,analytics.pluginsarray in config) so users don't need to fork for Plausible, Mixpanel, etc. Not blocking for this PR, but worth a follow-up issue.
Minor
PageViewTrackerfires on every pathname change but not on initial load — TheuseEffectwith[pathname, page]will fire on mount (initial page view) and subsequent navigations, which is correct. But since SSR pre-renders the page, the firstpage()call fires after hydration, which might double-count if GA's script tag also fires a pageview on load. Verify there's no duplicate initial pageview in the Network tab.
2 participants
Summary
AnalyticsProvidercomponent usinguse-analytics+@analytics/google-analyticsuseEffectto avoid SSR crashesanalytics.enabledandanalytics.googleAnalytics.measurementIdfromchronicle.yamluseLocationConfig example
Test plan
chronicle.yamlwith a valid measurement IDgoogle-analytics.comrequests fireenabled: false) — verify no GA requests🤖 Generated with Claude Code