Skip to content

Optimize provider update sidebar pill renders#3915

Draft
cursor[bot] wants to merge 2 commits into
mainfrom
cursor/component-performance-optimization-3247
Draft

Optimize provider update sidebar pill renders#3915
cursor[bot] wants to merge 2 commits into
mainfrom
cursor/component-performance-optimization-3247

Conversation

@cursor

@cursor cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

What Changed

  • Refactored SidebarProviderUpdatePill so the visible pill view is derived during render instead of synchronized through useEffect state.
  • Split the atom/router wrapper from SidebarProviderUpdatePillContent so the content owns its dismissal/exit state and can lazily capture the first provider check threshold on mount.
  • Kept the remaining auto-dismiss timer effect inside the named useProviderUpdatePillAutoDismiss hook.

Why

react-doctor flagged this component for state that was only used by handlers and synchronous setState inside effects, both of which caused extra commits. The React “You Might Not Need an Effect” guidance recommends calculating derived values during render instead of storing them via effects.

Post-fix react-doctor no longer lists SidebarProviderUpdatePill under those two findings.

UI Changes

React Scan recordings captured with React Scan enabled:

  • Before: react_scan_provider_pill_before.mp4 — five remounts increased the Profiler counter from 4 to 12.
  • After: react_scan_provider_pill_after.mp4 — the same five remounts increased the counter from 3 to 7.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Validation:

  • react-doctor@0.7.4 . --verbose --category performance --no-telemetry (post-fix scan still reports unrelated repo-wide findings, but no longer reports SidebarProviderUpdatePill for the fixed categories)
  • vp check
  • vp run typecheck
Open in Web View Automation 

Note

Refactor SidebarProviderUpdatePill to fix visible-after timestamp and simplify state model

  • Extracts pill state and UI into a new SidebarProviderUpdatePillContent component, making SidebarProviderUpdatePill a thin wrapper that short-circuits when there are no providers.
  • Replaces the multi-state model (renderedView, pendingView, exitingKey, syncing effect) with a simpler exitState object and a dismissedKeys set.
  • Fixes visibleAfterIso to be initialized once at mount from latestProviderCheckedAt(providers) and never re-synchronized, so the set of visible updates stays stable after mount.
  • Behavioral Change: the pill no longer performs an exit transition when the underlying computed view changes — it updates immediately to the new view instead.
📊 Macroscope summarized bd0cd27. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

cursoragent and others added 2 commits July 12, 2026 16:10
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 12, 2026
Comment on lines 63 to +75
useEffect(() => {
if (!dismissAfterVisibleMs || !viewKey) {
if (!displayedView || !dismissAfterVisibleMs || !viewKey) {
return;
}
if (exitingKey === viewKey) {
return;
}
const timeoutId = window.setTimeout(() => {
startExit(viewKey, null, viewKey);
startExit(displayedView, viewKey);
}, dismissAfterVisibleMs);

return () => window.clearTimeout(timeoutId);
}, [dismissAfterVisibleMs, exitingKey, startExit, viewKey]);
}, [dismissAfterVisibleMs, displayedView, exitingKey, startExit, viewKey]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium sidebar/SidebarProviderUpdatePill.tsx:63

The auto-dismiss timeout resets on every rerender because useProviderUpdatePillAutoDismiss now depends on the displayedView object reference, and getProviderUpdateSidebarPillView returns a fresh object each render even when the view key is unchanged. Any unrelated rerender during the dismiss window clears and restarts the timer. With repeated rerenders the success notification can stay visible indefinitely instead of dismissing after dismissAfterVisibleMs. The previous implementation relied on a stored renderedView that remained referentially stable for an unchanged key. Consider keying the effect on viewKey (and dismissAfterVisibleMs) rather than the whole view object, or memoizing the view by key so the reference stays stable.

  useEffect(() => {
    if (!displayedView || !dismissAfterVisibleMs || !viewKey) {
      return;
    }
    if (exitingKey === viewKey) {
      return;
    }
-    const displayedViewRef = displayedView;
    const timeoutId = window.setTimeout(() => {
-      startExit(displayedView, viewKey);
+      startExit(displayedViewRef, viewKey);
    }, dismissAfterVisibleMs);
 
    return () => window.clearTimeout(timeoutId);
-  }, [dismissAfterVisibleMs, displayedView, exitingKey, startExit, viewKey]);
+  }, [dismissAfterVisibleMs, viewKey, exitingKey, startExit]);
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/sidebar/SidebarProviderUpdatePill.tsx around lines 63-75:

The auto-dismiss timeout resets on every rerender because `useProviderUpdatePillAutoDismiss` now depends on the `displayedView` object reference, and `getProviderUpdateSidebarPillView` returns a fresh object each render even when the view `key` is unchanged. Any unrelated rerender during the dismiss window clears and restarts the timer. With repeated rerenders the success notification can stay visible indefinitely instead of dismissing after `dismissAfterVisibleMs`. The previous implementation relied on a stored `renderedView` that remained referentially stable for an unchanged key. Consider keying the effect on `viewKey` (and `dismissAfterVisibleMs`) rather than the whole view object, or memoizing the view by key so the reference stays stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant