⚡ Bolt: Throttle scroll event listener in ChatContainer to prevent excessive re-renders#348
⚡ Bolt: Throttle scroll event listener in ChatContainer to prevent excessive re-renders#348daggerstuff wants to merge 5 commits intostagingfrom
Conversation
Co-authored-by: daggerstuff <261005129+daggerstuff@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 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. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideChatContainer’s scroll handler is now throttled via requestAnimationFrame and the scroll listener is registered as passive to reduce unnecessary re-renders and improve scroll performance. Sequence diagram for throttled scroll handling in ChatContainersequenceDiagram
actor User
participant Browser
participant ChatContainer
participant ScrollHandler
participant ReactState
User->>Browser: scrolls chat content
Browser->>ChatContainer: scroll event on container
ChatContainer->>ScrollHandler: handleScroll()
alt ticking is false
ScrollHandler->>ScrollHandler: set ticking = true
ScrollHandler->>Browser: window.requestAnimationFrame(callback)
Browser-->>ScrollHandler: RAF callback
ScrollHandler->>ChatContainer: read scrollTop, scrollHeight, clientHeight
ScrollHandler->>ScrollHandler: compute isNearBottom
ScrollHandler->>ReactState: setShowScrollButton(!isNearBottom)
ScrollHandler->>ScrollHandler: set ticking = false
else ticking is true
ScrollHandler-->>Browser: ignore event (no state update)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe change introduces memoization to optimize symptom matching result calculations in the SyntheticTherapyDemo component. Previously computed inline on each render, these results are now calculated once via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new rAF throttling improves scroll performance, but it introduces scheduled work that can outlive the effect lifecycle. Canceling the pending animation frame (or guarding with a disposed flag) in the cleanup would make this change safer and more robust.
Summary of changes
What changed
- Updated the chat scroll handler to be throttled via
requestAnimationFrameto reduce render frequency during rapid scrolling. - Registered the scroll listener with
{ passive: true }to improve scrolling responsiveness.
Where
src/components/chat/ChatContainer.tsx:useEffectthat attaches the scroll event listener now wrapssetShowScrollButton(...)in an rAF “ticking” gate.
| // ⚡ Bolt: Throttled scroll event listener using requestAnimationFrame to prevent unnecessary frequent re-renders during scrolling without external dependencies. | ||
| let ticking = false | ||
| const handleScroll = () => { | ||
| const { scrollTop, scrollHeight, clientHeight } = container | ||
| const isNearBottom = scrollHeight - scrollTop - clientHeight < 100 | ||
| setShowScrollButton(!isNearBottom) | ||
| if (!ticking) { | ||
| window.requestAnimationFrame(() => { | ||
| if (!container) return | ||
| const { scrollTop, scrollHeight, clientHeight } = container | ||
| const isNearBottom = scrollHeight - scrollTop - clientHeight < 100 | ||
| setShowScrollButton(!isNearBottom) | ||
| ticking = false | ||
| }) | ||
| ticking = true | ||
| } | ||
| } | ||
|
|
||
| container.addEventListener('scroll', handleScroll) | ||
| container.addEventListener('scroll', handleScroll, { passive: true }) | ||
| return () => container.removeEventListener('scroll', handleScroll) | ||
| }, []) |
There was a problem hiding this comment.
The rAF callback can still run after the effect has been cleaned up (e.g., unmount/navigation) and call setShowScrollButton, which is an avoidable lifecycle hazard. Since you’re now explicitly scheduling async work, you should cancel the pending frame (or guard with a disposed flag) in the cleanup to prevent state updates after teardown.
Suggestion
Track and cancel the scheduled animation frame in the effect cleanup (or guard with a disposed flag). For example:
let rafId: number | null = null
let disposed = false
const handleScroll = () => {
if (rafId !== null) return
rafId = window.requestAnimationFrame(() => {
rafId = null
if (disposed) return
const { scrollTop, scrollHeight, clientHeight } = container
const isNearBottom = scrollHeight - scrollTop - clientHeight < 100
setShowScrollButton(!isNearBottom)
})
}
container.addEventListener('scroll', handleScroll, { passive: true })
return () => {
disposed = true
if (rafId !== null) window.cancelAnimationFrame(rafId)
container.removeEventListener('scroll', handleScroll)
}Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this change.
There was a problem hiding this comment.
Pull request overview
This PR improves chat scrolling performance by throttling the scroll event handler in ChatContainer using requestAnimationFrame, reducing the frequency of state updates during rapid scroll.
Changes:
- Throttles the scroll listener with a
ticking+requestAnimationFramepattern to limitsetShowScrollButtoncalls to once per frame. - Registers the scroll listener as
{ passive: true }to reduce main-thread blocking during scrolling.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| window.requestAnimationFrame(() => { | ||
| if (!container) return | ||
| const { scrollTop, scrollHeight, clientHeight } = container | ||
| const isNearBottom = scrollHeight - scrollTop - clientHeight < 100 | ||
| setShowScrollButton(!isNearBottom) | ||
| ticking = false | ||
| }) |
There was a problem hiding this comment.
The scheduled requestAnimationFrame callback isn’t cancelled on unmount. If a scroll event fires shortly before the component unmounts, the RAF callback can still run and call setShowScrollButton after unmount, causing React warnings and leaking work. Track the RAF id (or a disposed flag) and cancel/guard it in the effect cleanup; the if (!container) guard inside the RAF callback is ineffective because container is a captured constant.
| // ⚡ Bolt: Throttled scroll event listener using requestAnimationFrame to prevent unnecessary frequent re-renders during scrolling without external dependencies. | ||
| let ticking = false | ||
| const handleScroll = () => { | ||
| const { scrollTop, scrollHeight, clientHeight } = container | ||
| const isNearBottom = scrollHeight - scrollTop - clientHeight < 100 | ||
| setShowScrollButton(!isNearBottom) | ||
| if (!ticking) { | ||
| window.requestAnimationFrame(() => { | ||
| if (!container) return | ||
| const { scrollTop, scrollHeight, clientHeight } = container | ||
| const isNearBottom = scrollHeight - scrollTop - clientHeight < 100 | ||
| setShowScrollButton(!isNearBottom) | ||
| ticking = false | ||
| }) | ||
| ticking = true | ||
| } |
There was a problem hiding this comment.
This throttling logic changes scroll-button visibility behavior but there’s no test coverage to ensure (1) showScrollButton toggles correctly on scroll and (2) pending RAF work doesn’t update state after cleanup. Consider adding a small ChatContainer test that fires a scroll event, advances RAF, and asserts expected button visibility/cleanup behavior.
![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 `@src/components/chat/ChatContainer.tsx`:
- Around line 58-64: The scroll handler queues work via
window.requestAnimationFrame which can run after the component unmounts and call
setShowScrollButton; modify the effect that defines requestAnimationFrame and
ticking (the scroll handler inside ChatContainer / useEffect) to capture the
returned frame id (e.g., rafId) when calling requestAnimationFrame and call
cancelAnimationFrame(rafId) in the cleanup/unmount path (in addition to removing
the scroll listener) so any pending animation frame is cancelled before
teardown.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebb13242-3371-4312-b4f0-58bb8f9aadd0
📒 Files selected for processing (1)
src/components/chat/ChatContainer.tsx
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: daggerstuff <261005129+daggerstuff@users.noreply.github.com>
Co-authored-by: daggerstuff <261005129+daggerstuff@users.noreply.github.com>
Co-authored-by: daggerstuff <261005129+daggerstuff@users.noreply.github.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/chat/ChatContainer.tsx">
<violation number="1" location="src/components/chat/ChatContainer.tsx:68">
P2: The scroll listener was changed to non-passive, which regresses the intended scroll performance optimization. Re-add `{ passive: true }` when attaching the handler.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| } | ||
|
|
||
| container.addEventListener('scroll', handleScroll) |
There was a problem hiding this comment.
P2: The scroll listener was changed to non-passive, which regresses the intended scroll performance optimization. Re-add { passive: true } when attaching the handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/chat/ChatContainer.tsx, line 68:
<comment>The scroll listener was changed to non-passive, which regresses the intended scroll performance optimization. Re-add `{ passive: true }` when attaching the handler.</comment>
<file context>
@@ -51,23 +51,25 @@ export function ChatContainer({
- container.addEventListener('scroll', handleScroll, { passive: true })
- return () => container.removeEventListener('scroll', handleScroll)
+ container.addEventListener('scroll', handleScroll)
+ return () => {
+ container.removeEventListener('scroll', handleScroll)
</file context>
| container.addEventListener('scroll', handleScroll) | |
| container.addEventListener('scroll', handleScroll, { passive: true }) |
Co-authored-by: daggerstuff <261005129+daggerstuff@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/ai/SyntheticTherapyDemo.tsx (1)
244-263: Extract a shared symptom matcher to avoid logic drift.Lines 247–260 repeat the same bidirectional
includeslogic in three places. Centralizing it will make this safer to maintain.♻️ Suggested refactor
- const { correctlyIdentified, missedSymptoms, incorrectlyIdentified } = useMemo(() => { + const { correctlyIdentified, missedSymptoms, incorrectlyIdentified } = useMemo(() => { if (!selectedConversation) return { correctlyIdentified: [], missedSymptoms: [], incorrectlyIdentified: [] } + const matchesSymptom = (encodedName: string, decodedName: string) => + decodedName.includes(encodedName) || encodedName.includes(decodedName) + const correctlyIdentified = selectedConversation.encodedSymptoms.filter((encoded) => selectedConversation.decodedSymptoms.some( - (decoded) => decoded.includes(encoded.name) || encoded.name.includes(decoded) + (decoded) => matchesSymptom(encoded.name, decoded) ) ) const missedSymptoms = selectedConversation.encodedSymptoms.filter((encoded) => !selectedConversation.decodedSymptoms.some( - (decoded) => decoded.includes(encoded.name) || encoded.name.includes(decoded) + (decoded) => matchesSymptom(encoded.name, decoded) ) ) const incorrectlyIdentified = selectedConversation.decodedSymptoms.filter((decoded) => !selectedConversation.encodedSymptoms.some( - (encoded) => encoded.name.includes(decoded) || decoded.includes(encoded.name) + (encoded) => matchesSymptom(encoded.name, decoded) ) ) return { correctlyIdentified, missedSymptoms, incorrectlyIdentified } }, [selectedConversation])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ai/SyntheticTherapyDemo.tsx` around lines 244 - 263, The three filters inside the useMemo (producing correctlyIdentified, missedSymptoms, incorrectlyIdentified) duplicate the same bidirectional includes logic; extract that into a shared predicate (e.g., matchesSymptom or symptomMatches) and call it from each filter to avoid logic drift. Update the useMemo to define the matcher once (taking encoded.name and decoded string) and then use it in the three array operations that populate correctlyIdentified, missedSymptoms, and incorrectlyIdentified, leaving all return values and names unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/ai/SyntheticTherapyDemo.tsx`:
- Around line 244-263: The three filters inside the useMemo (producing
correctlyIdentified, missedSymptoms, incorrectlyIdentified) duplicate the same
bidirectional includes logic; extract that into a shared predicate (e.g.,
matchesSymptom or symptomMatches) and call it from each filter to avoid logic
drift. Update the useMemo to define the matcher once (taking encoded.name and
decoded string) and then use it in the three array operations that populate
correctlyIdentified, missedSymptoms, and incorrectlyIdentified, leaving all
return values and names unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb4c0c01-3e46-4ab6-afcb-5aeb920758ec
📒 Files selected for processing (1)
src/components/ai/SyntheticTherapyDemo.tsx
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/chat/ChatContainer.tsx">
<violation number="1">
P1: The scroll handler is no longer throttled, so it sets React state on every scroll event and can cause excessive re-renders during fast scrolling.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
💡 What: Throttled scroll event listener in ChatContainer.tsx using custom requestAnimationFrame logic.
🎯 Why: To prevent unnecessary frequent re-renders during scrolling and reduce layout thrashing.
📊 Impact: Improves scroll performance and reduces CPU usage in long chat sessions.
🔬 Measurement: Scroll through a long chat and observe fewer React re-renders and smoother scrolling.
PR created automatically by Jules for task 10905856307350038655 started by @daggerstuff
Summary by Sourcery
Throttle chat scroll handling to reduce unnecessary re-renders and improve scroll performance.
Enhancements:
Summary by cubic
Throttles the
ChatContainerscroll handler with a 150mssetTimeoutand memoizes symptom matching inSyntheticTherapyDemoto reduce re-renders and smooth scrolling, lowering CPU.setTimeout-based scroll throttling and clear pending timeouts; memoize O(n*m) symptom filters to compute correctly/missed/incorrect lists once per conversation.Written for commit 597c98c. Summary will update on new commits.
Summary by CodeRabbit