Release: Firebase Analytics Hardening, SEO Meta Tags, and Glass Effects#104
Release: Firebase Analytics Hardening, SEO Meta Tags, and Glass Effects#104BillChirico merged 58 commits intomainfrom
Conversation
…tform support BREAKING CHANGE: Analytics module moved from lib/analytics.ts to lib/analytics/index.ts - Restructure analytics module to use Metro's built-in platform file resolution - impl.native.ts for iOS/Android (uses @react-native-firebase/analytics) - impl.web.ts for web (uses firebase/analytics JS SDK) - impl.ts as fallback for unsupported platforms - Add custom Expo plugin (withModularHeaders.js) for Firebase iOS compatibility - Configure Firebase config file paths in app.config.ts for both platforms - Add @expo/config-plugins as explicit dependency for pnpm compatibility - Update all test files to use new import paths - Add Firebase config files to .gitignore (environment-specific) Fixes runtime error: "setUserId is not a function (it is undefined)" on iOS caused by Metro bundling both web and native modules with static imports.
Remove explicit return type to avoid JSX namespace issue in React 19. Uses TypeScript inference instead, consistent with other components.
The pnpm symlink structure causes ESLint's import/no-unresolved rule to fail for this package even when properly installed.
…ve conventions Per React Native docs, platform-specific files should be named after what they are (platform), not what they do (impl). This follows the convention: - platform.ts (fallback) - platform.native.ts (iOS/Android) - platform.web.ts (web) Updated all imports and module documentation accordingly.
- Add tests for lib/analytics/platform.ts (fallback implementation) - Add tests for isDebugMode() and getAnalyticsEnvironment() - Coverage now meets all thresholds: - Statements: 80.9% (threshold: 80%) - Branches: 73.82% (threshold: 70%) - Functions: 81.16% (threshold: 80%) - Lines: 82% (threshold: 80%)
Remove the TrendingUp icon from the single metric display to properly center the days count and label. Also simplified the statMain style by removing the now-unused row layout and statMainContent wrapper.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Bill Chirico <bill@chirico.dev>
Eliminate GoogleService-Info.plist and google-services.json from .gitignore to allow for proper inclusion in version control. This change aligns with the project's configuration management practices.
- Modified sign out confirmation to dismiss the modal before signing out, preventing navigation conflicts. - Updated comments in the SettingsScreen component to clarify the new sign out flow. - Enhanced tests for the SettingsScreen to reflect the updated behavior. - Added analytics tracking for successful Apple sign-ins in the AppleSignInButton component, with tests to ensure proper event tracking under various scenarios.
- Remove async from trackEventPlatform, setUserIdPlatform, setUserPropertiesPlatform, and trackScreenViewPlatform - Use fire-and-forget pattern with .catch() for error handling - Fix type safety issue: convert boolean user properties to strings - Prevents unhandled promise rejections when called without await Addresses review feedback about async/sync mismatch in index.ts
When Firebase app was already initialized, the module returned early without setting the analytics variable, causing all subsequent analytics calls to silently fail. Now retrieves the existing app and analytics instances.
Log a warning when required Firebase config values (apiKey, projectId, appId) are empty strings, helping identify configuration issues before they cause silent failures.
- Rename inner callback parameter to avoid shadowing outer config - Allow optional leading whitespace in platform regex for indented Podfiles
Add test coverage for glassTint, glassFallback, and glassBorder properties in dark theme to ensure both theme variants are covered.
- Update native analytics tests to use synchronous assertions - Add flushPromises helper for testing fire-and-forget error handlers - Add tests for boolean-to-string conversion in user properties - Update expected log message for web analytics re-initialization
On native platforms (iOS/Android), Firebase is configured via platform- specific config files (GoogleService-Info.plist / google-services.json), not environment variables. The previous check for EXPO_PUBLIC_FIREBASE_ MEASUREMENT_ID would incorrectly skip initialization on native builds when this web-specific env var wasn't set. Changes: - shouldInitializeAnalytics now returns true on native platforms - Config validation warning only fires on web platform - Added platform-specific test coverage
BREAKING: Firebase configuration files are no longer committed to git. Developers must download their own copies from Firebase Console. Changes: - Remove google-services.json and GoogleService-Info.plist from tracking - Add these files to .gitignore - Update .env.example with setup instructions for native config files Note: These files contain API keys that were exposed in git history. Consider rotating Firebase API keys for production environments.
Add withFirebaseConfig plugin that handles Firebase configuration files during EAS builds by decoding base64-encoded secrets. How it works: 1. During EAS build, reads GOOGLE_SERVICES_JSON and GOOGLE_SERVICE_INFO_PLIST 2. Decodes from base64 and writes to the correct platform directories 3. Falls back to local files for development Setup for production: base64 -i google-services.json > temp.b64 eas secret:create --scope project --name GOOGLE_SERVICES_JSON --value "$(cat temp.b64)"
Update withFirebaseConfig plugin to automatically detect and handle: - File secrets (raw content - RECOMMENDED) - String secrets (base64 encoded - legacy support) The new decodeIfBase64() helper inspects content to determine if it's already raw JSON/XML or needs base64 decoding, making the plugin work seamlessly with either secret type.
Remove base64 decoding logic since EAS file secrets (--type file) will always be used. This eliminates unnecessary complexity: - Removed decodeIfBase64() helper function - Removed Buffer import - Updated docs to show file secret commands only File secrets provide raw content directly, no encoding needed.
- Disable automatic screen reporting (we use manual tracking) - Keep analytics auto-collection enabled - Simplify @react-native-firebase/app plugin config in app.config.ts
- Replace namespaced API with modular imports in platform.native.ts - Use getAnalytics() instance pattern instead of default export - Update test mocks to reflect modular API function signatures - Fix web analytics tests for updated logging format (event_params wrapper) - Fix supabase test mock key consistency (test-anon-key) This fixes deprecation warnings about the namespaced API being removed in the next major release of React Native Firebase. Migration guide: https://rnfirebase.io/migrating-to-v22
- Add shared test-utils.tsx with renderWithProviders helper - Refactor GlassView tests to use shared utilities - Fix withModularHeaders plugin variable reference bug - Remove android/app/google-services.json from version control - Update web analytics with enhanced logging structure
…rning
Replace deprecated logScreenView with logEvent('screen_view', ...) in
native analytics. The logScreenView modular function internally triggers
the deprecation warning even when using the modular API.
Changes:
- Remove logScreenView import, use logEvent with 'screen_view' event
- Add type assertion for flexible event name typing
- Update tests to verify logEvent is called correctly
- Update test to expect hashed user ID in debug logs
- Lazy-initialize Firebase Analytics instance on first use instead of at module load time to prevent crashes when config files are missing - Add null checks and warning logs when Firebase is unavailable - Fix doc comment (8 chars not 16 for hex output) - Fix EAS file secret handling: use fs.copyFileSync instead of fs.writeFileSync since --type file secrets provide a path, not contents - Add comprehensive tests for unavailable Firebase scenarios
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Add WeakSet-based circular reference detection to prevent stack overflow when params contain circular object references during logging.
… conditions - Replace boolean isInitialized flag with Promise-based state machine - Concurrent callers now await the same initialization Promise - Initialize Vercel Analytics independently of Firebase success/failure - Only throw errors when Firebase is the sole provider and fails - Enables proper retry after failures without race conditions
- Replace boolean isInitialized flag with Promise-based state machine - Concurrent callers now await the same initialization Promise - Enables proper retry after failures without race conditions - Consistent pattern with platform.web.ts implementation
- Use object type syntax for props parameter - Mark children as optional (React.ReactNode | undefined) - Use more accurate return type for React.createElement result
- Export __resetForTesting function from index.ts for test isolation - Update analytics.test.ts to reset module state between tests - Remove unnecessary throw in platform.web.ts for graceful error handling - Analytics errors are logged but don't throw (non-critical functionality)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/analytics/platform.web.ts (1)
93-116: Consolidate duplicate reserved-key constants.
LOGGER_RESERVED_KEYS(line 98) andRESERVED_LOGGER_KEYS(line 116) have identical values. Consolidate into a single constant to reduce duplication and maintenance burden.const LOGGER_RESERVED_KEYS = ['error_message', 'error_stack', 'error_name'] as const; ... -/** - * Reserved logger keys that should not be overwritten by user params. - * These keys are used internally by the logger for error information. - */ -const RESERVED_LOGGER_KEYS = ['error_message', 'error_stack', 'error_name'] as const;Then update
sanitizeParamsForLogging(line 257) to useLOGGER_RESERVED_KEYS:- if (RESERVED_LOGGER_KEYS.includes(key as (typeof RESERVED_LOGGER_KEYS)[number])) { + if (LOGGER_RESERVED_KEYS.includes(key as (typeof LOGGER_RESERVED_KEYS)[number])) {
♻️ Duplicate comments (4)
plugins/withFirebaseConfig.js (1)
1-1: Console usage persists despite past review resolution.A previous review flagged the use of
console.log/warn/errorthroughout this file as conflicting with the repo's logging guidelines, which restrict console usage to onlylib/logger.ts,lib/sentry.ts, andjest.setup.js. That review was marked "✅ Addressed in commit 30ef8be," yet all console statements remain in the current code.As a build-time Node.js config plugin, importing the TypeScript-based
lib/logger.ts(which integrates with Sentry for app runtime) may not be practical. However, the blanketeslint-disable no-consoleat line 1 and the continued console usage contradict the stated guidelines without documented justification.Please either:
- Document this file as an explicit, intentional exception to the no-console rule (update lint config or add comment explaining why build plugins may use console), or
- Clarify why the previous review was marked resolved when the console statements were retained.
As per coding guidelines, console usage should be restricted to the three specified files.
Based on learnings, console.log/error/warn are only permitted in logger.ts, sentry.ts, and jest.setup.js.
Also applies to: 121-128, 137-137, 165-165, 176-176, 210-210, 221-221
lib/analytics/platform.ts (1)
53-58: Missing@paramtags fortrackScreenViewPlatform.This function is still missing JSDoc
@paramtags for consistency with other functions in this file and per coding guidelines requiring JSDoc for exported functions./** * Fallback implementation for tracking a screen view on unsupported platforms; performs no action. + * + * @param _screenName - The screen name to track; ignored by this fallback + * @param _screenClass - Optional screen class; ignored by this fallback */ export function trackScreenViewPlatform(_screenName: string, _screenClass?: string): void {app/_layout.tsx (1)
127-146: SEO meta tags implementation looks good.The
seoHeadconstant is well-structured with proper Open Graph and Twitter Card meta tags using absolute URLs. This addresses previous concerns about relative URLs not working for social media crawlers.Minor consideration: The hardcoded domain (
https://sobrietywaypoint.com) works for production but may cause issues in staging environments. Consider extracting this to an environment variable if you need preview cards to work correctly in non-production environments.+// At the top of the file or in a constants file +const BASE_URL = process.env.EXPO_PUBLIC_BASE_URL || 'https://sobrietywaypoint.com'; const seoHead = ( <Head> {/* ... */} - <meta property="og:image" content="https://sobrietywaypoint.com/assets/images/banner.png" /> + <meta property="og:image" content={`${BASE_URL}/assets/images/banner.png`} /> {/* ... */} - <meta name="twitter:image" content="https://sobrietywaypoint.com/assets/images/banner.png" /> + <meta name="twitter:image" content={`${BASE_URL}/assets/images/banner.png`} /> </Head> );lib/analytics/index.ts (1)
145-162: Verify that the catch block can be reached given platform implementations.The error handling here is defensive. However, looking at
platform.web.ts,doInitialize(lines 370-435) explicitly catches errors internally and does not throw (see comment at lines 431-435). Similarly,platform.native.tscatches internally. This means this catch block may never execute unless a platform implementation changes to throw.This is likely intentional future-proofing, but consider adding a comment to clarify this defensive intent:
initializationPromise = doInitialize(config) .then(() => { initializationState = 'completed'; }) .catch((error) => { - // Reset state to allow retry + // Reset state to allow retry + // Note: Currently platform implementations catch internally and don't throw, + // but this handler exists defensively for future changes initializationState = 'failed';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/images/banner.pngis excluded by!**/*.png
📒 Files selected for processing (10)
__mocks__/expoRouterHead.js(1 hunks)__tests__/lib/analytics.test.ts(2 hunks)__tests__/lib/analytics.web.test.ts(3 hunks)app/_layout.tsx(2 hunks)jest.config.js(2 hunks)lib/analytics/index.ts(4 hunks)lib/analytics/platform.native.ts(5 hunks)lib/analytics/platform.ts(1 hunks)lib/analytics/platform.web.ts(8 hunks)plugins/withFirebaseConfig.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: All database operations must use typed client fromtypes/database.ts(source of truth)
Use ErrorBoundary component for graceful failure handling
All imports must use@/path alias (configured in tsconfig.json)
File organization order: imports → types → constants → helpers → main logic → exports with required section comments
JSDoc/TSDoc documentation is mandatory for all exported functions and components
Use mount status tracking with useRef to prevent closure issues in async operations
**/*.{ts,tsx}: Runpnpm typecheckto verify TypeScript types before committing
Write tests for any new or modified code, including happy path, edge cases, and error handling
Every exported function, hook, and component MUST have JSDoc/TSDoc comments that work with IntelliSense
Preferinterfacefor defining object shapes in TypeScript
Use strict mode in TypeScript (strict: truein tsconfig); avoidanytype, useunknownwith type guards instead
Use database types fromtypes/database.tsas the source of truth
Use Supabase client with typed database schema (types/database.ts); all data models derive from database types
Use platform-aware storage: SecureStore (native) / localStorage (web)
Session tokens are stored securely (SecureStore on native, localStorage on web)
Tasks flow unidirectionally from sponsor → sponsee within a relationship
Files:
lib/analytics/platform.tslib/analytics/index.ts__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.tslib/analytics/platform.native.tslib/analytics/platform.web.tsapp/_layout.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: NEVER use console.log/error/warn directly - use logger fromlib/logger.ts
All logs are automatically sent to Sentry as breadcrumbs with categorized metadata using LogCategory enum
Logger reserved keys (error_message, error_stack, error_name) must be avoided in metadata
Boolean variables must use is/has/should prefix (isLoading, hasSlipUps)
**/*.{ts,tsx,js,jsx}: Runpnpm lintto check for code quality issues using ESLint before committing
Use camelCase for function and variable names
Use PascalCase for component and type/interface names
Use SCREAMING_SNAKE_CASE for constants
Use 'is', 'has', or 'should' prefix for boolean variables (e.g.,isLoading,hasSlipUps)
Use 'handle' prefix for event handlers (e.g.,handlePress,handleSubmit)
Name async functions with verbs describing the action (e.g.,fetchTasks,updateProfile)
Always use@/path alias for all local imports
Remove unused imports before committing
Always pass Error objects tologger.error()for stack traces and include contextual metadata
Validate and sanitize all user inputs
Use universal logger (lib/logger.ts) for centralized, structured logging with categorization viaLogCategoryenum
Five log levels available:logger.error(),logger.warn(),logger.info(),logger.debug(),logger.trace()
Organize files logically with sections: imports, types, constants, helper functions, component, styles
Files:
lib/analytics/platform.ts__mocks__/expoRouterHead.jsjest.config.jslib/analytics/index.ts__tests__/lib/analytics.web.test.tsplugins/withFirebaseConfig.js__tests__/lib/analytics.test.tslib/analytics/platform.native.tslib/analytics/platform.web.tsapp/_layout.tsx
**/*.{ts,tsx,js,jsx,json,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm formatto ensure consistent code formatting before committing
Files:
lib/analytics/platform.ts__mocks__/expoRouterHead.jsjest.config.jslib/analytics/index.ts__tests__/lib/analytics.web.test.tsplugins/withFirebaseConfig.js__tests__/lib/analytics.test.tslib/analytics/platform.native.tslib/analytics/platform.web.tsapp/_layout.tsx
!(lib/logger.ts|lib/sentry.ts|jest.setup.js)
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint no-console rule; never use console.log/error/warn directly - use the universal logger instead (exceptions: logger.ts, sentry.ts, jest.setup.js)
Files:
lib/analytics/platform.ts__mocks__/expoRouterHead.jsjest.config.jslib/analytics/index.ts__tests__/lib/analytics.web.test.tsplugins/withFirebaseConfig.js__tests__/lib/analytics.test.tslib/analytics/platform.native.tslib/analytics/platform.web.tsapp/_layout.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: UserenderWithProvidersfrom test-utils to wrap components with AuthContext/ThemeContext in tests
Test mocks must include platform detection (defaults to iOS) and timezone handling
Files:
__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
**/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/__tests__/**/*.test.{ts,tsx}: Runpnpm testto verify all tests pass and coverage stays above 80% before committing
Place test files in__tests__/directory mirroring the source structure (e.g.,app/(tabs)/profile.tsx→__tests__/app/profile.test.tsx)
Use Jest with React Native Testing Library for testing; mock Supabase client for database operations
All tests must use descriptive test names and be independent; group related tests withdescribe()blocks
Clean up mocks inbeforeEach()orafterEach()blocks in tests
Test behavior, not implementation details
Wrap test components with AuthContext, ThemeContext; mock Supabase client for database operations
Use MSW (Mock Service Worker) for API mocking in tests
Add regression tests for bug fixes that would have caught the bug
Update tests when modifying existing code; add new tests for new behavior
UserenderWithProvidershelper function to wrap test components with required contexts
Mock navigation with expo-router mocks in tests
Files:
__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
app/_layout.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Root layout (
app/_layout.tsx) must enforce auth routing: no user →/login, user without profile →/onboarding, complete profile →/(tabs)
app/_layout.tsx: Routing uses Expo Router v6 with typed routes (file-based routing inapp/) and authentication flow enforced in root layout (app/_layout.tsx)
Authentication flow in root layout must route users: unauthenticated → /login, authenticated without profile → /onboarding, fully onboarded → /(tabs)
Wrap app with Sentry SDK in root component for crash reporting and error tracking
Load JetBrains Mono font via expo-font and hide splash screen after fonts load inapp/_layout.tsx
Files:
app/_layout.tsx
{app/_layout.tsx,**/*context*.{ts,tsx}}
📄 CodeRabbit inference engine (AGENTS.md)
Use Context API only (no Redux/Zustand) - contexts must wrap entire app in root layout
Files:
app/_layout.tsx
🧠 Learnings (44)
📓 Common learnings
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Mock navigation with expo-router mocks in tests
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:08:12.192Z
Learning: Applies to src/app/page.tsx : Homepage must use `Promise.allSettled()` for data fetching to tolerate partial content loading failures
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to jest.setup.js : AsyncStorage and SecureStore must be mocked to prevent actual storage during tests
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to app/_layout.tsx : Routing uses Expo Router v6 with typed routes (file-based routing in `app/`) and authentication flow enforced in root layout (`app/_layout.tsx`)
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to src/app/page.tsx : Use Promise.allSettled in src/app/page.tsx to tolerate partial content loading failures and log issues via reportError
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to src/**/*.{ts,tsx} : Use vercel/analytics for user analytics tracking in the application
Applied to files:
lib/analytics/platform.tslib/analytics/index.ts__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.tslib/analytics/platform.web.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/*.{ts,tsx} : Every exported function, hook, and component MUST have JSDoc/TSDoc comments that work with IntelliSense
Applied to files:
lib/analytics/platform.ts
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to **/*.{ts,tsx} : JSDoc/TSDoc documentation is mandatory for all exported functions and components
Applied to files:
lib/analytics/platform.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Mock navigation with expo-router mocks in tests
Applied to files:
__mocks__/expoRouterHead.jsjest.config.jsapp/_layout.tsx
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to **/*.test.{ts,tsx} : Test mocks must include platform detection (defaults to iOS) and timezone handling
Applied to files:
__mocks__/expoRouterHead.jsjest.config.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.tslib/analytics/platform.web.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use MSW (Mock Service Worker) for API mocking in tests
Applied to files:
__mocks__/expoRouterHead.jsjest.config.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to jest.setup.ts : Initialize testing-library/jest-dom and mocks in jest.setup.ts for unit test setup
Applied to files:
__mocks__/expoRouterHead.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Clean up mocks in `beforeEach()` or `afterEach()` blocks in tests
Applied to files:
__mocks__/expoRouterHead.jsjest.config.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Test behavior, not implementation details
Applied to files:
__mocks__/expoRouterHead.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to jest.setup.js : AsyncStorage and SecureStore must be mocked to prevent actual storage during tests
Applied to files:
__mocks__/expoRouterHead.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use Jest with React Native Testing Library for testing; mock Supabase client for database operations
Applied to files:
__mocks__/expoRouterHead.jsjest.config.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use `renderWithProviders` helper function to wrap test components with required contexts
Applied to files:
__mocks__/expoRouterHead.js
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to **/*.test.{ts,tsx} : Use `renderWithProviders` from test-utils to wrap components with AuthContext/ThemeContext in tests
Applied to files:
__mocks__/expoRouterHead.js
📚 Learning: 2025-12-02T00:08:12.192Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:08:12.192Z
Learning: Applies to tests/**/*.{ts,tsx,js} : Use Jest for unit tests located in `tests/` directory
Applied to files:
jest.config.js
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to {jest.config.ts,jest.setup.ts,tests/**/*.{ts,tsx}} : Use Jest with Testing Library for unit tests located in tests/ directory with configuration via jest.config.ts and setup via jest.setup.ts
Applied to files:
jest.config.js__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to tests/**/*.{ts,tsx} : Add comprehensive test coverage for components, hooks, utilities, and API routes in tests/ directory
Applied to files:
jest.config.js__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Place test files in `__tests__/` directory mirroring the source structure (e.g., `app/(tabs)/profile.tsx` → `__tests__/app/profile.test.tsx`)
Applied to files:
jest.config.js
📚 Learning: 2025-12-02T00:08:12.192Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:08:12.192Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use path alias `@/*` mapping to `src/*` as configured in tsconfig
Applied to files:
jest.config.js
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Run `pnpm test` to verify all tests pass and coverage stays above 80% before committing
Applied to files:
jest.config.js__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Always use `@/` path alias for all local imports
Applied to files:
jest.config.js
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Update tests when modifying existing code; add new tests for new behavior
Applied to files:
__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Add regression tests for bug fixes that would have caught the bug
Applied to files:
__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : All tests must use descriptive test names and be independent; group related tests with `describe()` blocks
Applied to files:
__tests__/lib/analytics.web.test.ts__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/*.{ts,tsx} : Write tests for any new or modified code, including happy path, edge cases, and error handling
Applied to files:
__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to {lib/logger.ts,lib/sentry.ts,jest.setup.js} : Console.log/error/warn are only permitted in logger.ts, sentry.ts, and jest.setup.js
Applied to files:
__tests__/lib/analytics.web.test.tsplugins/withFirebaseConfig.js
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to !(lib/logger.ts|lib/sentry.ts|jest.setup.js) : Use ESLint no-console rule; never use console.log/error/warn directly - use the universal logger instead (exceptions: logger.ts, sentry.ts, jest.setup.js)
Applied to files:
plugins/withFirebaseConfig.js
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : NEVER use console.log/error/warn directly - use logger from `lib/logger.ts`
Applied to files:
plugins/withFirebaseConfig.js
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to src/**/*.{ts,tsx} : Use reportError(context, error) from src/lib/logger.ts to report errors to Sentry with context metadata, falling back to console.error
Applied to files:
plugins/withFirebaseConfig.js
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Run `pnpm lint` to check for code quality issues using ESLint before committing
Applied to files:
plugins/withFirebaseConfig.js
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Logger reserved keys (error_message, error_stack, error_name) must be avoided in metadata
Applied to files:
plugins/withFirebaseConfig.js
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to jest.setup.js : Supabase client must be mocked with chainable query builder in jest.setup.js
Applied to files:
__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Wrap test components with AuthContext, ThemeContext; mock Supabase client for database operations
Applied to files:
__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to {src/app/layout.tsx,src/app/blog/[slug]/page.tsx} : Define SEO metadata in src/app/layout.tsx for the site root, and generate dynamic metadata for blog posts in src/app/blog/[slug]/page.tsx including OpenGraph and Twitter card metadata
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to app/_layout.tsx : Routing uses Expo Router v6 with typed routes (file-based routing in `app/`) and authentication flow enforced in root layout (`app/_layout.tsx`)
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to app/_layout.tsx : Wrap app with Sentry SDK in root component for crash reporting and error tracking
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to src/**/*.{ts,tsx} : Use generateHeadingId(text, fallback?) from src/lib/utils.ts to create URL-safe heading IDs from text with hash-based fallbacks for non-ASCII content
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-02T00:08:12.192Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:08:12.192Z
Learning: Applies to **/*.{ts,tsx} : Use `generateHeadingId()` to create URL-safe heading IDs with hash-based fallbacks for international content
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to app/_layout.tsx : Root layout (`app/_layout.tsx`) must enforce auth routing: no user → `/login`, user without profile → `/onboarding`, complete profile → `/(tabs)`
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to {app/_layout.tsx,**/*context*.{ts,tsx}} : Use Context API only (no Redux/Zustand) - contexts must wrap entire app in root layout
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to app/_layout.tsx : Authentication flow in root layout must route users: unauthenticated → /login, authenticated without profile → /onboarding, fully onboarded → /(tabs)
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Deep linking scheme must use `sobrietywaypoint://` for native OAuth callbacks
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Use Context API for global state (AuthContext, ThemeContext) wrapping the entire app in `app/_layout.tsx`; do not use Redux/Zustand
Applied to files:
app/_layout.tsx
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/components/**/*.{ts,tsx} : Use functional components with hooks (no class components) in React/React Native
Applied to files:
app/_layout.tsx
🧬 Code graph analysis (5)
lib/analytics/index.ts (3)
lib/analytics/platform.web.ts (4)
initializePlatformAnalytics(456-492)trackScreenViewPlatform(627-632)resetAnalyticsPlatform(639-645)__resetForTesting(658-667)lib/analytics/platform.native.ts (3)
initializePlatformAnalytics(59-85)trackScreenViewPlatform(262-288)resetAnalyticsPlatform(295-317)lib/analytics-utils.ts (2)
isDebugMode(113-115)shouldInitializeAnalytics(93-103)
__tests__/lib/analytics.web.test.ts (2)
lib/analytics/index.ts (3)
__resetForTesting(265-271)setUserId(205-207)setUserProperties(226-228)lib/analytics/platform.web.ts (1)
__resetForTesting(658-667)
__tests__/lib/analytics.test.ts (2)
lib/analytics/index.ts (1)
__resetForTesting(265-271)lib/analytics/platform.web.ts (1)
__resetForTesting(658-667)
lib/analytics/platform.web.ts (6)
types/analytics.ts (2)
EventParams(30-32)AnalyticsConfig(65-70)lib/logger.ts (1)
logger(256-262)lib/analytics/platform.native.ts (1)
initializePlatformAnalytics(59-85)lib/analytics/platform.ts (1)
initializePlatformAnalytics(19-23)lib/analytics-utils.ts (1)
isDebugMode(113-115)lib/analytics/index.ts (1)
__resetForTesting(265-271)
app/_layout.tsx (1)
jest.setup.js (1)
View(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build for Web
- GitHub Check: Cursor Bugbot
- GitHub Check: claude-review
🔇 Additional comments (35)
plugins/withFirebaseConfig.js (4)
28-69: LGTM! Validation helpers are well-implemented.The new validation functions properly address the previous fragile first-character checks with robust format detection:
isValidJsonsafely validates JSON via parse attemptisValidPlistchecks for XML declaration, DOCTYPE, and proper plist tag structuredetectConfigFormatprovides a clean, typed abstractionNote: Binary plist files will be detected as invalid and written as-is, which is acceptable since Firebase configuration files are expected to be text-based XML plists.
85-140: LGTM! Robust secret handling with proper fallbacks.The function correctly handles all three input formats (absolute file path, base64-encoded content, raw JSON/plist) with appropriate fallbacks:
- Path detection now uses
path.isAbsolutefor cross-platform compatibility ✓- Proper validation via
detectConfigFormatbefore writing ✓- Clear warning messages when content cannot be validated ✓
- Comprehensive error handling for write failures ✓
The implementation addresses all previous review concerns.
154-186: LGTM! Clear fallback chain for Android configuration.The function properly implements the EAS-secret-first, local-file-fallback pattern:
- Attempts EAS secret (
GOOGLE_SERVICES_JSON) first- Falls back to project root file for local development
- Provides clear warning when neither source is available
The logic correctly handles both build environments and development workflows.
198-231: LGTM! iOS configuration mirrors Android pattern consistently.The function follows the same well-structured pattern as the Android version:
- EAS secret first (
GOOGLE_SERVICE_INFO_PLIST)- Local file fallback
- Clear warning when neither source is available
- Defensive fallback for project name (line 203)
Consistent structure between platforms aids maintainability.
lib/analytics/platform.ts (1)
14-23: LGTM!The JSDoc documentation update clearly communicates that this is a fallback implementation and that the config parameter is ignored. The warning log ensures developers are aware when the fallback is active.
jest.config.js (2)
6-8: LGTM!Adding
expo-routertotransformIgnorePatternsensures Babel/Jest correctly handles expo-router modules during tests.
25-26: LGTM!The
moduleNameMapperentry correctly mapsexpo-router/headto the mock implementation, enabling tests that import components usingHeadfromexpo-router/headto run without ESM parsing errors. Based on learnings, expo-router mocks should be used in tests.__mocks__/expoRouterHead.js (1)
1-19: LGTM!The mock correctly implements a passthrough
Headcomponent that renders children in a React Fragment. The ESModule interop is properly configured with__esModule = true,module.exports, andmodule.exports.defaultto support both CommonJS and ESModule import styles. This mock enables tests that use components importingHeadfromexpo-router/head.app/_layout.tsx (2)
31-31: LGTM!The
Headimport fromexpo-router/headenables declarative SEO meta tag management, which is the recommended approach for Expo Router web builds.
148-157: LGTM!Rendering
seoHeadunconditionally in both the loading state and the main render ensures that SEO meta tags are present in the initial HTML for search engines and social media crawlers, which typically don't execute JavaScript.__tests__/lib/analytics.test.ts (2)
13-13: LGTM!Importing
__resetForTestingenables proper test isolation by resetting the module's internal initialization state between tests.
68-74: LGTM!Calling
__resetForTesting()inbeforeEachensures each test starts with a clean analytics initialization state, preventing test pollution from prior initialization attempts. This follows the coding guideline to clean up mocks inbeforeEach()orafterEach()blocks.__tests__/lib/analytics.web.test.ts (9)
23-23: LGTM!Importing
__resetForTestingfrom the web platform module enables proper test isolation for web-specific analytics initialization state.
64-72: LGTM!Properly resetting module state and clearing mocks in
beforeEachensures test isolation. The order (reset state first, then clear mocks) is correct.
354-393: LGTM!The "Provider Configuration" tests verify Firebase initialization and debug logging behavior. The test coverage is comprehensive.
395-466: LGTM!The "Error Handling" tests comprehensively cover graceful degradation for Firebase errors, including both
Errorobjects and non-Error exceptions. This aligns with the PR objective of analytics hardening.
468-522: LGTM!The "Idempotency and Re-initialization" tests verify the Promise-based initialization guard prevents duplicate initialization and correctly allows re-initialization after
__resetForTesting(). This is a key regression test for the hot reload/Strict Mode fix.
587-658: LGTM!The "User Properties Filtering" tests verify that undefined values are filtered out while preserving legitimate falsy values like
false. This prevents unintended data from being sent to Firebase.
660-738: Excellent PII sanitization test coverage.The "Parameter Sanitization for Logging" tests verify that PII-prone keys (like
error_message) are properly filtered from debug logs. This is important for compliance and preventing sensitive data leakage.
740-760: LGTM!Testing that
__resetForTestingthrows in non-test environments is a good defensive measure to prevent accidental production usage.
762-816: Comprehensive edge case coverage.The tests cover null params, empty objects, empty strings, long event names, special characters, and circular references. This ensures the analytics module handles unexpected inputs gracefully without crashing.
lib/analytics/index.ts (3)
46-72: Promise-based initialization pattern is well-designed.The state machine approach (
null→pending→completed/skipped/failed) with a shared Promise effectively prevents race conditions from concurrent initialization attempts (hot reload, React Strict Mode). Concurrent callers await the same Promise, and completed state returns immediately.
242-252: Documentation improvement addresses previous review feedback.The updated JSDoc now explicitly clarifies that
resetAnalyticsdoes not reset the initialization state. This directly addresses the previous Copilot comment requesting this clarification.
265-271: Testing utility is properly guarded.The
__resetForTestinghelper correctly throws when called outside test environments, preventing accidental use in production. This aligns with the same pattern inplatform.web.tsand enables proper test isolation.lib/analytics/platform.native.ts (5)
52-85: Documentation updates clarify native platform behavior.The updated JSDoc for
initializePlatformAnalyticsproperly documents that the_configparameter is ignored on native platforms (which use bundled config files). The error handling pattern (catch, log, don't throw) is consistent with the non-critical nature of analytics.
146-174: LGTM —setUserIdPlatformdocumentation is clear.The updated docstring accurately describes the
nullclearing behavior and the error handling policy (logged, not propagated). This aligns with the implementation.
214-252: LGTM —setUserPropertiesPlatformdocumentation is comprehensive.The docstring now clearly describes the type conversions (boolean →
'true'/'false',nullclears,undefinedignored) and error handling policy. This matches the implementation and helps consumers understand the API contract.
254-288: Documentation accurately describes fire-and-forget behavior.The updated docstring for
trackScreenViewPlatformcorrectly notes the fire-and-forget pattern with non-throwing error handling. The type assertion workaround for Firebase's strict overloads is pragmatic.
290-317: Documentation clarifiesresetAnalyticsPlatformedge cases.The updated docstring now explicitly describes behavior when Firebase is unavailable (logs warning, returns without error) and the error handling policy. This improves API clarity.
lib/analytics/platform.web.ts (6)
76-88: Promise-based initialization pattern matchesindex.tsapproach.The shared
initializationPromiseand state machine pattern consistently prevents race conditions across concurrent calls. This addresses the previous review concerns about flag desynchronization.
245-282: Circular reference detection is a good defensive addition.Using a
WeakSetto track visited objects prevents infinite recursion when logging params with circular references. The[Circular]placeholder provides clear feedback without crashing.
370-435: Firebase failure no longer blocks Vercel initialization — addresses previous review.The refactored
doInitializenow captures Firebase errors infirebaseError(line 412) but continues toinitializeVercelAnalytics()(line 420). This addresses the previous cursor[bot] concern that Firebase failures would skip Vercel entirely.The explicit design decision to not throw on analytics failures (lines 432-435) is appropriate since analytics is non-critical to app functionality.
634-645: Documentation clarifies Vercel Analytics limitation.The updated docstring for
resetAnalyticsPlatformnow explicitly states that Vercel Analytics is a no-op due to lacking a reset API. This sets correct expectations for consumers.
658-667: Testing utility is properly implemented.The
__resetForTestinghelper correctly:
- Guards against non-test environment usage
- Resets all module state (
initializationState,initializationPromise,analytics,app,vercelAnalytics)This enables proper test isolation, aligning with learnings about mocking and test setup requirements.
456-492: Promise-based wrapper coordinates initialization correctly.The
initializePlatformAnalyticswrapper properly:
- Returns early if completed
- Awaits in-progress initialization to prevent race conditions
- Sets pending state before starting
The catch block (lines 484-489) is indeed defensive—
doInitializeintentionally doesn't throw (by design), as documented at lines 432-433. Both Firebase and Vercel initialization failures are caught and logged gracefully rather than propagated, making the re-throw pattern future-proof for any changes to error handling.
- Expand withFirebaseConfig secretValue docs with detailed format examples - Simplify expoRouterHead mock return type annotation
… paths Add comprehensive tests for previously uncovered code paths: - Concurrent initialization handling (Promise-based state machine) - Debug logging when skipping re-initialization - Debug logging when clearing user ID (null) without initialization - Graceful failure handling with state completion Addresses patch coverage feedback from PR #104 review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/analytics/platform.web.ts
Outdated
| continue; | ||
| } | ||
| seen.add(value); | ||
| sanitized[key] = sanitizeParamsForLogging(value as EventParams, seen); |
There was a problem hiding this comment.
The WeakSet tracking circular references is never cleaned up between calls. If the same object is used in multiple non-circular contexts across different sanitizeParamsForLogging calls, it will be incorrectly marked as [Circular] on subsequent calls. The seen WeakSet should be scoped to a single sanitization operation, which it is for the initial call, but objects added during recursion remain in the set for the entire call chain.
| sanitized[key] = sanitizeParamsForLogging(value as EventParams, seen); | |
| sanitized[key] = sanitizeParamsForLogging(value as EventParams, seen); | |
| seen.delete(value); |
| it('handles params with circular references gracefully', () => { | ||
| const circular: any = { prop: 'value' }; | ||
| circular.self = circular; | ||
|
|
||
| // This should not crash, even if logging might fail internally | ||
| expect(() => trackEventWeb('test_event', circular)).not.toThrow(); | ||
| }); |
There was a problem hiding this comment.
This test verifies that circular references don't crash but doesn't verify the actual sanitization behavior. Add an assertion to check that the circular reference is properly replaced with '[Circular]' in the debug logs.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
__tests__/lib/analytics.test.ts(3 hunks)__tests__/lib/analytics.web.test.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: All database operations must use typed client fromtypes/database.ts(source of truth)
Use ErrorBoundary component for graceful failure handling
All imports must use@/path alias (configured in tsconfig.json)
File organization order: imports → types → constants → helpers → main logic → exports with required section comments
JSDoc/TSDoc documentation is mandatory for all exported functions and components
Use mount status tracking with useRef to prevent closure issues in async operations
**/*.{ts,tsx}: Runpnpm typecheckto verify TypeScript types before committing
Write tests for any new or modified code, including happy path, edge cases, and error handling
Every exported function, hook, and component MUST have JSDoc/TSDoc comments that work with IntelliSense
Preferinterfacefor defining object shapes in TypeScript
Use strict mode in TypeScript (strict: truein tsconfig); avoidanytype, useunknownwith type guards instead
Use database types fromtypes/database.tsas the source of truth
Use Supabase client with typed database schema (types/database.ts); all data models derive from database types
Use platform-aware storage: SecureStore (native) / localStorage (web)
Session tokens are stored securely (SecureStore on native, localStorage on web)
Tasks flow unidirectionally from sponsor → sponsee within a relationship
Files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: NEVER use console.log/error/warn directly - use logger fromlib/logger.ts
All logs are automatically sent to Sentry as breadcrumbs with categorized metadata using LogCategory enum
Logger reserved keys (error_message, error_stack, error_name) must be avoided in metadata
Boolean variables must use is/has/should prefix (isLoading, hasSlipUps)
**/*.{ts,tsx,js,jsx}: Runpnpm lintto check for code quality issues using ESLint before committing
Use camelCase for function and variable names
Use PascalCase for component and type/interface names
Use SCREAMING_SNAKE_CASE for constants
Use 'is', 'has', or 'should' prefix for boolean variables (e.g.,isLoading,hasSlipUps)
Use 'handle' prefix for event handlers (e.g.,handlePress,handleSubmit)
Name async functions with verbs describing the action (e.g.,fetchTasks,updateProfile)
Always use@/path alias for all local imports
Remove unused imports before committing
Always pass Error objects tologger.error()for stack traces and include contextual metadata
Validate and sanitize all user inputs
Use universal logger (lib/logger.ts) for centralized, structured logging with categorization viaLogCategoryenum
Five log levels available:logger.error(),logger.warn(),logger.info(),logger.debug(),logger.trace()
Organize files logically with sections: imports, types, constants, helper functions, component, styles
Files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: UserenderWithProvidersfrom test-utils to wrap components with AuthContext/ThemeContext in tests
Test mocks must include platform detection (defaults to iOS) and timezone handling
Files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
**/*.{ts,tsx,js,jsx,json,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm formatto ensure consistent code formatting before committing
Files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
**/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/__tests__/**/*.test.{ts,tsx}: Runpnpm testto verify all tests pass and coverage stays above 80% before committing
Place test files in__tests__/directory mirroring the source structure (e.g.,app/(tabs)/profile.tsx→__tests__/app/profile.test.tsx)
Use Jest with React Native Testing Library for testing; mock Supabase client for database operations
All tests must use descriptive test names and be independent; group related tests withdescribe()blocks
Clean up mocks inbeforeEach()orafterEach()blocks in tests
Test behavior, not implementation details
Wrap test components with AuthContext, ThemeContext; mock Supabase client for database operations
Use MSW (Mock Service Worker) for API mocking in tests
Add regression tests for bug fixes that would have caught the bug
Update tests when modifying existing code; add new tests for new behavior
UserenderWithProvidershelper function to wrap test components with required contexts
Mock navigation with expo-router mocks in tests
Files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
!(lib/logger.ts|lib/sentry.ts|jest.setup.js)
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint no-console rule; never use console.log/error/warn directly - use the universal logger instead (exceptions: logger.ts, sentry.ts, jest.setup.js)
Files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Mock navigation with expo-router mocks in tests
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to jest.setup.js : AsyncStorage and SecureStore must be mocked to prevent actual storage during tests
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to **/*.test.{ts,tsx} : Test mocks must include platform detection (defaults to iOS) and timezone handling
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to jest.setup.ts : Initialize testing-library/jest-dom and mocks in jest.setup.ts for unit test setup
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to tests/**/*.{ts,tsx} : Add comprehensive test coverage for components, hooks, utilities, and API routes in tests/ directory
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use Jest with React Native Testing Library for testing; mock Supabase client for database operations
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to app/_layout.tsx : Routing uses Expo Router v6 with typed routes (file-based routing in `app/`) and authentication flow enforced in root layout (`app/_layout.tsx`)
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use MSW (Mock Service Worker) for API mocking in tests
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to {src/app/layout.tsx,src/app/blog/[slug]/page.tsx} : Define SEO metadata in src/app/layout.tsx for the site root, and generate dynamic metadata for blog posts in src/app/blog/[slug]/page.tsx including OpenGraph and Twitter card metadata
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Wrap test components with AuthContext, ThemeContext; mock Supabase client for database operations
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Update tests when modifying existing code; add new tests for new behavior
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to src/**/*.{ts,tsx} : Use vercel/analytics for user analytics tracking in the application
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Test behavior, not implementation details
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Add regression tests for bug fixes that would have caught the bug
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to tests/**/*.{ts,tsx} : Add comprehensive test coverage for components, hooks, utilities, and API routes in tests/ directory
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : All tests must use descriptive test names and be independent; group related tests with `describe()` blocks
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Clean up mocks in `beforeEach()` or `afterEach()` blocks in tests
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to jest.setup.ts : Initialize testing-library/jest-dom and mocks in jest.setup.ts for unit test setup
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Run `pnpm test` to verify all tests pass and coverage stays above 80% before committing
Applied to files:
__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/*.{ts,tsx} : Write tests for any new or modified code, including happy path, edge cases, and error handling
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to jest.setup.js : AsyncStorage and SecureStore must be mocked to prevent actual storage during tests
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to **/*.test.{ts,tsx} : Test mocks must include platform detection (defaults to iOS) and timezone handling
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use Jest with React Native Testing Library for testing; mock Supabase client for database operations
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use MSW (Mock Service Worker) for API mocking in tests
Applied to files:
__tests__/lib/analytics.test.ts__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to jest.setup.js : Supabase client must be mocked with chainable query builder in jest.setup.js
Applied to files:
__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Wrap test components with AuthContext, ThemeContext; mock Supabase client for database operations
Applied to files:
__tests__/lib/analytics.test.ts
📚 Learning: 2025-12-04T03:11:15.142Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-04T03:11:15.142Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use `renderWithProviders` helper function to wrap test components with required contexts
Applied to files:
__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T03:55:42.734Z
Learnt from: CR
Repo: VolvoxCommunity/Sobriety-Waypoint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T03:55:42.734Z
Learning: Applies to {lib/logger.ts,lib/sentry.ts,jest.setup.js} : Console.log/error/warn are only permitted in logger.ts, sentry.ts, and jest.setup.js
Applied to files:
__tests__/lib/analytics.web.test.ts
📚 Learning: 2025-12-02T00:07:55.345Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T00:07:55.345Z
Learning: Applies to {jest.config.ts,jest.setup.ts,tests/**/*.{ts,tsx}} : Use Jest with Testing Library for unit tests located in tests/ directory with configuration via jest.config.ts and setup via jest.setup.ts
Applied to files:
__tests__/lib/analytics.web.test.ts
🧬 Code graph analysis (1)
__tests__/lib/analytics.test.ts (2)
lib/analytics/index.ts (2)
__resetForTesting(265-271)initializeAnalytics(92-163)lib/analytics/platform.web.ts (1)
__resetForTesting(658-667)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build for Web
- GitHub Check: auto-label
- GitHub Check: Cursor Bugbot
- GitHub Check: claude-review
🔇 Additional comments (9)
__tests__/lib/analytics.test.ts (3)
13-13: LGTM! Test isolation helper imported correctly.The
__resetForTestingimport enables proper test isolation by resetting internal initialization state between test cases.
70-71: Excellent test isolation pattern.Calling
__resetForTesting()inbeforeEachensures each test starts with a clean initialization state, preventing test interdependencies.
103-150: Comprehensive coverage of initialization guard behaviors.These three test cases thoroughly validate critical aspects of the analytics initialization flow:
- Idempotency prevents redundant initializations
- Concurrency handling ensures thread-safe initialization
- Retry logic allows recovery from transient failures
The deferred promise pattern in the concurrency test is particularly effective for controlling async timing.
__tests__/lib/analytics.web.test.ts (6)
23-23: Proper test isolation setup for web analytics.The
__resetForTestingimport and usage ensures clean state between test runs, consistent with the pattern in the unified analytics tests.Also applies to: 66-67
178-269: Excellent coverage of web analytics initialization flows.These tests effectively validate platform-specific initialization behaviors:
- Idempotency verification with debug mode logging
- Concurrency safety with deferred promise pattern
- Graceful degradation on Firebase failures (lines 240-269) is particularly important for resilience
The graceful failure test correctly verifies that initialization is marked complete even when Firebase init fails, preventing retry loops.
447-486: LGTM! Provider configuration tests are well-structured.These tests validate the provider initialization flow and debug logging with appropriate metadata flags.
488-615: Comprehensive error handling and idempotency validation.These test suites thoroughly cover:
- Graceful error handling for all Firebase operations (both Error and non-Error exceptions)
- Prevention of multiple initialization attempts
- Proper reset behavior with
__resetForTestingExcellent defensive testing practices.
697-848: Excellent data integrity and privacy safeguards.These test suites validate critical functionality:
- User Properties Filtering: Correctly removes undefined values while preserving false booleans
- Parameter Sanitization: Redacts PII-prone keys, handles nested objects, filters reserved logger keys
The comprehensive coverage of edge cases (empty objects, all undefined, recursive sanitization) demonstrates strong attention to data quality and privacy concerns.
850-927: Comprehensive edge case and reset utility validation.These final test sections provide:
- Proper environment gating for
__resetForTesting(production safeguard)- Edge case handling: null params, empty strings, long names, special characters, circular references
The circular reference test (lines 919-925) correctly verifies graceful degradation (no crash). The actual sanitization behavior is tested implicitly through the no-throw assertion.
| describe('Analytics Not Initialized', () => { | ||
| beforeEach(() => { | ||
| __resetForTesting(); | ||
| jest.clearAllMocks(); | ||
| mockIsDebugMode.mockReturnValue(false); | ||
| }); | ||
|
|
||
| it('logs debug message when tracking event without initialization', () => { | ||
| mockIsDebugMode.mockReturnValue(true); | ||
|
|
||
| trackEventWeb('test_event', { param1: 'value1' }); | ||
|
|
||
| expect(mockLoggerDebug).toHaveBeenCalledWith( | ||
| expect.stringContaining('Event (not sent - no provider initialized)'), | ||
| expect.objectContaining({ | ||
| category: 'ANALYTICS', | ||
| event_params: expect.any(Object), | ||
| }) | ||
| ); | ||
| }); | ||
|
|
||
| it('logs debug message when setting user ID without initialization', () => { | ||
| mockIsDebugMode.mockReturnValue(true); | ||
|
|
||
| setUserIdWeb('user-123'); | ||
|
|
||
| // Wait for async hash operation | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => { | ||
| expect(mockLoggerDebug).toHaveBeenCalledWith( | ||
| expect.stringContaining('setUserId (not sent - Firebase not initialized)'), | ||
| expect.objectContaining({ category: 'ANALYTICS' }) | ||
| ); | ||
| resolve(undefined); | ||
| }, 10); | ||
| }); | ||
| }); | ||
|
|
||
| it('logs debug message when clearing user ID (null) without initialization', () => { | ||
| mockIsDebugMode.mockReturnValue(true); | ||
|
|
||
| setUserIdWeb(null); | ||
|
|
||
| // Wait for async hash operation (even for null, there's async processing) | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => { | ||
| expect(mockLoggerDebug).toHaveBeenCalledWith( | ||
| 'setUserId (not sent - Firebase not initialized): null', | ||
| expect.objectContaining({ category: 'ANALYTICS' }) | ||
| ); | ||
| resolve(undefined); | ||
| }, 10); | ||
| }); | ||
| }); | ||
|
|
||
| it('logs debug message when setting user properties without initialization', () => { | ||
| mockIsDebugMode.mockReturnValue(true); | ||
|
|
||
| setUserPropertiesWeb({ theme_preference: 'dark' }); | ||
|
|
||
| expect(mockLoggerDebug).toHaveBeenCalledWith( | ||
| 'setUserProperties (not sent - Firebase not initialized)', | ||
| expect.objectContaining({ | ||
| category: 'ANALYTICS', | ||
| user_properties: expect.any(Object), | ||
| }) | ||
| ); | ||
| }); | ||
|
|
||
| it('does not call Firebase methods when not initialized', () => { | ||
| trackEventWeb('test_event'); | ||
| setUserIdWeb('user-123'); | ||
| setUserPropertiesWeb({ theme_preference: 'dark' }); | ||
|
|
||
| expect(logEvent).not.toHaveBeenCalled(); | ||
| expect(setUserId).not.toHaveBeenCalled(); | ||
| expect(setUserProperties).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Thorough coverage of uninitialized analytics behavior.
These tests validate that operations called without initialization properly:
- Log debug messages indicating no provider is initialized
- Avoid calling Firebase methods when not ready
- Handle both null and non-null userId cases
The async hash operation handling with setTimeout (lines 644-652, 661-669) works correctly. For future iterations, consider using async/await pattern: await new Promise((resolve) => setTimeout(resolve, 10)); for cleaner async handling, similar to the suggestion in past reviews.
🤖 Prompt for AI Agents
In __tests__/lib/analytics.web.test.ts around lines 644-652 and 661-669, replace
the manual Promise with setTimeout wrappers used to wait for the async hash
operation by converting those two test cases to async functions and awaiting a
small delay: use await new Promise(resolve => setTimeout(resolve, 10)); right
before the assertions so the tests still wait for the async work but use
async/await for clarity and consistency.
The WeakSet for circular reference detection was tracking all visited objects, which incorrectly marked shared (non-circular) objects as [Circular] when they appeared in multiple places. Fixed by: - Renaming 'seen' to 'ancestors' to clarify intent - Removing objects from the set after processing their subtree - This ensures only true circular references (ancestor in current path) are detected, not merely shared objects Added test case verifying shared objects are sanitized correctly without being marked as circular.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * 4. Handling circular references gracefully | ||
| * | ||
| * The `ancestors` WeakSet tracks objects in the current recursion path only. | ||
| * This correctly handles shared objects that appear in multiple non-circular positions |
There was a problem hiding this comment.
[nitpick] The documentation explains that the ancestors WeakSet tracks objects in the current recursion path, but it doesn't clarify that the WeakSet is passed down through recursive calls and modified (added to/deleted from) during traversal. Consider adding a note that the WeakSet is mutated during recursion to track the ancestry chain, then cleaned up after processing each nested object.
| * This correctly handles shared objects that appear in multiple non-circular positions | |
| * It is passed down through recursive calls and is mutated during traversal: | |
| * each object is added to the WeakSet before recursing into its children, and removed after processing. | |
| * This ensures correct detection of circular references, while allowing shared objects to appear in multiple non-circular positions |
| // Capture error but don't return - still initialize Vercel if enabled | ||
| firebaseError = error instanceof Error ? error : new Error(String(error)); |
There was a problem hiding this comment.
[nitpick] The variable firebaseError is assigned but only used in the debug logging block and the final debug log. If debug mode is disabled, this variable is assigned but never read. Consider moving the assignment inside the debug logging block at line 428-436 to avoid unnecessary work when debugging is disabled.
| // Reset state to allow retry | ||
| initializationState = 'failed'; | ||
| initializationPromise = null; | ||
| const err = error instanceof Error ? error : new Error(String(error)); | ||
| logger.error('Failed to initialize analytics platform', err, { | ||
| category: LogCategory.ANALYTICS, | ||
| }); | ||
| // Don't rethrow - analytics failures shouldn't crash the app |
There was a problem hiding this comment.
[nitpick] The error handling in the catch block logs the error but swallows it by not rethrowing. While the comment explains this is intentional, the Promise chain will resolve successfully even on failure. Consider documenting this behavior more explicitly in the function's JSDoc, noting that the returned Promise always resolves (never rejects) to prevent confusion for callers who might expect rejection on failure.
| <meta property="og:title" content="Sobriety Waypoint" /> | ||
| <meta property="og:description" content="Your companion on the journey to recovery" /> | ||
| <meta property="og:site_name" content="Sobriety Waypoint" /> | ||
| <meta property="og:image" content="https://sobrietywaypoint.com/assets/images/banner.png" /> |
There was a problem hiding this comment.
[nitpick] The Open Graph image URL is hardcoded as "https://sobrietywaypoint.com/assets/images/banner.png". Consider using an environment variable or configuration constant for the base URL to support different environments (development, staging, production) and make updates easier.
| <meta name="twitter:card" content="summary_large_image" /> | ||
| <meta name="twitter:title" content="Sobriety Waypoint" /> | ||
| <meta name="twitter:description" content="Your companion on the journey to recovery" /> | ||
| <meta name="twitter:image" content="https://sobrietywaypoint.com/assets/images/banner.png" /> |
There was a problem hiding this comment.
[nitpick] The Twitter Card image URL is hardcoded. This duplicates the same URL from the Open Graph image tag. Consider extracting this to a constant that can be reused for both meta tags to maintain consistency and simplify future updates.
Remove unused firstCallCount variable that was declared but not used after jest.clearAllMocks() reset the mock call counts.
1 participant
Summary
This PR merges the develop branch into main, bringing several features and improvements:
🔥 Firebase Analytics Improvements
platform.web.ts) and public API (index.ts) levels🌐 SEO & Social Sharing
🪟 Glass Effects (iOS)
expo-glass-effectfor liquid glass effects on iOSGlassViewcomponent with graceful fallback for non-iOS platforms🧪 Testing & CI
expo-router/headto support new Head import__resetForTesting()helper for proper test isolation🔧 Other Improvements
Test plan
Important
Enhances Firebase Analytics, adds SEO meta tags, introduces iOS glass effects, and improves testing and CI processes.
platform.web.tsandindex.ts.app/_layout.tsx.expo-glass-effectfor iOS.GlassViewcomponent with fallback for non-iOS platforms.expo-router/headinjest.config.js.expo-routercompatibility.__resetForTesting()for test isolation.analytics.test.tsandanalytics.web.test.ts.withFirebaseConfig.js.This description was created by
for e958a54. You can customize this summary. It will automatically update as commits are pushed.