Repository navigation
perf(player): keep the playback position out of the player context - #809
Conversation
player:position ticks four times a second, and as context state each tick re-rendered every usePlayer consumer, the library view included. The position now lives in a small external store: components that show it call usePlayerPosition, the A-B loop button reads it on click only.
InstaZDLL
added
scope: frontend
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (12)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. 8 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour. 📝 WalkthroughWalkthroughLa position de lecture est retirée de ChangesPosition de lecture
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to The change is mergeable; normal validation should still cover the listed playback interactions. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change remains confined to playback-position updates within each window. Existing command paths and event handling are preserved, and no introduced security concern was identified. The main residual uncertainty is how the shared-state lifecycle behaves under unusual event interleavings. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Problem
player:positionticks four times a second.positionMswas state inPlayerContext, and the provider's value is an inline object, so every tick re-rendered everyusePlayer()consumer (38 of them,LibraryViewincluded) to redraw the seven places that actually show the position.Fix
src/hooks/usePlayerPosition.ts: a tiny external store (useSyncExternalStore) withusePlayerPosition()for components that render the position,getPlayerPosition()for code that only needs it in a handler, andsetPlayerPosition()written byPlayerProvideronly (ticks, seeks, resets: the same eight call sites as before). One copy per webview, like the provider itself.positionMsis removed fromPlayerContextValue, so the compiler lists every reader:ProgressBar,MiniPlayer,LyricsEditorModal,HeldNote,useKaraokeWordFillanduseTrackLyricsnow subscribe to the store;AbLoopButtonreads it on click and no longer re-renders on ticks at all.No behaviour change intended. Docs: an invariant in
invariants.md(puttingpositionMsback in the context brings the 4 Hz re-render back), one line inCLAUDE.md.Manual check before merge
Progress bar (drag + keyboard seek), synced lyrics + karaoke word fill + held notes, the lyrics editor's capture button, A-B loop capture, the mini-player's seek bar.
Summary by CodeRabbit