⚡ Bolt: Fix search debounce to prevent redundant URL updates and renders#156
⚡ Bolt: Fix search debounce to prevent redundant URL updates and renders#156anyulled wants to merge 1 commit into
Conversation
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. |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoFix search debounce anti-pattern in TalksFilterBar component
WalkthroughsDescription• Fixed search debounce implementation in TalksFilterBar.tsx using useRef • Prevents redundant URL updates and re-renders on every keystroke • Properly clears timeout before setting new one in event handler • Documented anti-pattern and solution in project learning notes Diagramflowchart LR
A["Event Handler<br/>handleSearchChange"] -->|"Previously: returned<br/>cleanup function"| B["❌ Ignored by React<br/>No debounce effect"]
A -->|"Now: stores timeout<br/>in useRef"| C["✅ Proper debounce<br/>Single URL update"]
D["useEffect cleanup"] -->|"Clears timeout<br/>on unmount"| C
File Changes1. .jules/bolt.md
|
📝 WalkthroughWalkthroughFixed a React debouncing anti-pattern in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
Code Review by Qodo
|
![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 fixes a broken debounce implementation in the TalksFilterBar component by replacing an invalid cleanup return within an event handler with a useRef and clearTimeout approach. It also adds a learning entry to the project documentation regarding this React anti-pattern. While the fix correctly addresses the immediate bug where debouncing failed entirely, the review identifies a race condition where the debounced search update can overwrite immediate track filter changes. Additionally, feedback suggests improvements to prevent the URL synchronization logic from overwriting the user's input during active typing and recommends expanding the documentation to include these race condition scenarios.
| searchTimeoutRef.current = setTimeout(() => { | ||
| updateFilters(selectedTrack, newQuery); | ||
| }, 300); |
There was a problem hiding this comment.
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.
| ## 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. |
There was a problem hiding this comment.
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.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| if (searchTimeoutRef.current) { | ||
| clearTimeout(searchTimeoutRef.current); | ||
| } | ||
|
|
||
| // Debounce the URL update for search | ||
| const timeoutId = setTimeout(() => { | ||
| searchTimeoutRef.current = setTimeout(() => { | ||
| updateFilters(selectedTrack, newQuery); | ||
| }, 300); |
There was a problem hiding this comment.
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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 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.
🪄 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: 46261599-d28e-4367-a60c-ec3f5aea9d1c
📒 Files selected for processing (2)
.jules/bolt.mdcomponents/layout/TalksFilterBar.tsx
| searchTimeoutRef.current = setTimeout(() => { | ||
| updateFilters(selectedTrack, newQuery); | ||
| }, 300); |
There was a problem hiding this comment.
🧩 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.tsxRepository: anyulled/devbcn-nextjs
Length of output: 93
🏁 Script executed:
cat -n components/layout/TalksFilterBar.tsx | head -100Repository: 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 -20Repository: 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.
1 participant
💡 What: Refactored the search input debouncing in
TalksFilterBar.tsxto use auseRefto store and clear the timeout ID.🎯 Why: The previous implementation incorrectly returned a cleanup function directly from the
onChangeevent handler. React does not execute cleanup functions returned from event handlers (unlikeuseEffect). This resulted in the timeout never being cleared, meaning every single keystroke triggered a state update and URL parameter change after 300ms, defeating the purpose of the debounce and causing excessive re-renders and history stack bloat.📊 Impact: Reduces redundant React re-renders and Next.js router navigations (
router.push) by ~90% when a user types a multi-character search query. For a 10-character search, it now fires 1 update instead of 10.🔬 Measurement: Type rapidly in the search bar on the Talks page. Observe the Network tab or React Profiler; URL updates and re-renders will now only occur once the user stops typing for 300ms, rather than on every keystroke.
PR created automatically by Jules for task 6944799865442708773 started by @anyulled
Summary by CodeRabbit
Bug Fixes
Documentation