Merged
Conversation
…istence + async label workflow (#364)
…tics cleanup (#367) * perf(vercel): cut runtime costs via notification, blog cache, and analytics changes * perf(blog): remove server searchParams usage to preserve ISR
… to client-side (#370) * refactor(frontend): remove locale layout dynamic auth and move header auth client-side * fix(frontend): prevent stale auth responses in useAuth and remove redundant dashboard dynamic layout
…abs (#372) * refactor(frontend): remove locale layout dynamic auth and move header auth client-side * fix(frontend): prevent stale auth responses in useAuth and remove redundant dashboard dynamic layout * feat(frontend): sync auth state across tabs via BroadcastChannel
…tars cache (#371) * perf(quiz-flow): move quiz progress to client-side fetch, enable ISR for quizzes page - Move user progress fetch from SSR to client-side API (/api/quiz/progress) - Remove force-dynamic and getCurrentUser() from quizzes page - Add revalidate=300 for ISR caching - Use window.history.replaceState for tab URL sync (avoid Next.js navigation) - Add forceMount to TabsContent to prevent layout shift on tab switch - Fix nested <main> — use <section> inside DynamicGridBackground - Cache GitHub stars count in sessionStorage to avoid refetch + re-animation * perf: replace useRef with useState lazy initializer in GitHubStarButton Fixes React 19 react-hooks/refs ESLint error — useRef.current cannot be read during render. Uses useState(getStoredStars) to capture the sessionStorage value once on mount instead. * fix: stop star icon trembling on hover in GitHubStarButton
…zes page (#373) * perf(quiz-flow): move quiz progress to client-side fetch, enable ISR for quizzes page - Move user progress fetch from SSR to client-side API (/api/quiz/progress) - Remove force-dynamic and getCurrentUser() from quizzes page - Add revalidate=300 for ISR caching - Use window.history.replaceState for tab URL sync (avoid Next.js navigation) - Add forceMount to TabsContent to prevent layout shift on tab switch - Fix nested <main> — use <section> inside DynamicGridBackground - Cache GitHub stars count in sessionStorage to avoid refetch + re-animation * perf: replace useRef with useState lazy initializer in GitHubStarButton Fixes React 19 react-hooks/refs ESLint error — useRef.current cannot be read during render. Uses useState(getStoredStars) to capture the sessionStorage value once on mount instead. * fix: stop star icon trembling on hover in GitHubStarButton * fix: eliminate quiz timer flash on language switch Remove Suspense boundary (loading.tsx) that unmounted QuizContainer during locale navigation. Synchronous session restore via useReducer lazy initializer and correct timer initialization via useState lazy initializer prevent any visible state reset on language switch * fix: replace quiz card layout shift with skeleton grid during progress load
ViktorSvertoka
added
bug
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📝 WalkthroughWalkthroughThis release introduces client-side authentication with cross-tab synchronization via BroadcastChannel, refactors layout and header components to use a new AuthProvider and useAuth hook, replaces server-side dynamic rendering with static revalidation (ISR), adds a client-side quiz progress API endpoint, and optimizes various UI components for stability and performance. Changes
Sequence DiagramsequenceDiagram
actor User
participant Tab1 as Browser Tab 1
participant BC as BroadcastChannel
participant Tab2 as Browser Tab 2
participant API as /api/auth/me
User->>Tab1: Login
Tab1->>API: POST auth request
API-->>Tab1: { id, role }
Tab1->>Tab1: Update AuthContext
Tab1->>BC: broadcastAuthUpdated()
BC-->>Tab2: AUTH_UPDATED event
Tab2->>Tab2: Receive event via subscribeToAuthUpdates
Tab2->>Tab2: Call refresh()
Tab2->>API: Fetch /api/auth/me
API-->>Tab2: { id, role }
Tab2->>Tab2: Update AuthContext
Note over Tab1,Tab2: Both tabs now in sync
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
frontend/components/shared/GitHubStarButton.tsx (1)
73-75: Consider adding minimal error logging in the catch block.The empty catch block silently swallows sessionStorage errors. While preventing crashes from quota-exceeded is reasonable, consider at least a comment or dev-mode logging:
Suggested improvement
try { sessionStorage.setItem(STORAGE_KEY, String(finalCount)); - } catch {} + } catch { + // sessionStorage may fail (quota exceeded, private browsing, etc.) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/shared/GitHubStarButton.tsx` around lines 73 - 75, The empty catch after sessionStorage.setItem(STORAGE_KEY, String(finalCount)) in GitHubStarButton silently swallows errors; update the catch to emit a minimal non-throwing log (e.g., console.debug or console.warn) that includes STORAGE_KEY and the caught error, gated to dev mode if desired (check NODE_ENV or a isDevelopment flag) so production noise is avoided while keeping behavior unchanged.frontend/hooks/useQuizSession.ts (1)
51-55: Simplified session handling on fresh visits.The new logic clears any existing session on fresh visits (without reload or allow-restore flags). Note that calling
loadQuizSessionbeforeclearQuizSessionis slightly redundant since you could clear unconditionally. However, this approach is explicit about only clearing valid sessions.♻️ Optional simplification
// Fresh visit (no restore flag) — clear any stale session if (!isReload && !allowRestore) { - const saved = loadQuizSession(quizId); - if (saved) clearQuizSession(quizId); + clearQuizSession(quizId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/hooks/useQuizSession.ts` around lines 51 - 55, The current fresh-visit branch calls loadQuizSession(quizId) then clearQuizSession(quizId) only if a saved session exists; simplify by removing the redundant load and clear unconditionally when !isReload && !allowRestore. Update the block in useQuizSession to check only the flags (isReload and allowRestore) and call clearQuizSession(quizId) directly to remove stale sessions, keeping quizId, isReload, allowRestore, loadQuizSession and clearQuizSession references for clarity.frontend/hooks/useAuth.tsx (1)
96-107: Consider the implications of caching failed auth requests.When
fetchAuthDedupedthrows, the cache is set tonullwith the current timestamp. This means authentication failures are cached for the full TTL (60 seconds), causing users to appear logged out even if the failure was due to a transient network issue.This may be acceptable since forced refreshes (cross-tab broadcasts, explicit
refresh()calls) bypass the cache, and the focus event will eventually re-check. However, if network reliability is a concern, consider either:
- Not caching errors (remove lines 105-107)
- Using a shorter TTL for error cases
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/hooks/useAuth.tsx` around lines 96 - 107, The code currently caches failed auth responses by setting authUserCache = null and authUserCacheAt = Date.now() inside the catch of fetchAuthDeduped, which causes transient network errors to be cached for the full TTL; change this behavior by either removing the two assignments so errors are not cached, or set a shorter TTL when handling errors (e.g., set authUserCacheAt to Date.now() - (TTL - shortErrorTTL)), and ensure the rest of the logic that checks latestRequestIdRef.current vs requestId and calls setUser(null) remains unchanged; touch the catch block around fetchAuthDeduped in useAuth.tsx where authUserCache/authUserCacheAt/setUser are referenced, and keep refresh()/cross-tab focus behavior intact.frontend/components/quiz/QuizContainer.tsx (1)
187-225: Consider clearing restore permission flags after successful restoration.The lazy initializer reads
QUIZ_ALLOW_RESTORE_KEYand the reload key from sessionStorage to determine if restoration should occur, but these keys aren't cleared after successful state restoration. If the component unmounts and remounts (e.g., during navigation), the restoration logic will run again against potentially stale localStorage data.Consider clearing the flags after restoration to ensure the restoration is a one-time operation:
♻️ Proposed fix
const saved = loadQuizSession(id); if (!saved) return defaultState; + sessionStorage.removeItem(QUIZ_ALLOW_RESTORE_KEY); + sessionStorage.removeItem(reloadKey); + return { status: saved.status,Additionally, there's minor code duplication between the Date conversion logic here (lines 214-217) and the
RESTORE_SESSIONreducer case (lines 125-128). Consider extracting a helper function if this pattern grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/quiz/QuizContainer.tsx` around lines 187 - 225, The lazy initializer currently reads QUIZ_ALLOW_RESTORE_KEY and getQuizReloadKey(id) from sessionStorage and restores via loadQuizSession(id) but never clears those flags; update the initializer in QuizContainer (the useReducer lazy init) to, after a successful loadQuizSession(id) restoration, remove the QUIZ_ALLOW_RESTORE_KEY and the reloadKey from sessionStorage so restoration happens only once per explicit permission, and ensure you still return the restored state object (with dates converted). Also factor out the Date conversion logic used here and in the RESTORE_SESSION reducer case into a small helper (e.g., restoreDatesFromSavedSession) and call it from both places to remove duplication.frontend/components/header/AppChrome.tsx (1)
27-40: Consider removing the CartProvider key or documenting its necessity.The
key={cart:${userId ?? 'guest'}}pattern forces a remount of the entire CartProvider subtree when userId changes. However, CartProvider already handles this via useEffect with proper state management: it listens tocartOwnerIdchanges and rehydrates the cart throughsyncCartWithServer. The key-forced remount appears redundant with this design and causes unnecessary reinitializations. If remounting is intentional for some specific reason, add a comment explaining why the built-in useEffect logic is insufficient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/AppChrome.tsx` around lines 27 - 40, The CartProvider currently uses an explicit React key (`key={`cart:${userId ?? 'guest'}`}`) which forces a full remount on userId change even though CartProvider already responds to changes via its cartOwnerId prop and internal useEffect (e.g., syncCartWithServer); remove the key prop from the CartProvider in AppChrome (or, if a remount is required for a specific edge case, add a clear comment next to the CartProvider explaining why the forced remount is necessary and why cartOwnerId/useEffect alone are insufficient) so the component tree no longer reinitializes unnecessarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/api/auth/me/route.ts`:
- Around line 5-10: The GET handler currently returns role from the JWT session
(via getAuthSession) which can be stale; update GET to fetch the current user
record from the database using the session user id (e.g., use your user lookup
function or model such as getUserById / User.findById with session.id) and
derive the role from that DB record before building payload, and if the DB user
is missing return payload with null or an appropriate unauthenticated shape
instead of the token role; keep returning id from session only if you still need
it but prefer using the DB-sourced id/role for authorization state.
In `@frontend/app/api/quiz/progress/route.ts`:
- Around line 18-22: The dynamic key assignment to progressMap using quizId can
allow prototype pollution (e.g., "__proto__"); change progressMap from a plain
object literal to a safe map — either initialize it with Object.create(null) or
use a Map — and update usages accordingly (references to progressMap,
rawProgress, and quizId) so keys are stored/retrieved from the safe container
instead of mutating the default object prototype.
In `@frontend/components/quiz/QuizzesSection.tsx`:
- Around line 57-67: The fetch in the useEffect that calls '/api/quiz/progress'
does not include credentials and silently swallows errors; update the fetch call
used inside the useEffect to pass an options object with credentials: 'include'
and improve error handling by logging the failure (e.g., log the response status
when !res.ok and the caught error in the .catch) before calling
setProgressLoaded(true) and setProgressMap, referencing the existing useEffect,
fetch('/api/quiz/progress'), setProgressMap, and setProgressLoaded symbols to
locate and update the code.
In `@frontend/components/shared/GitHubStarButton.tsx`:
- Around line 23-25: The component is calling getStoredStars during useState
initialization (storedStars) which can differ between server and client and
cause hydration mismatches; fix by initializing
storedStars/displayCount/finalCount to a consistent SSR-safe value (e.g., null
or 0) and move the call to getStoredStars into a useEffect that runs on mount to
read sessionStorage and then call setDisplayCount and setFinalCount (and
optionally setStoredStars if you keep that state) so all sessionStorage reads
happen only on the client; update references to
storedStars/displayCount/finalCount accordingly in the GitHubStarButton
component.
In `@frontend/lib/auth-sync.ts`:
- Around line 15-18: The BroadcastChannel constructor can throw SecurityError in
some environments; wrap the construction in both broadcastAuthUpdated and
subscribeToAuthUpdates with try/catch: in broadcastAuthUpdated, try to create
the channel, post the AuthSyncMessage and close the channel inside the try and
silently ignore errors in the catch; in subscribeToAuthUpdates, try to create
the channel and set onmessage/onmessage handlers and return a teardown that
closes the channel, but if construction fails return a no-op unsubscribe
function from the catch block so callers can always call the returned
unsubscribe safely.
In `@frontend/lib/logout.ts`:
- Around line 6-12: The code currently calls broadcastAuthUpdated() immediately
after the fetch to '/api/auth/logout'; change this so we only broadcast when the
logout request succeeds: await the fetch call, check response.ok (or specific
2xx status) and only call broadcastAuthUpdated() on success; on non-ok or
network errors, do not broadcast and instead throw or log the error so callers
can handle failure. Wrap the fetch in try/catch (or check res.ok) and use the
symbols fetch('/api/auth/logout') and broadcastAuthUpdated to locate and update
the logic.
---
Nitpick comments:
In `@frontend/components/header/AppChrome.tsx`:
- Around line 27-40: The CartProvider currently uses an explicit React key
(`key={`cart:${userId ?? 'guest'}`}`) which forces a full remount on userId
change even though CartProvider already responds to changes via its cartOwnerId
prop and internal useEffect (e.g., syncCartWithServer); remove the key prop from
the CartProvider in AppChrome (or, if a remount is required for a specific edge
case, add a clear comment next to the CartProvider explaining why the forced
remount is necessary and why cartOwnerId/useEffect alone are insufficient) so
the component tree no longer reinitializes unnecessarily.
In `@frontend/components/quiz/QuizContainer.tsx`:
- Around line 187-225: The lazy initializer currently reads
QUIZ_ALLOW_RESTORE_KEY and getQuizReloadKey(id) from sessionStorage and restores
via loadQuizSession(id) but never clears those flags; update the initializer in
QuizContainer (the useReducer lazy init) to, after a successful
loadQuizSession(id) restoration, remove the QUIZ_ALLOW_RESTORE_KEY and the
reloadKey from sessionStorage so restoration happens only once per explicit
permission, and ensure you still return the restored state object (with dates
converted). Also factor out the Date conversion logic used here and in the
RESTORE_SESSION reducer case into a small helper (e.g.,
restoreDatesFromSavedSession) and call it from both places to remove
duplication.
In `@frontend/components/shared/GitHubStarButton.tsx`:
- Around line 73-75: The empty catch after sessionStorage.setItem(STORAGE_KEY,
String(finalCount)) in GitHubStarButton silently swallows errors; update the
catch to emit a minimal non-throwing log (e.g., console.debug or console.warn)
that includes STORAGE_KEY and the caught error, gated to dev mode if desired
(check NODE_ENV or a isDevelopment flag) so production noise is avoided while
keeping behavior unchanged.
In `@frontend/hooks/useAuth.tsx`:
- Around line 96-107: The code currently caches failed auth responses by setting
authUserCache = null and authUserCacheAt = Date.now() inside the catch of
fetchAuthDeduped, which causes transient network errors to be cached for the
full TTL; change this behavior by either removing the two assignments so errors
are not cached, or set a shorter TTL when handling errors (e.g., set
authUserCacheAt to Date.now() - (TTL - shortErrorTTL)), and ensure the rest of
the logic that checks latestRequestIdRef.current vs requestId and calls
setUser(null) remains unchanged; touch the catch block around fetchAuthDeduped
in useAuth.tsx where authUserCache/authUserCacheAt/setUser are referenced, and
keep refresh()/cross-tab focus behavior intact.
In `@frontend/hooks/useQuizSession.ts`:
- Around line 51-55: The current fresh-visit branch calls
loadQuizSession(quizId) then clearQuizSession(quizId) only if a saved session
exists; simplify by removing the redundant load and clear unconditionally when
!isReload && !allowRestore. Update the block in useQuizSession to check only the
flags (isReload and allowRestore) and call clearQuizSession(quizId) directly to
remove stale sessions, keeping quizId, isReload, allowRestore, loadQuizSession
and clearQuizSession references for clarity.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
CHANGELOG.mdfrontend/app/[locale]/layout.tsxfrontend/app/[locale]/quiz/[slug]/loading.tsxfrontend/app/[locale]/quizzes/page.tsxfrontend/app/api/auth/me/route.tsfrontend/app/api/quiz/progress/route.tsfrontend/components/auth/LoginForm.tsxfrontend/components/auth/SignupForm.tsxfrontend/components/header/AppChrome.tsxfrontend/components/header/MainSwitcher.tsxfrontend/components/q&a/AIWordHelper.tsxfrontend/components/quiz/CountdownTimer.tsxfrontend/components/quiz/QuizContainer.tsxfrontend/components/quiz/QuizzesSection.tsxfrontend/components/shared/GitHubStarButton.tsxfrontend/hooks/useAuth.tsxfrontend/hooks/useQuizSession.tsfrontend/lib/auth-sync.tsfrontend/lib/auth.tsfrontend/lib/logout.tsfrontend/package.jsonstudio/package.json
💤 Files with no reviewable changes (1)
- frontend/app/[locale]/quiz/[slug]/loading.tsx
Comment on lines
+5
to
+10
| import { getAuthSession } from '@/lib/auth'; | ||
|
|
||
| export async function GET() { | ||
| const user = await getCurrentUser(); | ||
| return NextResponse.json({ user }, { status: 200 }); | ||
| const session = await getAuthSession(); | ||
| const payload = session ? { id: session.id, role: session.role } : null; | ||
|
|
Contributor
There was a problem hiding this comment.
Avoid returning authorization role from stale token claims.
Line 8-Line 9 now return role directly from JWT session payload. If a user is downgraded/deleted in DB, this endpoint can continue exposing outdated auth state until token expiry.
✅ Safer approach (DB-backed role)
-import { getAuthSession } from '@/lib/auth';
+import { getCurrentUser } from '@/lib/auth';
export async function GET() {
- const session = await getAuthSession();
- const payload = session ? { id: session.id, role: session.role } : null;
+ const user = await getCurrentUser();
+ const payload = user ? { id: user.id, role: user.role } : null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/auth/me/route.ts` around lines 5 - 10, The GET handler
currently returns role from the JWT session (via getAuthSession) which can be
stale; update GET to fetch the current user record from the database using the
session user id (e.g., use your user lookup function or model such as
getUserById / User.findById with session.id) and derive the role from that DB
record before building payload, and if the DB user is missing return payload
with null or an appropriate unauthenticated shape instead of the token role;
keep returning id from session only if you still need it but prefer using the
DB-sourced id/role for authorization state.
Comment on lines
+18
to
+22
| const progressMap: Record<string, { bestScore: number; totalQuestions: number; attemptsCount: number }> = {}; | ||
|
|
||
| for (const [quizId, progress] of rawProgress) { | ||
| progressMap[quizId] = { | ||
| bestScore: progress.bestScore, |
Contributor
There was a problem hiding this comment.
Prevent prototype pollution in dynamic key mapping.
Line 18 and Line 21 build an object from dynamic quizId keys on {}. If a key like __proto__ is present, object prototype mutation becomes possible.
🔒 Proposed fix
- const progressMap: Record<string, { bestScore: number; totalQuestions: number; attemptsCount: number }> = {};
+ const progressMap: Record<string, { bestScore: number; totalQuestions: number; attemptsCount: number }> =
+ Object.create(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const progressMap: Record<string, { bestScore: number; totalQuestions: number; attemptsCount: number }> = {}; | |
| for (const [quizId, progress] of rawProgress) { | |
| progressMap[quizId] = { | |
| bestScore: progress.bestScore, | |
| const progressMap: Record<string, { bestScore: number; totalQuestions: number; attemptsCount: number }> = | |
| Object.create(null); | |
| for (const [quizId, progress] of rawProgress) { | |
| progressMap[quizId] = { | |
| bestScore: progress.bestScore, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/quiz/progress/route.ts` around lines 18 - 22, The dynamic
key assignment to progressMap using quizId can allow prototype pollution (e.g.,
"__proto__"); change progressMap from a plain object literal to a safe map —
either initialize it with Object.create(null) or use a Map — and update usages
accordingly (references to progressMap, rawProgress, and quizId) so keys are
stored/retrieved from the safe container instead of mutating the default object
prototype.
Comment on lines
+57
to
+67
| useEffect(() => { | ||
| fetch('/api/quiz/progress') | ||
| .then(res => res.ok ? res.json() : {}) | ||
| .then(data => { | ||
| setProgressMap(data); | ||
| setProgressLoaded(true); | ||
| }) | ||
| .catch(() => { | ||
| setProgressLoaded(true); | ||
| }); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the quiz progress API route
find . -type f -name "route.ts" | xargs grep -l "progress" 2>/dev/null | head -5Repository: DevLoversTeam/devlovers.net
Length of output: 112
🏁 Script executed:
# Look for any quiz-related API routes
fd -type f "route.ts" | xargs grep -l "quiz" 2>/dev/nullRepository: DevLoversTeam/devlovers.net
Length of output: 243
🏁 Script executed:
# Check the specific path for the API
fd -type f "progress" --path "*/api/*"Repository: DevLoversTeam/devlovers.net
Length of output: 241
🏁 Script executed:
cat -n ./frontend/app/api/quiz/progress/route.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1154
🏁 Script executed:
find ./frontend -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "getCurrentUser" | grep "lib/auth"Repository: DevLoversTeam/devlovers.net
Length of output: 122
🏁 Script executed:
cat -n ./frontend/lib/auth.ts 2>/dev/null || find ./frontend -type f -path "*lib/auth*" -name "*.ts"Repository: DevLoversTeam/devlovers.net
Length of output: 3935
Add explicit credentials for authenticated API request and improve error logging.
The fetch to /api/quiz/progress should explicitly include credentials: 'include' to ensure cookies are sent. While the auth cookie is configured with sameSite: 'lax' (which allows same-origin cookies in fetch requests), explicitly specifying credentials is considered best practice and makes the intent clear.
Additionally, the error handling silently swallows failures without logging, making debugging difficult if the request fails.
🔧 Suggested improvement
useEffect(() => {
- fetch('/api/quiz/progress')
- .then(res => res.ok ? res.json() : {})
+ fetch('/api/quiz/progress', { credentials: 'include' })
+ .then(res => {
+ if (!res.ok) {
+ console.warn('Failed to fetch quiz progress:', res.status);
+ return {};
+ }
+ return res.json();
+ })
.then(data => {
setProgressMap(data);
setProgressLoaded(true);
})
- .catch(() => {
+ .catch(err => {
+ console.error('Error fetching quiz progress:', err);
setProgressLoaded(true);
});
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| fetch('/api/quiz/progress') | |
| .then(res => res.ok ? res.json() : {}) | |
| .then(data => { | |
| setProgressMap(data); | |
| setProgressLoaded(true); | |
| }) | |
| .catch(() => { | |
| setProgressLoaded(true); | |
| }); | |
| }, []); | |
| useEffect(() => { | |
| fetch('/api/quiz/progress', { credentials: 'include' }) | |
| .then(res => { | |
| if (!res.ok) { | |
| console.warn('Failed to fetch quiz progress:', res.status); | |
| return {}; | |
| } | |
| return res.json(); | |
| }) | |
| .then(data => { | |
| setProgressMap(data); | |
| setProgressLoaded(true); | |
| }) | |
| .catch(err => { | |
| console.error('Error fetching quiz progress:', err); | |
| setProgressLoaded(true); | |
| }); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/quiz/QuizzesSection.tsx` around lines 57 - 67, The fetch
in the useEffect that calls '/api/quiz/progress' does not include credentials
and silently swallows errors; update the fetch call used inside the useEffect to
pass an options object with credentials: 'include' and improve error handling by
logging the failure (e.g., log the response status when !res.ok and the caught
error in the .catch) before calling setProgressLoaded(true) and setProgressMap,
referencing the existing useEffect, fetch('/api/quiz/progress'), setProgressMap,
and setProgressLoaded symbols to locate and update the code.
Comment on lines
+23
to
+25
| const [storedStars] = useState(getStoredStars); | ||
| const [displayCount, setDisplayCount] = useState(storedStars ?? 0); | ||
| const [finalCount, setFinalCount] = useState<number | null>(storedStars); |
Contributor
There was a problem hiding this comment.
Potential hydration mismatch with sessionStorage in state initializer.
Calling getStoredStars() in useState initialization causes different values on server (null) vs client (cached number), leading to a hydration mismatch. React 19 is stricter about this and will log warnings.
The standard pattern is to initialize with a consistent value and read sessionStorage in a useEffect:
Proposed fix
export function GitHubStarButton({ className = '' }: GitHubStarButtonProps) {
const t = useTranslations('aria');
- const [storedStars] = useState(getStoredStars);
- const [displayCount, setDisplayCount] = useState(storedStars ?? 0);
- const [finalCount, setFinalCount] = useState<number | null>(storedStars);
+ const [displayCount, setDisplayCount] = useState(0);
+ const [finalCount, setFinalCount] = useState<number | null>(null);
+ const [isInitialized, setIsInitialized] = useState(false);
const githubUrl = 'https://github.com/DevLoversTeam/devlovers.net';
+ // Read sessionStorage after hydration to avoid mismatch
+ useEffect(() => {
+ const stored = getStoredStars();
+ if (stored !== null) {
+ setDisplayCount(stored);
+ setFinalCount(stored);
+ }
+ setIsInitialized(true);
+ }, []);
+
useEffect(() => {
- if (storedStars !== null) return;
+ if (!isInitialized || finalCount !== null) return;
const fetchStars = async () => {
// ... fetch logic remains the same
};
fetchStars();
- }, []);
+ }, [isInitialized, finalCount]);
useEffect(() => {
- if (finalCount === null || storedStars !== null) return;
+ if (finalCount === null || !isInitialized) return;
+ // Skip animation if we loaded from cache (displayCount already equals finalCount)
+ if (displayCount === finalCount) return;
// ... animation logic remains the same
- }, [finalCount, storedStars]);
+ }, [finalCount, isInitialized, displayCount]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/shared/GitHubStarButton.tsx` around lines 23 - 25, The
component is calling getStoredStars during useState initialization (storedStars)
which can differ between server and client and cause hydration mismatches; fix
by initializing storedStars/displayCount/finalCount to a consistent SSR-safe
value (e.g., null or 0) and move the call to getStoredStars into a useEffect
that runs on mount to read sessionStorage and then call setDisplayCount and
setFinalCount (and optionally setStoredStars if you keep that state) so all
sessionStorage reads happen only on the client; update references to
storedStars/displayCount/finalCount accordingly in the GitHubStarButton
component.
Comment on lines
+15
to
+18
| const channel = new BroadcastChannel(AUTH_CHANNEL_NAME); | ||
| const message: AuthSyncMessage = { type: AUTH_UPDATED_EVENT }; | ||
| channel.postMessage(message); | ||
| channel.close(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "auth-sync.ts" -type fRepository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
cat -n ./frontend/lib/auth-sync.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1272
🌐 Web query:
BroadcastChannel constructor SecurityError restricted environment private browsing
💡 Result:
new BroadcastChannel(name) can fail (or appear to “not work”) in restricted privacy contexts because some browsers treat cross-context messaging as a form of state that could be used for tracking, and therefore isolate or gate it behind storage-access rules.
What’s happening
- Safari Private Browsing: Each tab is isolated from the others in a Private Browsing window, so same-origin tabs won’t see each other’s BroadcastChannel messages (even though the constructor may succeed). [1][2]
- “SecurityError: restricted environment” cases: In environments where the browser has not granted storage access (commonly third‑party iframes with blocked cookies/storage, and sometimes private/incognito policies), creating/using an unpartitioned BroadcastChannel may be blocked and surface as a
SecurityErrorDOMException. The HTML spec community has explicitly discussed disabling BroadcastChannel when storage access is denied to prevent “leaking” data from restricted contexts to normal tabs. [3] - Storage Access API path (where supported): Some browsers expose BroadcastChannel access via
document.requestStorageAccess({ BroadcastChannel: true })and thenhandle.BroadcastChannel(name). If access isn’t granted, that call throws/returns aSecurityError. [4]
Practical mitigation
- Wrap construction in
try/catchand provide a fallback transport (e.g.,postMessagevia a top-level window, Service Worker messaging, WebSocket, etc.). - If you’re in a third‑party/embed context and the browser supports it, request access first:
let bc;
try {
bc = new BroadcastChannel("my-channel");
} catch (e) {
// fallback
}
// If using Storage Access API (supported browsers only):
const handle = await document.requestStorageAccess({ BroadcastChannel: true });
bc = handle.BroadcastChannel("my-channel"); // may throw SecurityError if deniedSources: Apple Safari Private Browsing isolation [1], StackOverflow discussion pointing to Apple’s isolation behavior [2], WHATWG issue on disabling BroadcastChannel without storage access (incl. private browsing) [3], MDN StorageAccessHandle.BroadcastChannel() SecurityError behavior [4].
References: [1] [2] [3] [4]
Wrap BroadcastChannel construction with try/catch.
The constructor can throw SecurityError in restricted environments—including private browsing (Safari) and third-party iframes with blocked storage access—even when BroadcastChannel exists on window. Without error handling, this breaks auth synchronization flows.
Wrap construction at lines 15 and 26 in try/catch blocks. In the catch handler, either silently fail (for broadcastAuthUpdated) or return a no-op function (for subscribeToAuthUpdates).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/lib/auth-sync.ts` around lines 15 - 18, The BroadcastChannel
constructor can throw SecurityError in some environments; wrap the construction
in both broadcastAuthUpdated and subscribeToAuthUpdates with try/catch: in
broadcastAuthUpdated, try to create the channel, post the AuthSyncMessage and
close the channel inside the try and silently ignore errors in the catch; in
subscribeToAuthUpdates, try to create the channel and set onmessage/onmessage
handlers and return a teardown that closes the channel, but if construction
fails return a no-op unsubscribe function from the catch block so callers can
always call the returned unsubscribe safely.
Comment on lines
6
to
12
| await fetch('/api/auth/logout', { | ||
| method: 'POST', | ||
| credentials: 'same-origin', | ||
| }); | ||
|
|
||
| broadcastAuthUpdated(); | ||
| } |
Contributor
There was a problem hiding this comment.
Broadcast only after confirmed logout success.
Line 11 broadcasts auth updates even when /api/auth/logout returns an error. That can desynchronize tabs and UI state.
✅ Proposed fix
export async function logout() {
- await fetch('/api/auth/logout', {
+ const response = await fetch('/api/auth/logout', {
method: 'POST',
credentials: 'same-origin',
});
- broadcastAuthUpdated();
+ if (!response.ok) {
+ throw new Error('Logout failed');
+ }
+
+ broadcastAuthUpdated();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await fetch('/api/auth/logout', { | |
| method: 'POST', | |
| credentials: 'same-origin', | |
| }); | |
| broadcastAuthUpdated(); | |
| } | |
| const response = await fetch('/api/auth/logout', { | |
| method: 'POST', | |
| credentials: 'same-origin', | |
| }); | |
| if (!response.ok) { | |
| throw new Error('Logout failed'); | |
| } | |
| broadcastAuthUpdated(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/lib/logout.ts` around lines 6 - 12, The code currently calls
broadcastAuthUpdated() immediately after the fetch to '/api/auth/logout'; change
this so we only broadcast when the logout request succeeds: await the fetch
call, check response.ok (or specific 2xx status) and only call
broadcastAuthUpdated() on success; on non-ok or network errors, do not broadcast
and instead throw or log the error so callers can handle failure. Wrap the fetch
in try/catch (or check res.ok) and use the symbols fetch('/api/auth/logout') and
broadcastAuthUpdated to locate and update the logic.
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
3 participants
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 CodeRabbit
New Features
Bug Fixes
Performance