Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive SDK v2.0 features including identity tracking, Web Vitals monitoring, engagement metrics, and framework support for React, Vue, and Next.js.
Changes:
- Added identity tracking (identify, alias, reset) with persistent anonymous IDs
- Implemented Web Vitals tracking (LCP, FCP, FID, CLS, TTFB) and performance monitoring
- Enhanced engagement tracking (active time, scroll depth, interactions, rage clicks)
- Added framework integrations for React, Vue, and Next.js with SSR safety
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/demo.html | Updated demo page showcasing new v2.0 features with improved UI and comprehensive examples |
| packages/analytics-web/transport/sender.js | Enhanced event transport with configurable endpoints, retry logic, and sendBeacon support |
| packages/analytics-web/index.js | Major refactor adding SSR safety, dynamic imports, and framework initialization support |
| packages/analytics-web/frameworks/vue.js | New Vue 3 integration with plugin, router support, and custom directives |
| packages/analytics-web/frameworks/react.js | New React integration with hooks and SSR-safe wrappers |
| packages/analytics-web/frameworks/next.js | New Next.js integration supporting both Pages and App Router |
| packages/analytics-web/events/trackonView.js | Added JSDoc and improved return value for cleanup function |
| packages/analytics-web/events/track.js | Added trackNow function for immediate event delivery using sendBeacon |
| packages/analytics-web/events/performance.js | New comprehensive performance tracking including Web Vitals and resource stats |
| packages/analytics-web/events/navigation.js | New SPA navigation tracking with History API integration |
| packages/analytics-web/events/identify.js | New identity management with identify, alias, and reset functionality |
| packages/analytics-web/events/engagement.js | New engagement metrics tracking (active time, interactions, rage clicks) |
| packages/analytics-web/events/auto.js | Expanded auto-tracking with media, form fields, copy events, and better organization |
| packages/analytics-web/core/session.js | Enhanced session management with expiration, duration tracking, and page view counts |
| packages/analytics-web/core/identity.js | New identity management with anonymous ID, user ID, and traits storage |
| packages/analytics-web/core/event.js | Enhanced event schema with identity, traits, and richer context data |
| packages/analytics-web/core/config.js | New configuration module with SSR detection and initialization logic |
| app/analytics-worker/analyticsWorker.ts | Added placeholder for analytics session computation logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| function attemptSend() { | ||
| const attemptSend = async () => { |
There was a problem hiding this comment.
The function 'attemptSend' is now async but was previously a synchronous function. While the async refactor is appropriate, consider renaming to 'attemptSendAsync' to clearly indicate the behavioral change.
Owner
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| */ | ||
| export const GlimpsePlugin = { | ||
| install(app, options = {}) { | ||
| if (!isBrowser()) return; |
There was a problem hiding this comment.
The isBrowser function is being called as a function but it's imported as a constant from '../core/config.js'. Based on the config.js file, isBrowser is exported as a function, so this is correct. However, the import on line 7 doesn't use destructuring which could cause issues. Ensure the import matches the export pattern.
Owner
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| let sdkPromise = null; | ||
|
|
||
| function loadSDK() { | ||
| if (!isBrowser()) return Promise.resolve(null); |
There was a problem hiding this comment.
Similar to the Vue integration, isBrowser is called as a function. Verify the import statement matches the export from config.js (function vs constant).
Owner
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| else stats.other++; | ||
|
|
||
| // Track slow resources (> 1s) | ||
| if (r.duration > 1000 && stats.slowResources.length < 5) { |
There was a problem hiding this comment.
The magic number 1000 (milliseconds) and 5 (max slow resources) should be extracted as named constants like SLOW_RESOURCE_THRESHOLD and MAX_SLOW_RESOURCES_TRACKED for better maintainability.
Comment on lines
+143
to
+144
| const RAGE_CLICK_THRESHOLD = 3; | ||
| const RAGE_CLICK_WINDOW = 500; // 500ms |
There was a problem hiding this comment.
The rage click detection uses 3 clicks within 500ms. Consider documenting why these specific thresholds were chosen, or if they should be configurable, as rage click patterns may vary across different applications.
Suggested change
| const RAGE_CLICK_THRESHOLD = 3; | |
| const RAGE_CLICK_WINDOW = 500; // 500ms | |
| // Rage click detection heuristic: | |
| // - 3 or more rapid clicks (RAGE_CLICK_THRESHOLD) | |
| // - within a 500ms time window (RAGE_CLICK_WINDOW) | |
| // | |
| // These defaults are based on common UX heuristics for detecting | |
| // "rage clicks" and may need tuning depending on the application | |
| // (e.g., highly interactive UIs or different user behaviors). | |
| // To adjust sensitivity, change these constants rather than the | |
| // detection logic below. | |
| const RAGE_CLICK_THRESHOLD = 3; | |
| const RAGE_CLICK_WINDOW = 500; // 500ms time window for rage click detection |
| if (!sid) { | ||
| sid = `sess_${Math.random().toString(36).substring(2, 15)}`; | ||
| sessionStorage.setItem('analytics_session_id', sid); | ||
| sid = `sess_${crypto.randomUUID ? crypto.randomUUID().slice(0, 12) : Math.random().toString(36).substring(2, 15)}`; |
There was a problem hiding this comment.
Using crypto.randomUUID().slice(0, 12) reduces the UUID from 36 characters to 12, significantly decreasing entropy and increasing collision risk. Consider using the full UUID or a cryptographically secure alternative that maintains sufficient entropy.
Suggested change
| sid = `sess_${crypto.randomUUID ? crypto.randomUUID().slice(0, 12) : Math.random().toString(36).substring(2, 15)}`; | |
| sid = `sess_${crypto.randomUUID ? crypto.randomUUID() : Math.random().toString(36).substring(2, 15)}`; |
| export function getAnonymousId() { | ||
| let anonId = localStorage.getItem(ANON_ID_KEY); | ||
| if (!anonId) { | ||
| anonId = `anon_${crypto.randomUUID ? crypto.randomUUID() : generateUUID()}`; |
There was a problem hiding this comment.
The ternary operator checks if crypto.randomUUID exists but doesn't invoke it. It should be 'crypto.randomUUID()' in both the condition check and the usage. Currently this works due to JavaScript's truthy evaluation, but explicitly checking the function's availability would be clearer.
Suggested change
| anonId = `anon_${crypto.randomUUID ? crypto.randomUUID() : generateUUID()}`; | |
| anonId = `anon_${typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function' ? crypto.randomUUID() : generateUUID()}`; |
Comment on lines
+37
to
+47
| // AnalyticsSession computation | ||
|
|
||
| const analyticsSession = { | ||
| projectId: data.projectId, | ||
| sessionId: data.sessionId, | ||
|
|
||
|
|
||
| createdAt : new Date(), | ||
| updatedAt : new Date(), | ||
| } | ||
|
|
There was a problem hiding this comment.
The analyticsSession object is created but never used. This appears to be incomplete placeholder code with empty lines (42-43). Either complete the implementation or remove this unused code.
Suggested change
| // AnalyticsSession computation | |
| const analyticsSession = { | |
| projectId: data.projectId, | |
| sessionId: data.sessionId, | |
| createdAt : new Date(), | |
| updatedAt : new Date(), | |
| } | |
| // TODO: Implement AnalyticsSession computation and persistence using `data`. |
Comment on lines
+67
to
+70
| platform: navigator.userAgentData ? { | ||
| mobile: navigator.userAgentData.mobile, | ||
| platform: navigator.userAgentData.platform | ||
| } : undefined |
There was a problem hiding this comment.
The navigator.userAgentData API is part of the User-Agent Client Hints specification which has limited browser support. Consider documenting this or adding a feature detection note for developers using older browsers.
Contributor
This was referenced Feb 2, 2026
Contributor
Contributor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 18 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/analytics-web/events/navigation.js">
<violation number="1" location="packages/analytics-web/events/navigation.js:71">
P2: Race condition: `currentPath` is updated asynchronously in the RAF callback, but read synchronously in `handleNavigation`. If multiple navigations happen before the first RAF fires, all will capture the same stale `previousPath`. Update `currentPath` immediately after capturing `previousPath` to prevent this.</violation>
</file>
<file name="packages/analytics-web/core/identity.js">
<violation number="1" location="packages/analytics-web/core/identity.js:9">
P2: Missing error handling for localStorage access. This function accesses `localStorage.getItem()` and `localStorage.setItem()` without try-catch, but `getTraits()` in the same file wraps similar operations in try-catch. localStorage can throw `QuotaExceededError` or `SecurityError` in certain browser contexts (private browsing, disabled storage, quota exceeded). Consider wrapping in try-catch for consistency and resilience.</violation>
</file>
<file name="packages/analytics-web/events/performance.js">
<violation number="1" location="packages/analytics-web/events/performance.js:161">
P2: CLS `value` is a string (from `.toFixed(4)`) while all other Web Vital metrics use numbers (from `Math.round()`). This type inconsistency may cause issues in downstream data processing.</violation>
<violation number="2" location="packages/analytics-web/events/performance.js:209">
P2: Missing retry limit in `trackTTFB` could cause infinite recursion. If the navigation performance entry is never available, `setTimeout(trackTTFB, 100)` will be called indefinitely. Add a maximum retry count or timeout.</violation>
</file>
<file name="packages/analytics-web/events/track.js">
<violation number="1" location="packages/analytics-web/events/track.js:35">
P2: Hardcoded `localhost` fallback URL will cause issues in production. Consider throwing an error or using a production-safe default if endpoint is not configured.</violation>
<violation number="2" location="packages/analytics-web/events/track.js:39">
P1: `sendBeacon` with a string sends `Content-Type: text/plain`, not `application/json`. The server may reject or mishandle this request. Use a `Blob` to specify the correct content type.</violation>
</file>
<file name="packages/analytics-web/frameworks/vue.js">
<violation number="1" location="packages/analytics-web/frameworks/vue.js:8">
P2: Vue integration imports from `react.js`, creating confusing dependencies. The shared utilities (`track`, `trackPageView`, `initGlimpse`, `analytics`) are framework-agnostic and should be extracted to a shared module (e.g., `shared.js` or `common.js`) that both Vue and React integrations can import from.</violation>
</file>
<file name="packages/analytics-web/core/config.js">
<violation number="1" location="packages/analytics-web/core/config.js:85">
P2: `resetInit()` doesn't reset `config` to defaults, only the `initialized` flag. Since `init()` mutates the module-level `config`, this causes test pollution - configuration from previous tests will persist after reset. The function should also restore `config` to its default values.</violation>
</file>
<file name="packages/analytics-web/frameworks/react.js">
<violation number="1" location="packages/analytics-web/frameworks/react.js:124">
P1: The `usePageView` hook is exported and documented as working functionality, but it's actually a non-functional placeholder. The implementation is commented out, so calling this hook will silently do nothing. Either implement the hook properly or remove/document it as a template that users must implement themselves.</violation>
</file>
<file name="packages/analytics-web/core/session.js">
<violation number="1" location="packages/analytics-web/core/session.js:36">
P1: `isNewSession()` doesn't account for expired sessions. When a session has timed out (30 min inactivity), this function still returns `false` because the storage key exists, but `getSessionId()` will create a new session. This inconsistency could cause bugs in code that relies on `isNewSession()` to detect the first page view of a session.</violation>
</file>
<file name="packages/analytics-web/events/engagement.js">
<violation number="1" location="packages/analytics-web/events/engagement.js:210">
P1: Duplicate page exit tracking: both `pagehide` and `beforeunload` events fire during page navigation, causing `trackPageExit()` to be called twice and sending duplicate 'Page Exit' events. Use a flag to prevent double-tracking, or use only `pagehide` (which is the modern, preferred event for this purpose).</violation>
</file>
<file name="packages/analytics-web/events/auto.js">
<violation number="1" location="packages/analytics-web/events/auto.js:191">
P2: The error message is not truncated when `event.reason?.message` exists, unlike other error tracking that limits length. This could result in very large payloads.</violation>
</file>
<file name="packages/analytics-web/transport/sender.js">
<violation number="1" location="packages/analytics-web/transport/sender.js:28">
P1: Events are silently lost if `navigator.sendBeacon` is unavailable. The buffer is cleared before checking sendBeacon availability, and there's no fallback mechanism. Consider checking for sendBeacon first or providing a fallback to regular fetch.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const payload = JSON.stringify(event); | ||
|
|
||
| if (navigator.sendBeacon) { | ||
| navigator.sendBeacon(endpoint, payload); |
There was a problem hiding this comment.
P1: sendBeacon with a string sends Content-Type: text/plain, not application/json. The server may reject or mishandle this request. Use a Blob to specify the correct content type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/events/track.js, line 39:
<comment>`sendBeacon` with a string sends `Content-Type: text/plain`, not `application/json`. The server may reject or mishandle this request. Use a `Blob` to specify the correct content type.</comment>
<file context>
@@ -14,3 +19,29 @@ export function track(name, properties) {
+ const payload = JSON.stringify(event);
+
+ if (navigator.sendBeacon) {
+ navigator.sendBeacon(endpoint, payload);
+ } else {
+ // Fallback to sync XHR
</file context>
Suggested change
| navigator.sendBeacon(endpoint, payload); | |
| navigator.sendBeacon(endpoint, new Blob([payload], { type: 'application/json' })); |
| * return null; | ||
| * } | ||
| */ | ||
| export function usePageView(pathname, options = {}) { |
There was a problem hiding this comment.
P1: The usePageView hook is exported and documented as working functionality, but it's actually a non-functional placeholder. The implementation is commented out, so calling this hook will silently do nothing. Either implement the hook properly or remove/document it as a template that users must implement themselves.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/frameworks/react.js, line 124:
<comment>The `usePageView` hook is exported and documented as working functionality, but it's actually a non-functional placeholder. The implementation is commented out, so calling this hook will silently do nothing. Either implement the hook properly or remove/document it as a template that users must implement themselves.</comment>
<file context>
@@ -0,0 +1,182 @@
+ * return null;
+ * }
+ */
+export function usePageView(pathname, options = {}) {
+ if (!isBrowser()) return;
+
</file context>
| document.addEventListener('click', handleDeadClick, { passive: true }); | ||
|
|
||
| // Page exit tracking | ||
| window.addEventListener('pagehide', trackPageExit); |
There was a problem hiding this comment.
P1: Duplicate page exit tracking: both pagehide and beforeunload events fire during page navigation, causing trackPageExit() to be called twice and sending duplicate 'Page Exit' events. Use a flag to prevent double-tracking, or use only pagehide (which is the modern, preferred event for this purpose).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/events/engagement.js, line 210:
<comment>Duplicate page exit tracking: both `pagehide` and `beforeunload` events fire during page navigation, causing `trackPageExit()` to be called twice and sending duplicate 'Page Exit' events. Use a flag to prevent double-tracking, or use only `pagehide` (which is the modern, preferred event for this purpose).</comment>
<file context>
@@ -0,0 +1,227 @@
+ document.addEventListener('click', handleDeadClick, { passive: true });
+
+ // Page exit tracking
+ window.addEventListener('pagehide', trackPageExit);
+ window.addEventListener('beforeunload', trackPageExit);
+
</file context>
| async function send(events) { | ||
| const endpoint = getEndpoint(); | ||
|
|
||
| for (const event of events) { |
There was a problem hiding this comment.
P1: Events are silently lost if navigator.sendBeacon is unavailable. The buffer is cleared before checking sendBeacon availability, and there's no fallback mechanism. Consider checking for sendBeacon first or providing a fallback to regular fetch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/transport/sender.js, line 28:
<comment>Events are silently lost if `navigator.sendBeacon` is unavailable. The buffer is cleared before checking sendBeacon availability, and there's no fallback mechanism. Consider checking for sendBeacon first or providing a fallback to regular fetch.</comment>
<file context>
@@ -1,55 +1,82 @@
+async function send(events) {
+ const endpoint = getEndpoint();
+
+ for (const event of events) {
const payload = JSON.stringify(event);
let attempts = 0;
</file context>
| const perf = performance.getEntriesByType('navigation')[0]; | ||
| if (!perf) { | ||
| // Wait and retry | ||
| setTimeout(trackTTFB, 100); |
There was a problem hiding this comment.
P2: Missing retry limit in trackTTFB could cause infinite recursion. If the navigation performance entry is never available, setTimeout(trackTTFB, 100) will be called indefinitely. Add a maximum retry count or timeout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/events/performance.js, line 209:
<comment>Missing retry limit in `trackTTFB` could cause infinite recursion. If the navigation performance entry is never available, `setTimeout(trackTTFB, 100)` will be called indefinitely. Add a maximum retry count or timeout.</comment>
<file context>
@@ -0,0 +1,287 @@
+ const perf = performance.getEntriesByType('navigation')[0];
+ if (!perf) {
+ // Wait and retry
+ setTimeout(trackTTFB, 100);
+ return;
+ }
</file context>
| const event = createEvent(projectId, sessionId, name, properties); | ||
|
|
||
| // Use sendBeacon for reliable delivery on page unload | ||
| const endpoint = window.GlimpseTracker?.endpoint || 'http://localhost:3000/event'; |
There was a problem hiding this comment.
P2: Hardcoded localhost fallback URL will cause issues in production. Consider throwing an error or using a production-safe default if endpoint is not configured.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/events/track.js, line 35:
<comment>Hardcoded `localhost` fallback URL will cause issues in production. Consider throwing an error or using a production-safe default if endpoint is not configured.</comment>
<file context>
@@ -14,3 +19,29 @@ export function track(name, properties) {
+ const event = createEvent(projectId, sessionId, name, properties);
+
+ // Use sendBeacon for reliable delivery on page unload
+ const endpoint = window.GlimpseTracker?.endpoint || 'http://localhost:3000/event';
+ const payload = JSON.stringify(event);
+
</file context>
| */ | ||
|
|
||
| import { isBrowser } from '../core/config.js'; | ||
| import { analytics, initGlimpse, track, trackPageView } from './react.js'; |
There was a problem hiding this comment.
P2: Vue integration imports from react.js, creating confusing dependencies. The shared utilities (track, trackPageView, initGlimpse, analytics) are framework-agnostic and should be extracted to a shared module (e.g., shared.js or common.js) that both Vue and React integrations can import from.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/frameworks/vue.js, line 8:
<comment>Vue integration imports from `react.js`, creating confusing dependencies. The shared utilities (`track`, `trackPageView`, `initGlimpse`, `analytics`) are framework-agnostic and should be extracted to a shared module (e.g., `shared.js` or `common.js`) that both Vue and React integrations can import from.</comment>
<file context>
@@ -0,0 +1,160 @@
+ */
+
+import { isBrowser } from '../core/config.js';
+import { analytics, initGlimpse, track, trackPageView } from './react.js';
+
+/**
</file context>
| /** | ||
| * Reset initialization (for testing) | ||
| */ | ||
| export function resetInit() { |
There was a problem hiding this comment.
P2: resetInit() doesn't reset config to defaults, only the initialized flag. Since init() mutates the module-level config, this causes test pollution - configuration from previous tests will persist after reset. The function should also restore config to its default values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/core/config.js, line 85:
<comment>`resetInit()` doesn't reset `config` to defaults, only the `initialized` flag. Since `init()` mutates the module-level `config`, this causes test pollution - configuration from previous tests will persist after reset. The function should also restore `config` to its default values.</comment>
<file context>
@@ -0,0 +1,87 @@
+/**
+ * Reset initialization (for testing)
+ */
+export function resetInit() {
+ initialized = false;
+}
</file context>
| track('Unhandled Promise Rejection', { | ||
| reason: event.reason ? (event.reason.stack || event.reason) : 'unknown' | ||
| track('Promise Rejection', { | ||
| reason: event.reason?.message || String(event.reason)?.substring(0, 200) |
There was a problem hiding this comment.
P2: The error message is not truncated when event.reason?.message exists, unlike other error tracking that limits length. This could result in very large payloads.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/events/auto.js, line 191:
<comment>The error message is not truncated when `event.reason?.message` exists, unlike other error tracking that limits length. This could result in very large payloads.</comment>
<file context>
@@ -1,154 +1,264 @@
- track('Unhandled Promise Rejection', {
- reason: event.reason ? (event.reason.stack || event.reason) : 'unknown'
+ track('Promise Rejection', {
+ reason: event.reason?.message || String(event.reason)?.substring(0, 200)
});
});
</file context>
Suggested change
| reason: event.reason?.message || String(event.reason)?.substring(0, 200) | |
| reason: (event.reason?.message || String(event.reason))?.substring(0, 200) |
Greptile OverviewGreptile SummaryThis PR transforms the analytics SDK from a basic tracker into a comprehensive analytics platform with framework support (React, Next.js, Vue), user identity tracking, session management, and extensive auto-tracking capabilities including Web Vitals, engagement metrics, and user interactions. Major Changes:
Issues Found:
The implementation is well-structured with good separation of concerns, but the critical bugs in initialization and event sending should be addressed before merging. Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| packages/analytics-web/index.js | Added SSR safety, dynamic imports, and framework initialization support with async race condition issue |
| packages/analytics-web/core/session.js | Added session expiration logic, duration tracking, and page view counting with proper timeout handling |
| packages/analytics-web/transport/sender.js | Enhanced batching with configurable endpoint, retry logic, and beacon API for page unload with async iterator issue |
| packages/analytics-web/events/auto.js | Comprehensive auto-tracking including scroll depth, forms, links, errors, media, and engagement metrics |
| packages/analytics-web/frameworks/react.js | React integration with SSR-safe hooks and analytics object, missing actual React dependencies |
| app/analytics-worker/analyticsWorker.ts | Minor updates to worker with incomplete analytics session computation logic |
Sequence Diagram
sequenceDiagram
participant User as User/Browser
participant SDK as index.js (Main SDK)
participant Identity as core/identity.js
participant Session as core/session.js
participant Event as core/event.js
participant Auto as events/auto.js
participant Track as events/track.js
participant Sender as transport/sender.js
participant Server as Backend API
Note over User,Server: SDK Initialization (Vanilla JS)
User->>SDK: Load script with data-project-id
SDK->>SDK: Check isBrowser()
SDK->>SDK: initBrowser() - async imports
par Dynamic Imports
SDK->>Track: import track.js
SDK->>Identity: import identity.js
SDK->>Session: import session.js
SDK->>Sender: import sender.js
end
SDK->>SDK: Parse script attributes
SDK->>SDK: Create window.GlimpseTracker
SDK->>User: Dispatch 'glimpse:ready' event
alt autoTrack enabled
SDK->>Auto: import auto.js
Auto->>Session: isNewSession()
Session-->>Auto: true/false
alt New Session
Auto->>Track: track('Session Start')
end
Auto->>Auto: Setup listeners (scroll, click, forms, etc.)
end
Note over User,Server: Framework Initialization (React/Next/Vue)
User->>SDK: initGlimpse({ projectId, endpoint })
SDK->>SDK: Wait for window.GlimpseTracker
SDK->>SDK: tracker.init(options)
SDK->>Auto: import auto.js (if autoTrack=true)
SDK-->>User: Return tracker instance
Note over User,Server: Identity Tracking
User->>SDK: identify(userId, traits)
SDK->>Identity: setIdentity(userId, traits)
Identity->>Identity: Store in localStorage
Identity-->>SDK: { userId, traits }
SDK->>Track: track('User Identified')
Track->>Event: createEvent('User Identified')
Event->>Identity: getAnonymousId(), getUserId(), getTraits()
Event->>Session: getSessionId()
Event-->>Track: Full event object
Track->>Sender: enqueueEvent(event)
Note over User,Server: Event Batching & Sending
Sender->>Sender: Add to eventBuffer
alt Buffer full (10 events)
Sender->>Sender: flushBuffer()
Sender->>Server: POST /event (batch)
Server-->>Sender: 200 OK
else Timer expires (5s)
Sender->>Sender: flushBuffer()
Sender->>Server: POST /event (batch)
Server-->>Sender: 200 OK
end
Note over User,Server: Auto-Tracking Examples
User->>User: Scroll page
Auto->>Auto: Calculate scroll depth
alt Milestone reached (25%, 50%, etc.)
Auto->>Track: track('Scroll Milestone', { depth })
Track->>Sender: enqueueEvent()
end
User->>User: Click button[data-track]
Auto->>Auto: Detect click on [data-track]
Auto->>Track: track('Button Click', { buttonId, buttonName })
Track->>Sender: enqueueEvent()
User->>User: Submit form
Auto->>Auto: Detect form submit
Auto->>Track: track('Form Submit', { formId, formAction })
Track->>Sender: enqueueEvent()
Note over User,Server: Page Unload
User->>User: Close tab/navigate away
User->>Sender: 'pagehide' event
Sender->>Sender: flush()
Sender->>Server: navigator.sendBeacon(endpoint, events)
Server-->>Sender: Queued for processing
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+200
to
+210
| // Wait for SDK to be ready if not yet initialized | ||
| if (!window.GlimpseTracker) { | ||
| await new Promise(resolve => { | ||
| if (window.GlimpseTracker) { | ||
| resolve(); | ||
| } else { | ||
| window.addEventListener('glimpse:ready', resolve, { once: true }); | ||
| // Timeout fallback | ||
| setTimeout(resolve, 1000); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Race condition in initialization logic - if window.GlimpseTracker already exists but is still initializing, the setTimeout fallback (1000ms) may resolve before the glimpse:ready event fires, causing initGlimpse to return early with a partially initialized tracker.
Suggested change
| // Wait for SDK to be ready if not yet initialized | |
| if (!window.GlimpseTracker) { | |
| await new Promise(resolve => { | |
| if (window.GlimpseTracker) { | |
| resolve(); | |
| } else { | |
| window.addEventListener('glimpse:ready', resolve, { once: true }); | |
| // Timeout fallback | |
| setTimeout(resolve, 1000); | |
| } | |
| }); | |
| // Wait for SDK to be ready if not yet initialized | |
| if (!window.GlimpseTracker) { | |
| await new Promise((resolve, reject) => { | |
| if (window.GlimpseTracker) { | |
| resolve(); | |
| } else { | |
| const timeout = setTimeout(() => { | |
| reject(new Error('GlimpseTracker initialization timeout')); | |
| }, 5000); | |
| window.addEventListener('glimpse:ready', () => { | |
| clearTimeout(timeout); | |
| resolve(); | |
| }, { once: true }); | |
| } | |
| }); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/analytics-web/index.js
Line: 200:210
Comment:
Race condition in initialization logic - if `window.GlimpseTracker` already exists but is still initializing, the setTimeout fallback (1000ms) may resolve before the `glimpse:ready` event fires, causing `initGlimpse` to return early with a partially initialized tracker.
```suggestion
// Wait for SDK to be ready if not yet initialized
if (!window.GlimpseTracker) {
await new Promise((resolve, reject) => {
if (window.GlimpseTracker) {
resolve();
} else {
const timeout = setTimeout(() => {
reject(new Error('GlimpseTracker initialization timeout'));
}, 5000);
window.addEventListener('glimpse:ready', () => {
clearTimeout(timeout);
resolve();
}, { once: true });
}
});
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+24
to
+57
| */ | ||
| async function send(events) { | ||
| const endpoint = getEndpoint(); | ||
|
|
||
| for (const event of events) { | ||
| const payload = JSON.stringify(event); | ||
| let attempts = 0; | ||
|
|
||
| function attemptSend() { | ||
| const attemptSend = async () => { | ||
| attempts++; | ||
| // Use fetch with no credentials for cross-origin requests | ||
| fetch(ENDPOINT, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: payload, | ||
| keepalive: true, | ||
| credentials: 'omit', | ||
| mode: 'cors' | ||
| }).catch((err) => { | ||
| try { | ||
| const response = await fetch(endpoint, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: payload, | ||
| keepalive: true, | ||
| credentials: 'omit', | ||
| mode: 'cors' | ||
| }); | ||
|
|
||
| if (!response.ok && attempts < MAX_RETRIES) { | ||
| setTimeout(attemptSend, RETRY_DELAY * attempts); | ||
| } | ||
| } catch (err) { | ||
| if (attempts < MAX_RETRIES) { | ||
| setTimeout(attemptSend, 1000 * attempts); | ||
| setTimeout(attemptSend, RETRY_DELAY * attempts); | ||
| } else { | ||
| console.error('Failed to send analytics events:', err); | ||
| console.error('GlimpseTracker: Failed to send event', err); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| attemptSend(); | ||
| }) | ||
| } |
There was a problem hiding this comment.
Sequential processing in for...of loop creates performance bottleneck - events are sent one at a time with retry delays blocking subsequent sends. With 10 events and retries, this could delay sending by 30+ seconds.
Suggested change
| */ | |
| async function send(events) { | |
| const endpoint = getEndpoint(); | |
| for (const event of events) { | |
| const payload = JSON.stringify(event); | |
| let attempts = 0; | |
| function attemptSend() { | |
| const attemptSend = async () => { | |
| attempts++; | |
| // Use fetch with no credentials for cross-origin requests | |
| fetch(ENDPOINT, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: payload, | |
| keepalive: true, | |
| credentials: 'omit', | |
| mode: 'cors' | |
| }).catch((err) => { | |
| try { | |
| const response = await fetch(endpoint, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: payload, | |
| keepalive: true, | |
| credentials: 'omit', | |
| mode: 'cors' | |
| }); | |
| if (!response.ok && attempts < MAX_RETRIES) { | |
| setTimeout(attemptSend, RETRY_DELAY * attempts); | |
| } | |
| } catch (err) { | |
| if (attempts < MAX_RETRIES) { | |
| setTimeout(attemptSend, 1000 * attempts); | |
| setTimeout(attemptSend, RETRY_DELAY * attempts); | |
| } else { | |
| console.error('Failed to send analytics events:', err); | |
| console.error('GlimpseTracker: Failed to send event', err); | |
| } | |
| }); | |
| } | |
| } | |
| }; | |
| attemptSend(); | |
| }) | |
| } | |
| async function send(events) { | |
| const endpoint = getEndpoint(); | |
| // Send all events in parallel | |
| await Promise.allSettled(events.map(event => { | |
| const payload = JSON.stringify(event); | |
| const attemptSend = async (attempts = 0) => { | |
| try { | |
| const response = await fetch(endpoint, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: payload, | |
| keepalive: true, | |
| credentials: 'omit', | |
| mode: 'cors' | |
| }); | |
| if (!response.ok && attempts < MAX_RETRIES - 1) { | |
| await new Promise(resolve => setTimeout(resolve, RETRY_DELAY * (attempts + 1))); | |
| return attemptSend(attempts + 1); | |
| } | |
| } catch (err) { | |
| if (attempts < MAX_RETRIES - 1) { | |
| await new Promise(resolve => setTimeout(resolve, RETRY_DELAY * (attempts + 1))); | |
| return attemptSend(attempts + 1); | |
| } else { | |
| console.error('GlimpseTracker: Failed to send event', err); | |
| } | |
| } | |
| }; | |
| return attemptSend(); | |
| })); | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/analytics-web/transport/sender.js
Line: 24:57
Comment:
Sequential processing in `for...of` loop creates performance bottleneck - events are sent one at a time with retry delays blocking subsequent sends. With 10 events and retries, this could delay sending by 30+ seconds.
```suggestion
async function send(events) {
const endpoint = getEndpoint();
// Send all events in parallel
await Promise.allSettled(events.map(event => {
const payload = JSON.stringify(event);
const attemptSend = async (attempts = 0) => {
try {
const response = await fetch(endpoint, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: payload,
keepalive: true,
credentials: 'omit',
mode: 'cors'
});
if (!response.ok && attempts < MAX_RETRIES - 1) {
await new Promise(resolve => setTimeout(resolve, RETRY_DELAY * (attempts + 1)));
return attemptSend(attempts + 1);
}
} catch (err) {
if (attempts < MAX_RETRIES - 1) {
await new Promise(resolve => setTimeout(resolve, RETRY_DELAY * (attempts + 1)));
return attemptSend(attempts + 1);
} else {
console.error('GlimpseTracker: Failed to send event', err);
}
}
};
return attemptSend();
}));
}
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
34
to
48
| case "analytics-job": | ||
| const { data } = job; | ||
| // Process the analytics data | ||
|
|
||
| // AnalyticsSession computation | ||
|
|
||
| const analyticsSession = { | ||
| projectId: data.projectId, | ||
| sessionId: data.sessionId, | ||
|
|
||
|
|
||
| createdAt : new Date(), | ||
| updatedAt : new Date(), | ||
| } | ||
|
|
||
| break; |
There was a problem hiding this comment.
Incomplete implementation leaves analyticsSession object constructed but never used - no database insert or return statement. The break statement exits the switch without processing.
Suggested change
| case "analytics-job": | |
| const { data } = job; | |
| // Process the analytics data | |
| // AnalyticsSession computation | |
| const analyticsSession = { | |
| projectId: data.projectId, | |
| sessionId: data.sessionId, | |
| createdAt : new Date(), | |
| updatedAt : new Date(), | |
| } | |
| break; | |
| case "analytics-job": | |
| const { data } = job; | |
| // AnalyticsSession computation | |
| const analyticsSession = { | |
| projectId: data.projectId, | |
| sessionId: data.sessionId, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| }; | |
| // Store in database | |
| await prisma.analyticsSession.upsert({ | |
| where: { sessionId: analyticsSession.sessionId }, | |
| update: { updatedAt: analyticsSession.updatedAt }, | |
| create: analyticsSession, | |
| }); | |
| break; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/analytics-worker/analyticsWorker.ts
Line: 34:48
Comment:
Incomplete implementation leaves `analyticsSession` object constructed but never used - no database insert or return statement. The `break` statement exits the switch without processing.
```suggestion
case "analytics-job":
const { data } = job;
// AnalyticsSession computation
const analyticsSession = {
projectId: data.projectId,
sessionId: data.sessionId,
createdAt: new Date(),
updatedAt: new Date(),
};
// Store in database
await prisma.analyticsSession.upsert({
where: { sessionId: analyticsSession.sessionId },
update: { updatedAt: analyticsSession.updatedAt },
create: analyticsSession,
});
break;
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+36
to
+38
| export function isNewSession() { | ||
| return !sessionStorage.getItem(SESSION_ID_KEY); | ||
| } |
There was a problem hiding this comment.
isNewSession() has incorrect logic - it returns true only before getSessionId() is first called, but if called after, it always returns false even for genuinely new sessions (e.g., after 30min timeout). This breaks session start tracking.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/analytics-web/core/session.js
Line: 36:38
Comment:
`isNewSession()` has incorrect logic - it returns true only before `getSessionId()` is first called, but if called after, it always returns false even for genuinely new sessions (e.g., after 30min timeout). This breaks session start tracking.
How can I resolve this? If you propose a fix, please make it concise.feat(init): improve GlimpseTracker initialization with timeout handling fix(sender): optimize event sending with concurrent attempts and better retry logic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/analytics-web/index.js">
<violation number="1" location="packages/analytics-web/index.js:206">
P2: Event listener not cleaned up on timeout rejection. When the timeout fires and rejects the Promise, the `glimpse:ready` event listener remains attached. Store the handler and remove it in the timeout callback to prevent the resource leak.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+206
to
+211
| const timeout = setTimeout(() => { | ||
| reject(new Error('GlimpseTracker initialization timeout')); | ||
| }, 5000); | ||
| window.addEventListener('glimpse:ready', () => { | ||
| clearTimeout(timeout); | ||
| resolve(); |
There was a problem hiding this comment.
P2: Event listener not cleaned up on timeout rejection. When the timeout fires and rejects the Promise, the glimpse:ready event listener remains attached. Store the handler and remove it in the timeout callback to prevent the resource leak.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/analytics-web/index.js, line 206:
<comment>Event listener not cleaned up on timeout rejection. When the timeout fires and rejects the Promise, the `glimpse:ready` event listener remains attached. Store the handler and remove it in the timeout callback to prevent the resource leak.</comment>
<file context>
@@ -199,13 +199,17 @@ export async function initGlimpse(options = {}) {
- window.addEventListener('glimpse:ready', resolve, { once: true });
- // Timeout fallback
- setTimeout(resolve, 1000);
+ const timeout = setTimeout(() => {
+ reject(new Error('GlimpseTracker initialization timeout'));
+ }, 5000);
</file context>
Suggested change
| const timeout = setTimeout(() => { | |
| reject(new Error('GlimpseTracker initialization timeout')); | |
| }, 5000); | |
| window.addEventListener('glimpse:ready', () => { | |
| clearTimeout(timeout); | |
| resolve(); | |
| const handler = () => { | |
| clearTimeout(timeout); | |
| resolve(); | |
| }; | |
| const timeout = setTimeout(() => { | |
| window.removeEventListener('glimpse:ready', handler); | |
| reject(new Error('GlimpseTracker initialization timeout')); | |
| }, 5000); | |
| window.addEventListener('glimpse:ready', handler, { once: true }); |
Izume01
added a commit
that referenced
this pull request
Feb 2, 2026
Merge pull request #3 from Izume01/feat/sdk2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Introduces Glimpse Analytics SDK v2 with SSR-safe initialization, React/Next/Vue integrations, identity/session APIs, modular auto-tracking, and a more reliable transport layer. Adds Web Vitals and engagement metrics to improve insight quality.
New Features
Migration
Original PR Description:
Written for commit 85b07b7. Summary will update on new commits.