⚡ Bolt: [performance improvement] fix TalksFilterBar debounce#143
⚡ Bolt: [performance improvement] fix TalksFilterBar debounce#143
Conversation
Fixes a bug where the search input in TalksFilterBar was not being debounced at all. The `return () => clearTimeout(...)` inside the event handler is ignored by React. By utilizing a `useRef` to store the timeout ID, we can properly clear the pending timeout across renders. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughUpdated TalksFilterBar to manage debounced search timeouts using a persistent useRef instead of local state. Timeout cancellation is now explicitly handled through ref state clearing rather than effect cleanup patterns, changing internal timeout management logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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 docstrings
🧪 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 |
Review Summary by QodoFix TalksFilterBar search debounce with useRef timeout management
WalkthroughsDescription• Fixes non-functional debounce in TalksFilterBar search input • Uses useRef to properly store and clear timeout IDs • Eliminates unnecessary URL updates on every keystroke • Significantly reduces React re-renders and router transitions Diagramflowchart LR
A["User types in search"] -->|onChange event| B["handleSearchChange"]
B -->|clear previous timeout| C["timeoutRef.current"]
B -->|set new timeout| D["timeoutRef.current = setTimeout"]
D -->|after 300ms| E["updateFilters called"]
E -->|URL updated| F["Router transition"]
File Changes1. components/layout/TalksFilterBar.tsx
|
Code Review by Qodo
1. Stale track in debounce
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a debouncing mechanism for the search input using useRef. The review feedback suggests refactoring this logic into a useEffect hook to avoid stale closures, ensure proper cleanup on unmount, and more reliably synchronize the search query with the URL.
| 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); | ||
| }; |
There was a problem hiding this comment.
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.
| 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]); |
|
|
||
| import { usePathname, useRouter, useSearchParams } from "next/navigation"; | ||
| import { useCallback, useEffect, useState, useTransition } from "react"; | ||
| import { useCallback, useEffect, useState, useTransition, useRef } from "react"; |
There was a problem hiding this comment.
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.
| import { useCallback, useEffect, useState, useTransition, useRef } from "react"; | |
| import { useCallback, useEffect, useState, useTransition } from "react"; |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| timeoutRef.current = setTimeout(() => { | ||
| updateFilters(selectedTrack, newQuery); |
There was a problem hiding this comment.
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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/layout/TalksFilterBar.tsx`:
- Around line 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.
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 184b2b58-2240-4afa-a60e-ec62501dcd17
📒 Files selected for processing (1)
components/layout/TalksFilterBar.tsx
| const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "TalksFilterBar.tsx" -type fRepository: anyulled/devbcn-nextjs
Length of output: 104
🏁 Script executed:
cat -n components/layout/TalksFilterBar.tsxRepository: 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.
| if (timeoutRef.current) { | ||
| clearTimeout(timeoutRef.current); | ||
| } | ||
|
|
||
| // Debounce the URL update for search | ||
| const timeoutId = setTimeout(() => { | ||
| timeoutRef.current = setTimeout(() => { | ||
| updateFilters(selectedTrack, newQuery); | ||
| }, 300); |
There was a problem hiding this comment.
🧩 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.tsxRepository: anyulled/devbcn-nextjs
Length of output: 608
🏁 Script executed:
cat -n components/layout/TalksFilterBar.tsxRepository: 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.
1 participant
💡 What:
Fixes a non-functional debounce implementation in the
TalksFilterBarcomponent's search input. By using auseRefto store the timeout ID instead of attempting to return a cleanup function directly from theonChangeevent handler (which React does not execute), we can successfully cancel pending search updates.🎯 Why:
The previous code resulted in rapid keystrokes triggering a flood of state and URL updates. Since URL updates in Next.js trigger router transitions and layout re-evaluations, this caused massive performance overhead and degraded the typing experience.
📊 Impact:
Significantly reduces React re-renders and Next.js router transitions when a user searches for talks. By actually debouncing the keystrokes (300ms), the UI will remain highly responsive even when typing quickly.
🔬 Measurement:
Start typing continuously into the search input on the talks page. Before this change, the URL query parameters would update with every single letter typed. After this change, the URL will only update 300ms after you stop typing. You can also measure the reduction in React commit phases using the React Profiler.
PR created automatically by Jules for task 13049950721628530604 started by @anyulled
Summary by CodeRabbit