refactor: eliminate unnecessary useEffects — bridges, effect events, budget ratchet (E1–E3, E7) - #163
refactor: eliminate unnecessary useEffects — bridges, effect events, budget ratchet (E1–E3, E7)#163TheOrcDev wants to merge 1 commit into
Conversation
…atchet) use-studio.tsx drops from 58 to 48 useEffects with zero behavior changes; franky47's PR #150 review generalized (vault plan 2026-07-19). E1 — latest-value ref mirrors become render-body assignments (the file's existing settingsRef pattern); the client-reset of the layout-commit proof moves into the client-setup effect; localStorage persistence rides a new LocalStoragePersister bridge (identity fast path so renders are free, content dedupe, 150ms trailing debounce, pagehide flush, explicit quota diagnostics) instead of two serialize-on-every-change effects. E2 — subscription churn bugs fixed with React 19.2 effect events: the window keydown listener re-registered on TEN callback identities (practically every session-state render) and the comment-highlight IPC listener resubscribed on every chat message — each swap a window where an event could drop. Both handlers are now useEffectEvent with ONE mount-only registration; the two comment IPC listeners share a single effect. E3 — state-transition reactions move into the handlers that apply the transition: the X Producer reminder runs from applyRecordingStatus, and failed-stream-target toasts run from the streamTargets mutation sites (snapshot-driven, empty snapshot resets the dedupe). E4-lite — one-way pushes ride a generic RenderSyncedCall bridge (content dedupe, post-commit send): preview-window aspect ratio and the captions window snapshot. The orientation integration test now awaits the bridge tick; the contract (new dimensions arrive, never stale ones) is unchanged and still asserted. E7 — scripts/check-effect-budget.mjs ratchet wired into pnpm lint: per-file useEffect budgets (108 across 34 renderer files) fail the gate on ANY increase and demand a ratchet-down when a file improves, so the count can never silently grow again. Verification: desktop 1206 green incl. all 29 studio-provider integration tests, new persister/bridge unit tests, typecheck, lint + budget gate, format, scripts 759 green. smoke:remote-control could not run: the dev-app launcher times out on CLEAN main too (no [smoke] markers after "start electron app") — pre-existing, tracked separately. Remaining plan slices (E3 rest, E4 heavy bridges, E5 window reconciler, E6 component tail) are follow-ups; the ratchet pins today's counts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ 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 |
1 participant
Executes the first tranche of the useEffect elimination plan (follow-up to franky47's #150 review).
use-studio.tsx: 58 → 48 effects; renderer total 108, now enforced by a ratchet. Zero behavior changes.What changed
E1 — mirrors + persistence. Latest-value ref mirrors (
deviceListRef,captureConfigRef) become render-body assignments — the pattern the file already used forsettingsRef. localStorage persistence moves to aLocalStoragePersisterbridge: object-identity fast path (re-renders are free), content dedupe, 150ms trailing debounce,pagehideflush, and explicit quota-failure diagnostics. The old effects JSON-serialized the full captureConfig synchronously on every change.E2 — real subscription bugs. The window keydown listener listed ten callback identities in its deps, so it removed/re-added itself on practically every session-state render; the comment-highlight IPC listener resubscribed on every chat message. Every swap was a window where a shortcut or highlight request could drop. Both are now
useEffectEventhandlers with one mount-only registration; the two comment IPC listeners share a single effect.E3 — transition logic lives with the transition. The X Producer reminder runs from
applyRecordingStatus(the single recording-state funnel); failed-stream-target toasts run from thestreamTargetsmutation sites, driven by the snapshot the backend just delivered.E4-lite — one-way pushes. A generic
RenderSyncedCall<T>bridge (content dedupe, post-commit send, unit-tested) replaces the preview-aspect-ratio and captions-window-snapshot effects. The orientation integration test now awaits the bridge tick — the asserted contract (the new dimensions arrive, never stale ones) is unchanged.E7 — the ratchet.
scripts/check-effect-budget.mjsis wired intopnpm lint: per-file budgets (checked in at today's reality, 108 across 34 files) fail on any increase, and a file that improves fails until the budget is ratcheted down — wins get locked in, and the count can never silently grow again.Verification
pnpm typecheck,pnpm lint(now including the budget gate),pnpm format:check,pnpm test:scripts(759) all green.smoke:remote-controlcould not be exercised: the dev-app launcher times out on clean main too (Electron starts, zero output, no[smoke]markers) — pre-existing after perf: bound runtime and Studio resource lifetimes #153/feat: add secure Windows alpha release pipeline #154, tracked as a separate task; not caused by this branch.Remaining plan slices (follow-ups, pinned by the ratchet)
E3 remainder (mic-meter reset, caption epochs, platform-account refreshes), E4 heavy bridges (LiveAudioMirror, CaptionAppearanceMirror, StreamKeyHintHydrator), E5 WindowStateReconciler, E6 component tail sweep. Each is a separate reviewable PR per the plan's non-negotiables.
🤖 Generated with Claude Code