From 38deb067f3884b0b1dcba5a3f14d57b842e2f7f2 Mon Sep 17 00:00:00 2001 From: Kiran Madhav Date: Sun, 12 Jul 2026 22:55:54 +0530 Subject: [PATCH 1/2] fix(compare): prevent stalled comparison requests --- app/compare/CompareClient.test.tsx | 128 ++++++++++++++++-- app/compare/CompareClient.tsx | 63 +++++++-- components/DeveloperArena.tsx | 6 +- ...valriesTicker.mouse-interactivity.test.tsx | 2 +- components/TopRivalriesTicker.tsx | 16 ++- 5 files changed, 190 insertions(+), 25 deletions(-) diff --git a/app/compare/CompareClient.test.tsx b/app/compare/CompareClient.test.tsx index acb9a9a88..26ac76b08 100644 --- a/app/compare/CompareClient.test.tsx +++ b/app/compare/CompareClient.test.tsx @@ -1,19 +1,20 @@ -import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom/vitest'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import CompareClient from './CompareClient'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import React, { type ReactNode } from 'react'; +import CompareClient from './CompareClient'; const replaceMock = vi.fn(); const mockSearchParams = { - get: vi.fn((key) => { + get: vi.fn((key: string) => { if (key === 'user1') return 'userA'; if (key === 'user2') return 'userB'; return null; }), toString: vi.fn(() => 'user1=userA&user2=userB'), }; + vi.mock('next/navigation', () => ({ useRouter: () => ({ replace: replaceMock, @@ -108,6 +109,7 @@ describe('CompareClient', () => { localStorage.clear(); const maybeCaches = (global as unknown as { caches?: CacheStorage }).caches; + if (maybeCaches && typeof maybeCaches.delete === 'function') { await maybeCaches.delete('commitpulse-compare'); } @@ -121,6 +123,10 @@ describe('CompareClient', () => { ); }); + afterEach(() => { + vi.useRealTimers(); + }); + it('renders comparison page', () => { render(); @@ -166,8 +172,13 @@ describe('CompareClient', () => { expect(screen.getByText(/stats showdown/i)).toBeInTheDocument(); }); - await waitFor(() => expect(screen.getByText(/5[,\s ]?000/)).toBeInTheDocument()); - await waitFor(() => expect(screen.getByText(/3[,\s ]?000/)).toBeInTheDocument()); + await waitFor(() => { + expect(screen.getByText(/5[,\s\u00a0]?000/)).toBeInTheDocument(); + }); + + await waitFor(() => { + expect(screen.getByText(/3[,\s\u00a0]?000/)).toBeInTheDocument(); + }); }); it('updates route when compare button is clicked', async () => { @@ -192,12 +203,13 @@ describe('CompareClient', () => { it('shows error message when api request fails', async () => { localStorage.clear(); - // Also clear the Cache API to avoid previously cached successful responses - // from other tests bypassing the network error path. + const maybeCaches = (global as unknown as { caches?: CacheStorage }).caches; + if (maybeCaches && typeof maybeCaches.delete === 'function') { await maybeCaches.delete('commitpulse-compare'); } + global.fetch = vi.fn( async () => ({ @@ -222,4 +234,104 @@ describe('CompareClient', () => { expect(await screen.findByText(/failed to fetch comparison data/i)).toBeInTheDocument(); }); + + it('stops loading and shows an error when the comparison request times out', async () => { + vi.useFakeTimers(); + + global.fetch = vi.fn( + (_input: RequestInfo | URL, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + + if (!signal) { + reject(new Error('Expected the comparison request to include an AbortSignal.')); + return; + } + + if (signal.aborted) { + reject(new DOMException('Request aborted', 'AbortError')); + return; + } + + signal.addEventListener( + 'abort', + () => { + reject(new DOMException('Request aborted', 'AbortError')); + }, + { once: true } + ); + }) + ); + + render(); + + // Allow the initial URL-driven comparison effect and cache lookup to finish. + await act(async () => { + for (let i = 0; i < 5; i += 1) { + await Promise.resolve(); + } + }); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(screen.getByText('Comparing...')).toBeInTheDocument(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(20_000); + }); + + expect(screen.getByText(/comparison timed out\. please try again\./i)).toBeInTheDocument(); + + expect( + screen.getByRole('button', { + name: /compare two github profiles/i, + }) + ).not.toBeDisabled(); + }); + + it('does not start a duplicate request while the same comparison is already loading', async () => { + let resolveRequest: ((response: Response) => void) | undefined; + + global.fetch = vi.fn( + () => + new Promise((resolve) => { + resolveRequest = resolve; + }) + ); + + render(); + + await waitFor(() => { + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + const firstUsernameInput = screen.getByPlaceholderText(/github username #1/i); + + // Enter can invoke handleCompare even while the submit button is disabled. + // These repeated requests must be ignored while the original one is active. + fireEvent.keyDown(firstUsernameInput, { + key: 'Enter', + code: 'Enter', + }); + + fireEvent.keyDown(firstUsernameInput, { + key: 'Enter', + code: 'Enter', + }); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(resolveRequest).toBeDefined(); + + await act(async () => { + resolveRequest?.({ + ok: true, + json: async () => mockResponse, + } as Response); + + await Promise.resolve(); + }); + + await waitFor(() => { + expect(screen.getByText(/stats showdown/i)).toBeInTheDocument(); + }); + }); }); diff --git a/app/compare/CompareClient.tsx b/app/compare/CompareClient.tsx index d377f199d..4906cec93 100644 --- a/app/compare/CompareClient.tsx +++ b/app/compare/CompareClient.tsx @@ -980,6 +980,10 @@ export default function CompareClient() { // and limits the history size automatically. const { searches, addSearch, clearSearches, removeSearch } = useRecentSearches(); const lastComparedRef = useRef({ user1: '', user2: '' }); + const activeRequestRef = useRef<{ + key: string; + controller: AbortController; + } | null>(null); const dataRef = useRef(data); useEffect(() => { dataRef.current = data; @@ -1086,7 +1090,7 @@ export default function CompareClient() { }; const handleCompare = useCallback( - async (u1: string, u2: string) => { + async (u1: string, u2: string): Promise => { const trimmedUser1 = u1.trim(); const trimmedUser2 = u2.trim(); @@ -1115,6 +1119,12 @@ export default function CompareClient() { return; } + const requestKey = `${trimmedUser1.toLowerCase()}|${trimmedUser2.toLowerCase()}`; + + if (activeRequestRef.current?.key === requestKey) { + return; + } + if ( lastComparedRef.current.user1.toLowerCase() === trimmedUser1.toLowerCase() && lastComparedRef.current.user2.toLowerCase() === trimmedUser2.toLowerCase() && @@ -1123,6 +1133,12 @@ export default function CompareClient() { return; } + activeRequestRef.current?.controller.abort(); + + const controller = new AbortController(); + const timeoutId = window.setTimeout(() => controller.abort(), 20_000); + + activeRequestRef.current = { key: requestKey, controller }; lastComparedRef.current = { user1: trimmedUser1, user2: trimmedUser2 }; setLoading(true); setData(null); @@ -1133,7 +1149,7 @@ export default function CompareClient() { ); try { - const cached = await readCompareCache(u1, u2); + const cached = await readCompareCache(trimmedUser1, trimmedUser2); if (cached) { setData(cached); @@ -1145,10 +1161,11 @@ export default function CompareClient() { } const res = await fetch( - `/api/compare?user1=${encodeURIComponent(trimmedUser1)}&user2=${encodeURIComponent(trimmedUser2)}` + `/api/compare?user1=${encodeURIComponent(trimmedUser1)}&user2=${encodeURIComponent(trimmedUser2)}`, + { signal: controller.signal } ); - const json = await res.json(); + const json = (await res.json()) as CompareResponse; if (!res.ok) { setError(json.error || 'Failed to fetch comparison data.'); @@ -1161,30 +1178,56 @@ export default function CompareClient() { // revisit previously compared developers. addSearch(`${trimmedUser1} vs ${trimmedUser2}`); - await writeCompareCache(u1, u2, json); + await writeCompareCache(trimmedUser1, trimmedUser2, json); setMonolithKey((k) => k + 1); - } catch { - setError('Network error. Please try again.'); + } catch (err: unknown) { + if (activeRequestRef.current?.key !== requestKey) { + return; + } + + if (err instanceof DOMException && err.name === 'AbortError') { + setError('Comparison timed out. Please try again.'); + } else { + setError('Network error. Please try again.'); + } } finally { - setLoading(false); + window.clearTimeout(timeoutId); + + if (activeRequestRef.current?.key === requestKey) { + activeRequestRef.current = null; + setLoading(false); + } } }, [router, addSearch] ); - // Auto-compare if URL has params on mount or param changes + // Auto-compare if URL has params on mount or param changes. useEffect(() => { const u1 = searchParams.get('user1'); const u2 = searchParams.get('user2'); + if (u1 && u2) { setUser1(u1); // eslint-disable-line react-hooks/set-state-in-effect -- syncing URL params to input state setUser2(u2); - handleCompare(u1, u2); + + const requestKey = `${u1.trim().toLowerCase()}|${u2.trim().toLowerCase()}`; + const lastRequestKey = `${lastComparedRef.current.user1.toLowerCase()}|${lastComparedRef.current.user2.toLowerCase()}`; + + if (requestKey !== lastRequestKey && activeRequestRef.current?.key !== requestKey) { + void handleCompare(u1, u2); + } } else { setData(null); } }, [searchParams, handleCompare]); + useEffect(() => { + return () => { + activeRequestRef.current?.controller.abort(); + }; + }, []); + const d1 = data?.user1; const d2 = data?.user2; diff --git a/components/DeveloperArena.tsx b/components/DeveloperArena.tsx index a100ccdf7..27726c4cb 100644 --- a/components/DeveloperArena.tsx +++ b/components/DeveloperArena.tsx @@ -38,10 +38,10 @@ const SHOWDOWNS = [ badgeColor: 'border-purple-500/30 text-purple-400 bg-purple-500/5', }, { - u1: 'vercel', - u2: 'netlify', + u1: 'rauchg', + u2: 'biilmann', label: 'Vercel vs Netlify', - desc: 'Platform Wars', + desc: 'Founder Showdown', badgeColor: 'border-emerald-500/30 text-emerald-400 bg-emerald-500/5', }, { diff --git a/components/TopRivalriesTicker.mouse-interactivity.test.tsx b/components/TopRivalriesTicker.mouse-interactivity.test.tsx index 61015f347..e96f01286 100644 --- a/components/TopRivalriesTicker.mouse-interactivity.test.tsx +++ b/components/TopRivalriesTicker.mouse-interactivity.test.tsx @@ -88,7 +88,7 @@ describe('TopRivalriesTicker Mouse Interactivity', () => { expect(mockPush).toHaveBeenNthCalledWith(1, '/compare?user1=torvalds&user2=gaearon'); - expect(mockPush).toHaveBeenNthCalledWith(2, '/compare?user1=vercel&user2=netlify'); + expect(mockPush).toHaveBeenNthCalledWith(2, '/compare?user1=rauchg&user2=biilmann'); }); it('renders empty state safely without triggering navigation actions', () => { diff --git a/components/TopRivalriesTicker.tsx b/components/TopRivalriesTicker.tsx index 051514588..b76aeee8c 100644 --- a/components/TopRivalriesTicker.tsx +++ b/components/TopRivalriesTicker.tsx @@ -14,7 +14,15 @@ const MOCK_RIVALRIES = [ }, { u1: 'rich-harris', u2: 'antfu', label: 'Svelte vs Nuxt', icon: Zap, color: 'text-yellow-400' }, { u1: 'shadcn', u2: 'pacocoursey', label: 'UI Masters', icon: Target, color: 'text-indigo-400' }, - { u1: 'vercel', u2: 'netlify', label: 'Platform Wars', icon: Trophy, color: 'text-emerald-500' }, + { + u1: 'rauchg', + u2: 'biilmann', + displayU1: 'Vercel', + displayU2: 'Netlify', + label: 'Platform Wars', + icon: Trophy, + color: 'text-emerald-500', + }, { u1: 'dhh', u2: 'taylorotwell', label: 'Ruby vs PHP', icon: Star, color: 'text-rose-500' }, { u1: 'jhasourav07', @@ -28,6 +36,8 @@ const MOCK_RIVALRIES = [ export interface RivalryItem { u1: string; u2: string; + displayU1?: string; + displayU2?: string; label: string; icon: React.ComponentType<{ size?: number; className?: string }>; color: string; @@ -85,13 +95,13 @@ export default function TopRivalriesTicker({ />
- {rivalry.u1} + {rivalry.displayU1 ?? rivalry.u1} VS - {rivalry.u2} + {rivalry.displayU2 ?? rivalry.u2}
From eed0587882c303fdb6e5be995bc84d190fa32da8 Mon Sep 17 00:00:00 2001 From: Kiran Madhav Date: Mon, 13 Jul 2026 01:31:35 +0530 Subject: [PATCH 2/2] fix(compare): align founder rivalry preset --- .../TopRivalriesTicker.accessibility.test.tsx | 2 +- ...valriesTicker.mouse-interactivity.test.tsx | 2 +- components/TopRivalriesTicker.tsx | 72 ++++++++++++------- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/components/TopRivalriesTicker.accessibility.test.tsx b/components/TopRivalriesTicker.accessibility.test.tsx index 857d52aee..3acfe4070 100644 --- a/components/TopRivalriesTicker.accessibility.test.tsx +++ b/components/TopRivalriesTicker.accessibility.test.tsx @@ -58,7 +58,7 @@ describe('components/TopRivalriesTicker Accessibility Standards & Screen Reader render(); expect(screen.getAllByText('VS').length).toBeGreaterThan(0); - expect(screen.getAllByText('Platform Wars').length).toBeGreaterThan(0); + expect(screen.getAllByText('Vercel & Netlify Founders').length).toBeGreaterThan(0); }); it('preserves logical content order for rivalry information', () => { diff --git a/components/TopRivalriesTicker.mouse-interactivity.test.tsx b/components/TopRivalriesTicker.mouse-interactivity.test.tsx index e96f01286..7277dec8c 100644 --- a/components/TopRivalriesTicker.mouse-interactivity.test.tsx +++ b/components/TopRivalriesTicker.mouse-interactivity.test.tsx @@ -73,7 +73,7 @@ describe('TopRivalriesTicker Mouse Interactivity', () => { render(); const kernelVsReact = screen.getAllByText('Kernel vs React')[0]; - const platformWars = screen.getAllByText('Platform Wars')[0]; + const platformWars = screen.getAllByText('Vercel & Netlify Founders')[0]; const firstContainer = kernelVsReact.closest('div.group'); const secondContainer = platformWars.closest('div.group'); diff --git a/components/TopRivalriesTicker.tsx b/components/TopRivalriesTicker.tsx index b76aeee8c..b3641ec98 100644 --- a/components/TopRivalriesTicker.tsx +++ b/components/TopRivalriesTicker.tsx @@ -1,7 +1,7 @@ 'use client'; import { motion } from 'framer-motion'; -import { Flame, Zap, Trophy, Target, Star, Swords } from 'lucide-react'; +import { Flame, Star, Swords, Target, Trophy, Zap } from 'lucide-react'; import { useRouter } from 'next/navigation'; const MOCK_RIVALRIES = [ @@ -12,18 +12,34 @@ const MOCK_RIVALRIES = [ icon: Flame, color: 'text-orange-500', }, - { u1: 'rich-harris', u2: 'antfu', label: 'Svelte vs Nuxt', icon: Zap, color: 'text-yellow-400' }, - { u1: 'shadcn', u2: 'pacocoursey', label: 'UI Masters', icon: Target, color: 'text-indigo-400' }, + { + u1: 'rich-harris', + u2: 'antfu', + label: 'Svelte vs Nuxt', + icon: Zap, + color: 'text-yellow-400', + }, + { + u1: 'shadcn', + u2: 'pacocoursey', + label: 'UI Masters', + icon: Target, + color: 'text-indigo-400', + }, { u1: 'rauchg', u2: 'biilmann', - displayU1: 'Vercel', - displayU2: 'Netlify', - label: 'Platform Wars', + label: 'Vercel & Netlify Founders', icon: Trophy, color: 'text-emerald-500', }, - { u1: 'dhh', u2: 'taylorotwell', label: 'Ruby vs PHP', icon: Star, color: 'text-rose-500' }, + { + u1: 'dhh', + u2: 'taylorotwell', + label: 'Ruby vs PHP', + icon: Star, + color: 'text-rose-500', + }, { u1: 'jhasourav07', u2: 'leerob', @@ -36,10 +52,11 @@ const MOCK_RIVALRIES = [ export interface RivalryItem { u1: string; u2: string; - displayU1?: string; - displayU2?: string; label: string; - icon: React.ComponentType<{ size?: number; className?: string }>; + icon: React.ComponentType<{ + size?: number; + className?: string; + }>; color: string; } @@ -53,19 +70,18 @@ export default function TopRivalriesTicker({ const router = useRouter(); const handleRivalryClick = (u1: string, u2: string) => { - // Navigate to comparison and reload data router.push(`/compare?user1=${encodeURIComponent(u1)}&user2=${encodeURIComponent(u2)}`); }; const items = rivalries || []; return ( -
- {/* Edge Gradients for smooth fade in/out */} -
-
+
+ {/* Edge gradients for smooth fade in/out */} +
+
- {/* Marquee Content */} + {/* Marquee content */} - {/* We map twice to create the infinite seamless loop effect */} {items.length === 0 ? ( -
+
No active rivalries
) : ( [...items, ...items].map((rivalry, idx) => { const Icon = rivalry.icon; + return (
handleRivalryClick(rivalry.u1, rivalry.u2)} - className="group flex items-center gap-3 px-6 cursor-pointer hover:bg-black/5 dark:hover:bg-white/5 rounded-full transition-colors mx-2 py-1.5" + className="group mx-2 flex cursor-pointer items-center gap-3 rounded-full px-6 py-1.5 transition-colors hover:bg-black/5 dark:hover:bg-white/5" > +
- - {rivalry.displayU1 ?? rivalry.u1} + + {rivalry.u1} - + + VS - - {rivalry.displayU2 ?? rivalry.u2} + + + {rivalry.u2}
- + + {rivalry.label}