⚡ Bolt: Optimize TopNavbar telemetry rendering#18
Conversation
- Used `useSyncExternalStore` directly on `telemetryBus.onStatus` instead of subscribing to the entire `useTelemetry` context. - This prevents `TopNavbar` (and the heavy `UserButton` inside it) from re-rendering every 2 seconds when high-frequency telemetry data (watts, etc.) updates. - Added a `.jules/bolt.md` entry to document the learning about React Context with fast-updating feeds. Co-authored-by: Jandave0 <185737041+Jandave0@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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/layout/top-navbar.tsx`:
- Around line 44-51: The component currently ignores the mqttConnected prop by
reading status directly from telemetryBus via useSyncExternalStore
(telemetryBus, useSyncExternalStore, status), which silently breaks the public
contract; either remove the mqttConnected prop/JSDoc from the TopNavbar
component and related typings/usages, or make mqttConnected an explicit
fallback/override by using it when telemetryBus.status is undefined/unavailable
(e.g., compute status = telemetryBus.status ?? (mqttConnected ? "connected" :
"disconnected")), and update the component prop docs to reflect the chosen
behavior.
🪄 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 Plus
Run ID: 0fcca770-0a29-4db2-9f5a-1e5ba7a0588b
📒 Files selected for processing (2)
.jules/bolt.mdcomponents/layout/top-navbar.tsx
|
|
||
| // Use useSyncExternalStore directly on the telemetryBus to avoid re-rendering | ||
| // every 2 seconds when telemetry data (watts, etc.) updates. | ||
| const status = useSyncExternalStore( | ||
| useCallback((cb) => telemetryBus.onStatus(cb), []), | ||
| () => telemetryBus.status, | ||
| () => "disconnected" as const | ||
| ); |
There was a problem hiding this comment.
Remove or honor mqttConnected; it is ignored now.
This subscription bypasses the mqttConnected prop entirely, so callers can still pass a public prop that no longer affects the rendered status. Either delete the prop/JSDoc or make it an explicit fallback/override to avoid a silent contract break.
Suggested cleanup if the bus is now the sole source of truth
export interface TopNavbarProps {
- mqttConnected?: boolean;
}
@@
- * `@param` mqttConnected - Whether the MQTT connection is active (default: `false`).
* `@returns` The header element containing navigation controls and status/profile indicators.
*/
-export function TopNavbar({ mqttConnected = false }: TopNavbarProps) {
+export function TopNavbar({}: TopNavbarProps) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/layout/top-navbar.tsx` around lines 44 - 51, The component
currently ignores the mqttConnected prop by reading status directly from
telemetryBus via useSyncExternalStore (telemetryBus, useSyncExternalStore,
status), which silently breaks the public contract; either remove the
mqttConnected prop/JSDoc from the TopNavbar component and related
typings/usages, or make mqttConnected an explicit fallback/override by using it
when telemetryBus.status is undefined/unavailable (e.g., compute status =
telemetryBus.status ?? (mqttConnected ? "connected" : "disconnected")), and
update the component prop docs to reflect the chosen behavior.
1 participant
💡 What: Swapped
useTelemetry()context consumption inTopNavbarfor a direct subscription totelemetryBus.statususinguseSyncExternalStore.🎯 Why:
useTelemetry()updates every 2 seconds as new telemetry data (watts, wind speed, etc.) arrives. This caused the entireTopNavbar—including the computationally heavy ClerkUserButton—to unnecessarily re-render on every tick, even though the navbar only cares about the MQTT connection status which rarely changes.📊 Impact: Eliminates ~30 unnecessary re-renders per minute in the TopNavbar, reducing main thread blocking and CPU usage.
🔬 Measurement: Use React DevTools Profiler. Prior to the change,
TopNavbarwill flash green every 2 seconds. After the change, it will only flash when the MQTT status changes from connecting to connected/disconnected.PR created automatically by Jules for task 8773569652792296493 started by @Jandave0
Summary by CodeRabbit
Release Notes