Skip to content

⚡ Bolt: [performance improvement] fix TalksFilterBar debounce #143

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

Closed
anyulled wants to merge 1 commit into from
Closed

⚡ Bolt: [performance improvement] fix TalksFilterBar debounce #143

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
12 changes: 9 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";
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

medium

The useRef hook is no longer necessary if the debounce logic is moved to a useEffect hook as suggested below to handle the side effect more robustly.

Suggested change
import { useCallback, useEffect, useState, useTransition, useRef } from "react";
import { useCallback, useEffect, useState, useTransition } from "react";

import { motion } from "framer-motion";

interface TalksFilterBarProps {
Expand All @@ -22,6 +22,8 @@ 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 timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);

Comment on lines +25 to +26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -name "TalksFilterBar.tsx" -type f

Repository: anyulled/devbcn-nextjs

Length of output: 104

🏁 Script executed:

cat -n components/layout/TalksFilterBar.tsx

Repository: anyulled/devbcn-nextjs

Length of output: 6629

Add unmount cleanup for the debounced timer.

timeoutRef is never cleared when the component unmounts, so the 300ms debounce callback can still execute navigation logic after the component tears down. This should be cleaned up with a useEffect:

Proposed fix 
   const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);

+  useEffect(() => {
+    return () => {
+      if (timeoutRef.current) {
+        clearTimeout(timeoutRef.current);
+        timeoutRef.current = null;
+      }
+    };
+  }, []);
+
   // Update state when URL changes
   useEffect(() => {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/layout/TalksFilterBar.tsx` around lines 25 - 26, The component's
timeoutRef (const timeoutRef = useRef<ReturnType<typeof setTimeout> |
null>(null)) is not cleared on unmount, allowing the debounced navigation
callback to run after teardown; add a useEffect in TalksFilterBar that returns a
cleanup function which clears the pending timeout (if timeoutRef.current) via
clearTimeout and sets timeoutRef.current = null to prevent the handler from
firing after unmount. Ensure this cleanup runs only on unmount (empty dependency
array) and references timeoutRef exactly as declared.

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

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

// Debounce the URL update for search
const timeoutId = setTimeout(() => {
timeoutRef.current = setTimeout(() => {
updateFilters(selectedTrack, newQuery);
Comment on lines +71 to 72
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot Apr 6, 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

The debounced callback in handleSearchChange calls updateFilters(selectedTrack, newQuery) with
selectedTrack captured at the time of typing. If the user clicks a track within the 300ms window,
the pending timeout can later push a URL with the old track, undoing the user’s track selection.
Agent Prompt 
### Issue description
A pending debounced search timer can push a URL containing an out-of-date `selectedTrack` if the user changes tracks before the timer fires.

### Issue Context
`handleSearchChange` schedules `updateFilters(selectedTrack, newQuery)` after 300ms, but `handleTrackClick` does not clear any pending timeout. This allows the older scheduled callback to run later and overwrite the newly-selected track in the URL.

### Fix Focus Areas
- components/layout/TalksFilterBar.tsx[56-74]

### Suggested changes
- Clear any pending `timeoutRef.current` at the start of `handleTrackClick`.
- Optionally also keep `selectedTrack` in a ref (updated via `useEffect`) and read from that ref inside the timeout callback, so the callback always uses the latest track.
- (Optional) Clear pending timeout when `searchParams` changes, to avoid applying a debounce based on an obsolete URL snapshot.

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

}, 300);
Comment on lines +66 to 73
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 6, 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-callback risk patterns in the current file.
rg -n -C3 'const handleTrackClick|setTimeout\(|updateFilters\(selectedTrack,\s*newQuery\)|clearTimeout\(timeoutRef\.current\)' components/layout/TalksFilterBar.tsx

Repository: anyulled/devbcn-nextjs

Length of output: 608

🏁 Script executed:

cat -n components/layout/TalksFilterBar.tsx

Repository: anyulled/devbcn-nextjs

Length of output: 6629

Clear pending search timeout when track selection changes.

When a user types then clicks a track before the 300ms debounce fires, the pending callback still executes with the captured stale selectedTrack value, overwriting the fresh track selection in the URL. Add timeout cleanup to handleTrackClick to prevent this race condition.

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

   const handleSearchChange = (e: React.ChangeEvent<HTMLInputElement>) => {
     const newQuery = e.target.value;
     setSearchQuery(newQuery);

     if (timeoutRef.current) {
       clearTimeout(timeoutRef.current);
+      timeoutRef.current = null;
     }

     // Debounce the URL update for search
     timeoutRef.current = setTimeout(() => {
       updateFilters(selectedTrack, newQuery);
+      timeoutRef.current = null;
     }, 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 66 - 73, The pending
search debounce can fire with a stale selectedTrack and overwrite a fresh track
selection; modify handleTrackClick to clear any pending timeoutRef before
updating selectedTrack and calling updateFilters: call
clearTimeout(timeoutRef.current) (and set timeoutRef.current = undefined/null)
at the start of handleTrackClick so the existing debounce callback that captures
the old selectedTrack is canceled before you update selection and call
updateFilters.

return () => clearTimeout(timeoutId);
};
Comment on lines 62 to 74
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

high

The current implementation using setTimeout inside the event handler is susceptible to stale closures. For example, if a user types a search query and then quickly selects a track pill before the 300ms timeout completes, the pending search update will execute with the old track value, effectively reverting the user's track selection in the URL. Additionally, this implementation lacks a cleanup mechanism for when the component unmounts.

A more robust approach is to use a useEffect hook to synchronize the searchQuery state with the URL. This ensures the latest state is always used, handles cleanup automatically, and prevents race conditions between search and track filtering.

Suggested change
const handleSearchChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const newQuery = e.target.value;
setSearchQuery(newQuery);
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
}
// Debounce the URL update for search
const timeoutId = setTimeout(() => {
timeoutRef.current = setTimeout(() => {
updateFilters(selectedTrack, newQuery);
}, 300);
return () => clearTimeout(timeoutId);
};
const handleSearchChange = (e: React.ChangeEvent<HTMLInputElement>) => {
setSearchQuery(e.target.value);
};
useEffect(() => {
const currentQueryInUrl = searchParams.get("q") || "";
if (searchQuery === currentQueryInUrl) return;
const timer = setTimeout(() => {
updateFilters(selectedTrack, searchQuery);
}, 300);
return () => clearTimeout(timer);
}, [searchQuery, selectedTrack, updateFilters, searchParams]);


return (
Expand Down
Loading