Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds a telemetry privacy module and integrates telemetry-safe user identification into AuthContext, guards project persistence when userId is absent, memoizes project filtering in RadioToggle, makes small UI/content tweaks, and adds locale support to booking grouping. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthContext
participant TelemetryLib as telemetryPrivacy
participant Sentry
participant PostHog
Client->>AuthContext: init / login (userData)
AuthContext->>TelemetryLib: getTelemetryEmailMode()
TelemetryLib-->>AuthContext: mode ("none"|"hash")
alt mode == "hash"
AuthContext->>TelemetryLib: hashEmailForTelemetry(email)
TelemetryLib-->>AuthContext: hashedEmail or null
AuthContext->>Sentry: setUser({id, email: hashedEmail?, name, role})
AuthContext->>PostHog: identify(hashedEmail?, {name, role})
else mode == "none"
AuthContext->>Sentry: setUser({id, name, role})
AuthContext->>PostHog: identify(id, {name, role})
end
Note right of AuthContext: telemetry errors are caught and reported\nwithout blocking auth flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/home/RadioToggle.tsx (1)
48-60: ConsideruseMemoinstead ofuseCallbackfor filtered results.
useCallbackmemoizes the function reference, but every invocation still recomputes the filtered array. When other useEffect dependencies change (e.g.,selectedProject,onChange), the effect re-runs and callsgetFilteredProjects()again unnecessarily.Using
useMemowould memoize the computed result directly, avoiding redundant filtering.♻️ Proposed refactor using useMemo
- const getFilteredProjects = useCallback(() => { - const trimmedLower = searchQuery.trim().toLowerCase(); - - if (!trimmedLower) { - return projects; - } - - return projects.filter( - (project) => - project.name.toLowerCase().includes(trimmedLower) || - project.id.toLowerCase().includes(trimmedLower), - ); - }, [projects, searchQuery]); + const filteredProjects = useMemo(() => { + const trimmedLower = searchQuery.trim().toLowerCase(); + + if (!trimmedLower) { + return projects; + } + + return projects.filter( + (project) => + project.name.toLowerCase().includes(trimmedLower) || + project.id.toLowerCase().includes(trimmedLower), + ); + }, [projects, searchQuery]);Then update the import and useEffect accordingly:
-import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react";useEffect(() => { - const filteredProjects = getFilteredProjects(); setVisibleProjects(filteredProjects); // ... rest of effect - }, [projects, selectedProject, onChange, userId, getFilteredProjects, getInitialProject]); + }, [projects, selectedProject, onChange, userId, filteredProjects, getInitialProject]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/home/RadioToggle.tsx` around lines 48 - 60, The current getFilteredProjects function is memoized with useCallback but still recomputes the filtered array on every invocation; replace useCallback with useMemo to memoize the computed filteredProjects value (based on projects and searchQuery) and return that memoized array, then update any uses (e.g., useEffect that references getFilteredProjects(), selectedProject, onChange) to consume the memoized filteredProjects value instead of calling the function; update imports accordingly and ensure the dependency array for useMemo includes [projects, searchQuery].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/context/AuthContext.tsx`:
- Around line 66-86: The identifyTelemetryUser function can throw and it’s
currently awaited by init/login callers which can block setIsInitialized; modify
the code so telemetry identification cannot interrupt auth flow by either
wrapping calls to identifyTelemetryUser in a try/catch (in the init and login
call sites) or making identifyTelemetryUser itself swallow and log errors
internally (e.g., catch inside the useCallback and return). Specifically locate
the identifyTelemetryUser useCallback in AuthContext.tsx and update either the
function body to handle exceptions or change each await
identifyTelemetryUser(...) call to a fire-and-forget try { await
identifyTelemetryUser(...) } catch (e) { /* log */ } pattern so auth
initialization/login always proceeds.
In `@src/lib/bookingHelpers.ts`:
- Around line 155-158: The monthFormatter is hardcoded to "en-US" inside
groupBookingsByMonth, causing locale mismatch; update groupBookingsByMonth to
accept an optional locale parameter (e.g., locale?: string) or default to the
runtime locale (Intl.DateTimeFormat(undefined...)) and use that when
constructing monthFormatter instead of the fixed "en-US"; locate the
monthFormatter creation in groupBookingsByMonth and replace the literal with the
passed-in locale or undefined so month names follow the app/browser i18n
settings.
In `@src/lib/telemetryPrivacy.ts`:
- Around line 25-35: The client-exposed module telemetryPrivacy.ts must not
dynamically import Node's crypto; remove the fallback that calls createHash from
"crypto" and instead return null when WebCrypto (webCrypto?.subtle) is
unavailable. Update the function's return type to allow null (e.g., string |
null) and adjust any callers (e.g., AuthContext.tsx) to handle a null
fingerprint, keeping the existing toHex/webCrypto.subtle path intact and
deleting the createHash import/usage (search for createHash and the Node
fallback block).
---
Nitpick comments:
In `@src/components/home/RadioToggle.tsx`:
- Around line 48-60: The current getFilteredProjects function is memoized with
useCallback but still recomputes the filtered array on every invocation; replace
useCallback with useMemo to memoize the computed filteredProjects value (based
on projects and searchQuery) and return that memoized array, then update any
uses (e.g., useEffect that references getFilteredProjects(), selectedProject,
onChange) to consume the memoized filteredProjects value instead of calling the
function; update imports accordingly and ensure the dependency array for useMemo
includes [projects, searchQuery].
Summary by CodeRabbit
Bug Fixes
Style
Refactor
Chores
Tests