Skip to content

⚡ Bolt: Fix search debounce to prevent redundant URL updates and renders #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anyulled wants to merge 1 commit into
base: main
Choose a base branch
from
Open

⚡ Bolt: Fix search debounce to prevent redundant URL updates and renders #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,8 @@

**Learning:** When attempting to optimize an O(N^2) array spread operation (`[...existing, talk]`) inside a grouping loop in `groupTalksByTrack`, the purely functional/immutable constraint specified by the team (and the lack of `Map.groupBy` support in Node 20.x Jest environments) means that we must fall back to immutable reductions.
**Action:** When constraints require strict immutability without mutation of objects, use `reduce` with object and array spreads (e.g., `{ ...acc, [key]: [...(acc[key] || []), item] }`) even if it introduces O(N^2) overhead for large arrays. Avoid using `push()` or modifying accumulators directly. Always run Prettier/formatting checks before merge to resolve CI failures.

## 2026-04-12 - React Event Handler Debouncing Anti-Pattern

**Learning:** Found a broken debounce implementation in `TalksFilterBar.tsx` where `setTimeout` was called inside an event handler (`handleSearchChange`), and a cleanup function was returned. React event handlers (unlike `useEffect`) do not consume returned cleanup functions. As a result, every keystroke queued a new timer and state update, completely failing to debounce and causing redundant renders/URL updates.
**Action:** Always use a `useRef` to store the timeout ID for debouncing inside event handlers, manually clear it before setting a new one, and clean it up in a `useEffect` on component unmount.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The action item should be expanded to mention clearing the timeout in other event handlers that perform immediate updates to the same state or URL (e.g., track selection) to prevent race conditions where a debounced update overwrites a more recent immediate update.

19 changes: 16 additions & 3 deletions components/layout/TalksFilterBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use client";

import { usePathname, useRouter, useSearchParams } from "next/navigation";
import { useCallback, useEffect, useState, useTransition } from "react";
import { useCallback, useEffect, useState, useTransition, useRef } from "react";
import { motion } from "framer-motion";

interface TalksFilterBarProps {
Expand All @@ -21,6 +21,15 @@ export default function TalksFilterBar({ tracks, year: _year }: TalksFilterBarPr

const [selectedTrack, setSelectedTrack] = useState<string>(searchParams.get("track") || "");
const [searchQuery, setSearchQuery] = useState<string>(searchParams.get("q") || "");
const searchTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);

useEffect(() => {
return () => {
if (searchTimeoutRef.current) {
clearTimeout(searchTimeoutRef.current);
}
};
}, []);

// Update state when URL changes
useEffect(() => {
Expand Down Expand Up @@ -60,11 +69,15 @@ export default function TalksFilterBar({ tracks, year: _year }: TalksFilterBarPr
const handleSearchChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const newQuery = e.target.value;
setSearchQuery(newQuery);

if (searchTimeoutRef.current) {
clearTimeout(searchTimeoutRef.current);
}

// Debounce the URL update for search
const timeoutId = setTimeout(() => {
searchTimeoutRef.current = setTimeout(() => {
updateFilters(selectedTrack, newQuery);
}, 300);
Comment on lines +78 to 80
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This debounced call to updateFilters captures selectedTrack from the render cycle when the user started typing. If the user clicks a track filter while the 300ms timer is running, handleTrackClick will update the URL immediately, but this timeout will eventually fire and overwrite the URL with the old track value (and the old searchParams state captured in the updateFilters closure).

Additionally, there is a significant UX issue: when updateFilters eventually updates the URL, the useEffect (at line 35) will sync the URL back to the searchQuery state. If the user is still typing, their latest characters will be overwritten by the older value from the URL (e.g., typing 'abc' rapidly might result in the input being reset to 'a' once the first debounce fires).

To fix the race condition, you should clear searchTimeoutRef.current inside handleTrackClick. To fix the input overwrite, consider using a ref to track the last pushed query and only sync from the URL if the value differs from that ref.

Comment on lines +73 to 80
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Stale track in debounce 🐞 Bug ≡ Correctness

A pending debounced search timer can call updateFilters with a stale selectedTrack if the user
clicks a track pill within the 300ms debounce window, reverting the URL (and rendered results) back
to the old track. This happens because handleTrackClick doesn’t cancel the pending search timeout
and the timeout callback closes over selectedTrack from the earlier render.
Agent Prompt 
### Issue description
A debounced search timer can still be pending when the user clicks a track. When it fires, it uses a stale `selectedTrack` from closure and can overwrite the newer track choice in the URL.

### Issue Context
The timeout is cleared only on subsequent search changes and on unmount; track clicks do not clear the pending timeout.

### Fix Focus Areas
- components/layout/TalksFilterBar.tsx[63-81]

### Suggested fix
- In `handleTrackClick`, clear `searchTimeoutRef.current` (and set it back to `null`) before calling `updateFilters`.
- Optionally, avoid stale closures by storing the latest `selectedTrack` in a ref (e.g., `selectedTrackRef.current = selectedTrack` in an effect) and using that ref inside the timeout callback.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +78 to 80
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify stale-closure pattern is present in debounce callback
rg -nP --type=tsx 'setTimeout\(\(\)\s*=>\s*\{\s*updateFilters\(selectedTrack,\s*newQuery\)' components/layout/TalksFilterBar.tsx

Repository: anyulled/devbcn-nextjs

Length of output: 93

🏁 Script executed:

cat -n components/layout/TalksFilterBar.tsx | head -100

Repository: anyulled/devbcn-nextjs

Length of output: 3819

🏁 Script executed:

# Check the rest of the file to understand full context around lines 78-80
tail -n +75 components/layout/TalksFilterBar.tsx | head -20

Repository: anyulled/devbcn-nextjs

Length of output: 714

Debounced callback captures stale selectedTrack and can revert a recent track selection.

On Line 79, the timeout callback uses the render-time selectedTrack. If the user types and then clicks a track before 300ms, this callback can push stale track params and overwrite the newer selection.

💡 Proposed fix 
   const [selectedTrack, setSelectedTrack] = useState<string>(searchParams.get("track") || "");
   const [searchQuery, setSearchQuery] = useState<string>(searchParams.get("q") || "");
   const searchTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const selectedTrackRef = useRef(selectedTrack);
+
+  useEffect(() => {
+    selectedTrackRef.current = selectedTrack;
+  }, [selectedTrack]);

   const handleTrackClick = (track: string) => {
     const newTrack = track === selectedTrack ? "" : track;
+    selectedTrackRef.current = newTrack;
+    if (searchTimeoutRef.current) {
+      clearTimeout(searchTimeoutRef.current);
+    }
     setSelectedTrack(newTrack);
     updateFilters(newTrack, searchQuery);
   };

   searchTimeoutRef.current = setTimeout(() => {
-    updateFilters(selectedTrack, newQuery);
+    updateFilters(selectedTrackRef.current, newQuery);
   }, 300);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/layout/TalksFilterBar.tsx` around lines 78 - 80, The debounced
setTimeout callback closes over the render-time selectedTrack and can overwrite
a newer selection; fix by reading the latest selectedTrack at callback time
instead of relying on the closed-over variable: add a selectedTrackRef (update
it whenever selectedTrack changes or inside the track click handler), and in the
timeout callback use selectedTrackRef.current when calling
updateFilters(newQuery) (or call updateFilters(selectedTrackRef.current,
newQuery)); keep using searchTimeoutRef.current for cancelation as before and
ensure the ref is kept in sync with the selectedTrack state in the handlers that
change it.

return () => clearTimeout(timeoutId);
};

return (
Expand Down
Loading