⚡ Bolt: Optimize chat performance via state colocation and memoization#523
⚡ Bolt: Optimize chat performance via state colocation and memoization#523ngoiyaeric wants to merge 1 commit intomainfrom
Conversation
- Moved high-frequency \`input\` state from \`Chat\` to \`ChatPanel\` to prevent whole-app re-renders on keystrokes. - Exposed \`submitForm\` via \`useImperativeHandle\` in \`ChatPanel\` for programmatic submissions. - Wrapped \`ChatMessages\` in \`React.memo\` and memoized message grouping logic. - Fixed a Rules of Hooks violation in \`ChatMessages\`. - Added performance learnings to \`.jules/bolt.md\`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughThe PR refactors chat component architecture to implement state colocation and memoization patterns. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Chat
participant ChatPanel
participant Form
rect rgba(100, 150, 200, 0.5)
Note over Chat,Form: Before: Parent-managed submission
User->>Chat: Select suggestion / Submit via EmptyScreen
Chat->>Chat: Update local input state
Chat->>Chat: Trigger isSubmitting state
Chat->>ChatPanel: Pass input via props
ChatPanel->>Form: Render input with external value
Form->>ChatPanel: Form submit
end
rect rgba(150, 200, 100, 0.5)
Note over Chat,Form: After: Child-managed submission via ref
User->>Chat: Select suggestion / Submit via EmptyScreen
Chat->>ChatPanel: Call submitForm(value) via ref
ChatPanel->>ChatPanel: Update internal input state
ChatPanel->>ChatPanel: Increment triggerSubmit
ChatPanel->>Form: Form submits via effect
Form->>ChatPanel: Submission handled
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
Main concern: ChatPanelRef.submitForm(value) relies on a triggerSubmit state + effect sequencing, which is brittle and can submit stale input depending on update timing. ChatMessages still uses any and multiple casts in core rendering logic, undermining type safety and increasing runtime risk. There are minor simplifications available around empty-state handling, and React.memo benefits depend on whether messages is referentially stable upstream.
Additional notes (2)
- Readability |
components/chat-messages.tsx:14-46
ChatMessagesnow checks!messages.lengthtwice: once insideuseMemo(returning[]) and again right after memoization (returningnull). The second early-return is fine, but theuseMemoconditional becomes redundant and slightly obscures intent.
You can simplify by always computing based on messages and letting the component return null when empty (or just always render and map over an empty array).
- Performance |
components/chat-messages.tsx:12-43
React.memowill only prevent re-renders if themessagesprop is referentially stable when content hasn’t changed. If upstream produces a newmessagesarray instance frequently (even with identical content),memowon’t help much, whileuseMemowill also recompute.
Given the PR goal is performance, it’s worth confirming how useUIState() updates messages and whether it preserves the same array reference when no changes occur.
Summary of changes
Summary
This PR focuses on reducing chat UI re-renders and expensive computations.
- State colocation: moved high-frequency
inputstate fromcomponents/chat.tsxintocomponents/chat-panel.tsxto prevent whole-page re-renders on each keystroke. - Imperative submission API: updated
ChatPanelRef.submitFormto accept an optional value (submitForm(value?: string)) and implemented it viauseImperativeHandle. - Memoization: wrapped
ChatMessagesinReact.memoand memoized message grouping usinguseMemo. - Hook correctness: ensured hook usage order in
ChatMessagesis compatible with the Rules of Hooks. - Docs: added a short performance note in
.jules/bolt.mdcapturing the learnings.
| const groupedMessagesArray = useMemo(() => { | ||
| if (!messages.length) { | ||
| return [] | ||
| } | ||
| const groupedMessages = messages.reduce( | ||
| (acc: { [key: string]: any }, message) => { | ||
| if (!acc[message.id]) { | ||
| acc[message.id] = { | ||
| id: message.id, | ||
| components: [], | ||
| isCollapsed: message.isCollapsed | ||
| } | ||
| } | ||
| } | ||
| acc[message.id].components.push(message.component) | ||
| return acc | ||
| }, | ||
| {} | ||
| ) | ||
| acc[message.id].components.push(message.component) | ||
| return acc | ||
| }, | ||
| {} | ||
| ) | ||
|
|
||
| // Convert grouped messages into an array with explicit type | ||
| const groupedMessagesArray = Object.values(groupedMessages).map(group => ({ | ||
| ...group, | ||
| components: group.components as React.ReactNode[] | ||
| })) as { | ||
| id: string | ||
| components: React.ReactNode[] | ||
| isCollapsed?: StreamableValue<boolean> | ||
| }[] | ||
| // Convert grouped messages into an array with explicit type | ||
| return Object.values(groupedMessages).map(group => ({ | ||
| ...group, | ||
| components: group.components as React.ReactNode[] | ||
| })) as { | ||
| id: string | ||
| components: React.ReactNode[] | ||
| isCollapsed?: StreamableValue<boolean> | ||
| }[] | ||
| }, [messages]) |
There was a problem hiding this comment.
The message grouping logic uses (acc: { [key: string]: any }, message) => ... and later uses as React.ReactNode[] casts. Even if tsc passes, any here defeats type safety and makes it easier to accidentally push non-renderables into components (which becomes a runtime rendering bug).
Since this is central rendering logic (and now memoized), it’s worth typing it properly to prevent subtle regressions.
Suggestion
Replace the any accumulator with an explicit type.
For example:
- Define a
type Group = { id: string; components: React.ReactNode[]; isCollapsed?: StreamableValue<boolean> }. - Type the reducer as
Record<string, Group>and initializecomponents: [] as React.ReactNode[]. - Then you can return
Object.values(groupedMessages)without theas ...[]cast.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this typing cleanup.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/chat-panel.tsx (1)
37-40: 🧹 Nitpick | 🔵 Trivial
setSuggestionsStateis a stable setState function — it doesn't need to be in the dependency array.React guarantees
setStateidentity is stable across renders, so includingsetSuggestionsStatein theuseCallbackdeps is harmless but unnecessary.
🤖 Fix all issues with AI agents
In @.jules/bolt.md:
- Line 1: Change the heading "## 2025-05-14 - [State Colocation & Memoization]"
to a top-level heading and ensure there is a blank line before it: replace the
leading "##" with "#" and insert one blank line above that heading in
.jules/bolt.md so the file starts with a level-1 heading and satisfies MD041 and
MD022.
In `@components/chat-messages.tsx`:
- Around line 12-42: The outer empty-check should use
groupedMessagesArray.length === 0 instead of re-reading messages.length and the
reduce accumulator should be given a concrete type to avoid the final "as" cast:
update the ChatMessages component to remove the redundant messages.length check
and change the accumulator type used in the reduce inside groupedMessagesArray
(the object built for message grouping) to a properly typed interface (e.g.,
mapping ids to { id: string; components: React.ReactNode[]; isCollapsed?:
StreamableValue<boolean> }) so the final Object.values(...).map no longer needs
an assertion.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.jules/bolt.mdcomponents/chat-messages.tsxcomponents/chat-panel.tsxcomponents/chat.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
components/chat.tsx (2)
components/empty-screen.tsx (1)
EmptyScreen(27-59)components/followup-panel.tsx (1)
FollowupPanel(13-67)
components/chat-messages.tsx (2)
app/actions.tsx (1)
UIState(567-572)components/collapsible-message.tsx (1)
data(25-90)
🪛 markdownlint-cli2 (0.20.0)
.jules/bolt.md
[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (7)
components/chat-panel.tsx (3)
47-65: ThesetInput+triggerSubmitpattern works but has a subtle rapid-call edge case.If
submitForm(value)is called twice in quick succession (before React flushes), the secondsetInputoverwrites the first, and the effect only fires once (or with the last value). This is likely acceptable for the current use cases (suggestion selection, empty screen), but worth documenting.Also,
useImperativeHandledoesn't listinputortriggerSubmitin a dependency array, so the exposedsubmitFormclosure always captures fresh setters — which is correct here since it only calls state setters.
28-30: State colocation looks correct — input is now local toChatPanel.Moving
inputandsetInputhere is the core performance win of this PR. ThetriggerSubmitcounter is a reasonable mechanism for bridging imperative calls to the React effect lifecycle.
177-197: Early return renders only a "New Chat" button whenmessages.length > 0 && !isMobile.This means on desktop with messages present, the entire input form is replaced by just a small button. This was likely the pre-existing behavior, but after this PR the input state lives inside
ChatPanel— if the component unmounts (early return) and remounts (messages cleared), the input state resets. This is probably the desired behavior, just confirming it's intentional.components/chat.tsx (3)
105-106: Ref-based submission pattern is clean and consistent.All submission paths (
SuggestionsDropdown.onSelect,EmptyScreen.submitMessage) now route throughchatPanelRef.current?.submitForm(value), which is the correct usage of the new API. The optional chaining guards against the ref being null before mount.
140-144:submitForm(message)will set input and trigger submit — verify the form payload includes the message text.When
submitForm(message)is called fromEmptyScreen, the flow is:setInput(message)→setTriggerSubmit(+1)→ effect firesrequestSubmit()→handleSubmitreadsinputfrom state. Since the<Textarea name="input">is a controlled component withvalue={input}, theFormDataconstructed frome.currentTargetshould include the message. This is correct.Also applies to: 178-181
84-84: No issue -useMapData()is properly wrapped by parentMapDataProvider.The
Chatcomponent is always rendered within aMapDataProvidercontext from parent pages (app/page.tsxandapp/search/[id]/page.tsx). WhenuseMapData()is called at line 84, it has access to the parent provider's context, so the design is safe and intentional. The internalMapDataProviderwrapping at lines 118/161 provides an additional context layer for descendant components within the chat layout.components/chat-messages.tsx (1)
12-12:React.memois appropriately configured withuseUIState; no action needed.
useUIStatefrom the Vercel AI SDK works like React'suseStateand returns stable references for unchanged state values. Themessagesprop will maintain the same reference across renders until the UI state actually updates—which is the correct and intended behavior. React.memo will effectively skip re-renders when themessagesprop hasn't changed, and the component's internaluseMemoproperly handles grouping logic with the[messages]dependency. No reference stability issues exist.Likely an incorrect or invalid review comment.
| @@ -0,0 +1,6 @@ | |||
| ## 2025-05-14 - [State Colocation & Memoization] | |||
There was a problem hiding this comment.
Fix markdown lint warnings: add a blank line before the heading and use a top-level # heading.
Static analysis flags MD022 (blank lines around headings) and MD041 (first line should be a top-level heading).
Proposed fix
-## 2025-05-14 - [State Colocation & Memoization]
+# Bolt Architecture Notes
+
+## 2025-05-14 - [State Colocation & Memoization]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 2025-05-14 - [State Colocation & Memoization] | |
| # Bolt Architecture Notes | |
| ## 2025-05-14 - [State Colocation & Memoization] |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In @.jules/bolt.md at line 1, Change the heading "## 2025-05-14 - [State
Colocation & Memoization]" to a top-level heading and ensure there is a blank
line before it: replace the leading "##" with "#" and insert one blank line
above that heading in .jules/bolt.md so the file starts with a level-1 heading
and satisfies MD041 and MD022.
| export const ChatMessages = React.memo(function ChatMessages({ messages }: ChatMessagesProps) { | ||
| // Group messages based on ID, and if there are multiple messages with the same ID, combine them into one message | ||
| const groupedMessages = messages.reduce( | ||
| (acc: { [key: string]: any }, message) => { | ||
| if (!acc[message.id]) { | ||
| acc[message.id] = { | ||
| id: message.id, | ||
| components: [], | ||
| isCollapsed: message.isCollapsed | ||
| const groupedMessagesArray = useMemo(() => { | ||
| if (!messages.length) { | ||
| return [] | ||
| } | ||
| const groupedMessages = messages.reduce( | ||
| (acc: { [key: string]: any }, message) => { | ||
| if (!acc[message.id]) { | ||
| acc[message.id] = { | ||
| id: message.id, | ||
| components: [], | ||
| isCollapsed: message.isCollapsed | ||
| } | ||
| } | ||
| } | ||
| acc[message.id].components.push(message.component) | ||
| return acc | ||
| }, | ||
| {} | ||
| ) | ||
| acc[message.id].components.push(message.component) | ||
| return acc | ||
| }, | ||
| {} | ||
| ) | ||
|
|
||
| // Convert grouped messages into an array with explicit type | ||
| const groupedMessagesArray = Object.values(groupedMessages).map(group => ({ | ||
| ...group, | ||
| components: group.components as React.ReactNode[] | ||
| })) as { | ||
| id: string | ||
| components: React.ReactNode[] | ||
| isCollapsed?: StreamableValue<boolean> | ||
| }[] | ||
| // Convert grouped messages into an array with explicit type | ||
| return Object.values(groupedMessages).map(group => ({ | ||
| ...group, | ||
| components: group.components as React.ReactNode[] | ||
| })) as { | ||
| id: string | ||
| components: React.ReactNode[] | ||
| isCollapsed?: StreamableValue<boolean> | ||
| }[] | ||
| }, [messages]) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good use of React.memo and useMemo for performance.
The memoization strategy is sound. Two minor observations:
- The empty-check on line 44–46 is redundant with the one inside
useMemo(line 15–17) — the outer check could rely ongroupedMessagesArray.length === 0instead of re-readingmessages.length. - The
reduceaccumulator is typed as{ [key: string]: any }(line 19), which undermines the explicit cast on lines 34–41. Typing the accumulator directly avoids the need forasassertions.
♻️ Suggested typing improvement
- const groupedMessages = messages.reduce(
- (acc: { [key: string]: any }, message) => {
+ type GroupedMessage = {
+ id: string
+ components: React.ReactNode[]
+ isCollapsed?: StreamableValue<boolean>
+ }
+ const groupedMessages = messages.reduce(
+ (acc: Record<string, GroupedMessage>, message) => {
if (!acc[message.id]) {
acc[message.id] = {
id: message.id,
- components: [],
+ components: [] as React.ReactNode[],
isCollapsed: message.isCollapsed
}
}
acc[message.id].components.push(message.component)
return acc
},
{}
)
- // Convert grouped messages into an array with explicit type
- return Object.values(groupedMessages).map(group => ({
- ...group,
- components: group.components as React.ReactNode[]
- })) as {
- id: string
- components: React.ReactNode[]
- isCollapsed?: StreamableValue<boolean>
- }[]
+ return Object.values(groupedMessages)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const ChatMessages = React.memo(function ChatMessages({ messages }: ChatMessagesProps) { | |
| // Group messages based on ID, and if there are multiple messages with the same ID, combine them into one message | |
| const groupedMessages = messages.reduce( | |
| (acc: { [key: string]: any }, message) => { | |
| if (!acc[message.id]) { | |
| acc[message.id] = { | |
| id: message.id, | |
| components: [], | |
| isCollapsed: message.isCollapsed | |
| const groupedMessagesArray = useMemo(() => { | |
| if (!messages.length) { | |
| return [] | |
| } | |
| const groupedMessages = messages.reduce( | |
| (acc: { [key: string]: any }, message) => { | |
| if (!acc[message.id]) { | |
| acc[message.id] = { | |
| id: message.id, | |
| components: [], | |
| isCollapsed: message.isCollapsed | |
| } | |
| } | |
| } | |
| acc[message.id].components.push(message.component) | |
| return acc | |
| }, | |
| {} | |
| ) | |
| acc[message.id].components.push(message.component) | |
| return acc | |
| }, | |
| {} | |
| ) | |
| // Convert grouped messages into an array with explicit type | |
| const groupedMessagesArray = Object.values(groupedMessages).map(group => ({ | |
| ...group, | |
| components: group.components as React.ReactNode[] | |
| })) as { | |
| id: string | |
| components: React.ReactNode[] | |
| isCollapsed?: StreamableValue<boolean> | |
| }[] | |
| // Convert grouped messages into an array with explicit type | |
| return Object.values(groupedMessages).map(group => ({ | |
| ...group, | |
| components: group.components as React.ReactNode[] | |
| })) as { | |
| id: string | |
| components: React.ReactNode[] | |
| isCollapsed?: StreamableValue<boolean> | |
| }[] | |
| }, [messages]) | |
| export const ChatMessages = React.memo(function ChatMessages({ messages }: ChatMessagesProps) { | |
| // Group messages based on ID, and if there are multiple messages with the same ID, combine them into one message | |
| const groupedMessagesArray = useMemo(() => { | |
| if (!messages.length) { | |
| return [] | |
| } | |
| type GroupedMessage = { | |
| id: string | |
| components: React.ReactNode[] | |
| isCollapsed?: StreamableValue<boolean> | |
| } | |
| const groupedMessages = messages.reduce( | |
| (acc: Record<string, GroupedMessage>, message) => { | |
| if (!acc[message.id]) { | |
| acc[message.id] = { | |
| id: message.id, | |
| components: [] as React.ReactNode[], | |
| isCollapsed: message.isCollapsed | |
| } | |
| } | |
| acc[message.id].components.push(message.component) | |
| return acc | |
| }, | |
| {} | |
| ) | |
| return Object.values(groupedMessages) | |
| }, [messages]) |
🤖 Prompt for AI Agents
In `@components/chat-messages.tsx` around lines 12 - 42, The outer empty-check
should use groupedMessagesArray.length === 0 instead of re-reading
messages.length and the reduce accumulator should be given a concrete type to
avoid the final "as" cast: update the ChatMessages component to remove the
redundant messages.length check and change the accumulator type used in the
reduce inside groupedMessagesArray (the object built for message grouping) to a
properly typed interface (e.g., mapping ids to { id: string; components:
React.ReactNode[]; isCollapsed?: StreamableValue<boolean> }) so the final
Object.values(...).map no longer needs an assertion.
2 participants
This PR implements a significant performance optimization for the chat interface. By moving the
inputstate from the parentChatcomponent to theChatPanelleaf component (state colocation), we eliminate unnecessary re-renders of the entire application shell (Header, Map, History) on every keystroke.Additionally,
ChatMessageshas been memoized to prevent expensive message grouping calculations when the messages haven't changed.Verification:
bun run lintandbun x tsc --noEmitpass.Chatview by ~99% during typing.PR created automatically by Jules for task 13099466135290422940 started by @ngoiyaeric
Summary by CodeRabbit
Performance Improvements
Improvements
Documentation