17 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 14-performance-optimization | 03 | execute | 2 |
|
|
true |
|
|
Purpose: Every scroll frame, renderTrackRow and renderTrackItem create new arrow function closures for click, dblclick, contextmenu, and dragstart handlers. This causes GC pressure during rapid scrolling. Additionally, the queue store notifies synchronously (not batched), and the library store sends blanket notifications for any data change even if the subscribing component only cares about one data type.
Output: Scroll rendering is GC-friendly with stable function references; store notifications are batched and granular.
<execution_context> @/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md @/home/caleb/.config/opencode/get-shit-done/templates/summary.md </execution_context>
@.planning/PROJECT.md @.planning/ROADMAP.md @.planning/STATE.md @frontend/src/components/track-list/track-list.ts @frontend/src/components/queue-panel/queue-panel.ts @frontend/src/store/queue-store.ts @frontend/src/store/library-store.ts @frontend/src/store/controllers/library-controller.ts ```typescript private renderTrackRow = (track: library.Track, index: number): unknown => { // Creates new closures every render: // @click=${(e: MouseEvent) => this.onTrackRowClick(e, track, index)} // @dblclick=${() => this.onTrackRowDblClick(track)} // @contextmenu=${(e: MouseEvent) => this.onTrackContextMenu(e, track)} // @dragstart=${(e: DragEvent) => this.onTrackDragStart(e, track)} }; ```private notify(): void {
for (const sub of this.subscribers) {
sub();
}
}
// LibraryController subscribes to ALL store changes
hostConnected(): void {
this.unsubscribe = libraryStore.subscribe(() => {
this.host.requestUpdate();
});
}
private notify(): void {
if (this.notifyScheduled) return;
this.notifyScheduled = true;
queueMicrotask(() => {
this.notifyScheduled = false;
for (const sub of this.subscribers) {
sub();
}
});
}
track-list.ts — renderTrackRow:
The current pattern creates 5 new closures per row per render: @click, @dblclick, @contextmenu, @dragstart, and the fav-icon @click.
Convert to data-attribute event delegation:
-
Add
data-index="${index}"to the.track-rowdiv. -
Instead of per-row
@click,@dblclick,@contextmenu,@dragstartclosures, use a single delegated event handler pattern. Add stable (bound once) event handlers on thelit-virtualizerelement itself infirstUpdated():
override firstUpdated() {
// ... existing firstUpdated code ...
const virt = this.virtualizer;
if (virt) {
virt.addEventListener('click', this.onDelegatedClick);
virt.addEventListener('dblclick', this.onDelegatedDblClick);
virt.addEventListener('contextmenu', this.onDelegatedContextMenu);
// dragstart delegated on track-row (draggable="true" is on each row)
}
}
- Create stable bound methods that extract the index from
data-index:
private onDelegatedClick = (e: MouseEvent) => {
const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null;
if (!row) return;
const idx = Number(row.dataset.index);
const track = this.cachedSortedTracks[idx];
if (!track) return;
// Check if click was on fav-icon
const favEl = (e.target as HTMLElement).closest('.fav-icon');
if (favEl) {
e.stopPropagation();
void this.favCtrl.toggleFavorite(track.FilePath);
return;
}
this.onTrackRowClick(e, track, idx);
};
private onDelegatedDblClick = (e: MouseEvent) => {
const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null;
if (!row) return;
const idx = Number(row.dataset.index);
const track = this.cachedSortedTracks[idx];
if (track) this.onTrackRowDblClick(track);
};
private onDelegatedContextMenu = (e: MouseEvent) => {
const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null;
if (!row) return;
const idx = Number(row.dataset.index);
const track = this.cachedSortedTracks[idx];
if (track) this.onTrackContextMenu(e, track);
};
- For
@dragstart: Keep it inline on the row element but use the data-index delegation pattern. Sincedraggable="true"must be on the individual row, the dragstart event naturally targets the row. Add a single delegated handler:
private onDelegatedDragStart = (e: DragEvent) => {
const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null;
if (!row) return;
const idx = Number(row.dataset.index);
const track = this.cachedSortedTracks[idx];
if (track) this.onTrackDragStart(e, track);
};
- Update
renderTrackRowto remove ALL inline closures:
private renderTrackRow = (track: library.Track, index: number): unknown => {
// ... classMap, isFav, etc. remain the same ...
return html`
<div
class=${classMap({ 'track-row': true, active, selected })}
draggable="true"
data-index=${index}
>
<!-- fav icon — click handled by delegation -->
<div class=${classMap({ 'fav-icon': true, favorited: isFav })}>
<wa-icon name=${this.favCtrl.iconName} variant=${favVariant}></wa-icon>
</div>
${/* column rendering stays the same */}
</div>
`;
};
- Add
@dragendas a single stable handler on the virtualizer too (it's alreadythis.onTrackDragEndwhich is stable).
CRITICAL: Event delegation must work through shadow DOM. Since the virtualizer and its children are all within the same shadow root, event.target.closest('.track-row') works correctly. But verify with composedPath() if needed.
queue-panel.ts — renderTrackItem:
Apply the same event delegation pattern:
- Add
data-index="${track.position}"(or the loop index) to each.track-item - Register delegated handlers on the virtualizer in firstUpdated
- Extract item index via
closest('.track-item')?.dataset.index - Remove all inline closures from the template
cover-grid.ts — renderAlbumCard/renderGridEntry:
The cover grid already uses some delegation (it reads data-index for some operations). Verify that all click handlers on album cards use delegation. If any inline closures remain in renderGridEntry or renderAlbumCard, convert them.
Key rule: After this change, renderTrackRow and renderTrackItem should create ZERO new function objects. Every handler reference should be stable (either a bound class method or a property arrow function defined once in the class body).
cd frontend && npx vite build --mode development 2>&1 | tail -5 builds without errors.
Functional test: (1) Click a track → plays correctly, (2) Double-click → plays, (3) Right-click → context menu appears with correct track, (4) Drag a track → drag image shows, drop works, (5) Click favorite icon → toggles correctly, (6) Multi-select with Shift/Ctrl → works, (7) Queue panel: click, dblclick, drag, context menu all work.
renderTrackRow and renderTrackItem create zero inline closures. All event handling uses delegation via data-index attributes and stable bound handlers.
The library store already uses queueMicrotask batching (added in Phase 8). The queue store does NOT — it calls all subscribers synchronously on every state change. This means rapid queue mutations (e.g., adding multiple tracks) trigger multiple synchronous re-renders.
Add the same batching pattern from library-store:
private notifyScheduled = false;
private notify(): void {
if (this.notifyScheduled) return;
this.notifyScheduled = true;
queueMicrotask(() => {
this.notifyScheduled = false;
for (const sub of this.subscribers) {
sub();
}
});
}
This coalesces multiple synchronous notify() calls into a single subscriber notification per microtask tick. Safe because Lit's requestUpdate() already deduplicates internally, but this prevents the overhead of even invoking all subscriber callbacks multiple times.
library-store.ts — Add granular data-type subscriptions:
Currently subscribe() registers a callback that fires on ANY store change (tracks, albums, artists, genres, cover size, loading state). This means:
- Track list component gets notified when albums change (unnecessary requestUpdate)
- Album grid gets notified when genres change (unnecessary requestUpdate)
- All components get notified when any loading flag changes
Add type-specific subscriptions alongside the existing blanket subscribe():
type DataType = 'tracks' | 'albums' | 'artists' | 'genres' | 'coverSize';
private typedSubscribers = new Map<DataType, Set<Subscriber>>();
subscribeTo(type: DataType, callback: Subscriber): () => void {
if (!this.typedSubscribers.has(type)) {
this.typedSubscribers.set(type, new Set());
}
const subs = this.typedSubscribers.get(type)!;
subs.add(callback);
return () => subs.delete(callback);
}
private notifyType(type: DataType): void {
const subs = this.typedSubscribers.get(type);
if (subs) {
for (const sub of subs) {
sub();
}
}
}
Then update the data access methods to use notifyType:
getTracks()finally block: callnotifyType('tracks')instead ofnotify()getAlbums()finally block: callnotifyType('albums')instead ofnotify()getArtists()finally block: callnotifyType('artists')instead ofnotify()getGenres()finally block: callnotifyType('genres')instead ofnotify()setCoverSize(): callnotifyType('coverSize')instead ofnotify()invalidate(): Keep callingnotify()(blanket) since invalidation affects everything
Wait — this is tricky. The notify() method uses queueMicrotask batching. If we have both typed and blanket notifications in the same microtask, we need to be careful.
Simpler approach: Instead of typed subscriptions on the store, make LibraryController smarter. The controller already has access to cachedTracks, cachedAlbums, etc. On each store notification, the controller can CHECK if the data it cares about actually changed before calling requestUpdate():
// In LibraryController
hostConnected(): void {
// Track the references we last saw
let lastTracks = libraryStore.getCachedTracks();
let lastAlbums = libraryStore.getCachedAlbums();
this.unsubscribe = libraryStore.subscribe(() => {
const newTracks = libraryStore.getCachedTracks();
const newAlbums = libraryStore.getCachedAlbums();
const newArtists = libraryStore.getCachedArtists();
const newGenres = libraryStore.getCachedGenres();
// Only request update if data this host cares about changed
// Since we don't know what the host uses, check all and requestUpdate
// if ANY changed. But crucially, skip if loading state just toggled.
if (newTracks !== lastTracks ||
newAlbums !== lastAlbums ||
newArtists !== this.lastArtists ||
newGenres !== this.lastGenres) {
lastTracks = newTracks;
lastAlbums = newAlbums;
// ... etc
this.host.requestUpdate();
}
});
}
Actually, this is still checking everything. The real win is: don't call requestUpdate when only loading state changed. The loading state toggling on/off during eagerFetch causes 8+ unnecessary requestUpdate calls across all components.
Refined approach for library-store.ts:
Add a changeGeneration counter. Increment it only when actual data changes (not loading flags):
private changeGen = 0;
// In getTracks, getAlbums, etc. — after setting this.tracks = tracks:
this.changeGen++;
// In invalidate — after clearing caches:
this.changeGen++;
Then in notify(), also expose the generation. In the controller:
hostConnected(): void {
let lastGen = libraryStore.changeGeneration;
this.unsubscribe = libraryStore.subscribe(() => {
const gen = libraryStore.changeGeneration;
if (gen !== lastGen) {
lastGen = gen;
this.host.requestUpdate();
}
});
}
Add a public get changeGeneration(): number to the store.
This means: loading flag changes trigger notify() but subscribers skip the update because changeGeneration hasn't changed. Only when actual data arrives (or is invalidated) do components re-render.
Use this approach. It's simpler, backward-compatible, and eliminates the biggest source of unnecessary re-renders.
cd frontend && npx vite build --mode development 2>&1 | tail -5 builds without errors.
Functional test: (1) App starts → all views load data correctly, (2) Trigger a library scan → views update when scan completes, (3) Queue operations (add, remove, reorder) work without lag, (4) Rapid queue additions don't cause visual stuttering.
Queue store uses queueMicrotask batching. Library store has changeGeneration counter. LibraryController skips requestUpdate when only loading state changed. Result: fewer unnecessary component re-renders during data loading.
<success_criteria>
- renderTrackRow creates 0 inline closures (all delegation)
- renderTrackItem creates 0 inline closures (all delegation)
- queue-store.ts contains queueMicrotask batching
- library-store.ts has changeGeneration counter
- LibraryController checks changeGeneration before requestUpdate
- All existing interactions work correctly </success_criteria>