Skip to content

Commit 594589c

Browse files
naomiaroclaude
andcommitted
refactor: extract useChunkedCanvasRefs hook to standardize canvas ref pattern
The second Rule of Three violation: Channel, TimeScale, and SpectrogramChannel all had identical callback-ref + stale-cleanup logic for managing chunked canvas elements. Standardize on Map<number, HTMLCanvasElement> (TimeScale already used Map; Channel and SpectrogramChannel used sparse arrays). Net -57 lines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 07acf8f commit 594589c

File tree

6 files changed

+70
-81
lines changed

6 files changed

+70
-81
lines changed

CLAUDE.MD

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,10 @@ const LazyExample = createLazyExample(
467467
- `SpectrogramChannel` — only mounts visible chunks (biggest memory win)
468468
- All use absolute positioning (`left: chunkIndex * 1000px`) instead of `float: left`
469469

470+
**Shared hooks:**
471+
- `useVisibleChunkIndices(totalWidth, chunkWidth)` — returns memoized array of visible chunk indices. Uses string-key comparison internally for re-render gating. Exported from `ui-components`.
472+
- `useChunkedCanvasRefs()` — callback ref + Map storage + stale cleanup for chunked canvases. Internal only (not exported from package public API). Uses `Map<number, HTMLCanvasElement>` instead of sparse arrays.
473+
470474
**Backwards compatibility:** `useScrollViewport()` returns `null` without provider. All components default to rendering everything when viewport is `null`.
471475

472476
### Web Worker Peak Generation (2026-02-13)
@@ -634,6 +638,8 @@ const analyser = context.createAnalyser();
634638
17. **Fetch Cleanup with AbortController** - `useAudioTracks` uses AbortController to cancel in-flight fetches on cleanup. Follow this pattern for any fetch in useEffect. For per-item abort (e.g., removing one loading track), use `Map<id, AbortController>` instead of `Set<AbortController>`.
635639
18. **Derive Render Guards from Props, Not Effect State** - Don't use effect-set state (e.g., `audioBuffers`) in render guards. Effect state lags props by one+ renders, causing content to flash/disappear. Compute values synchronously from props instead.
636640
19. **Copy Refs in useEffect Body** - When accessing a ref in `useEffect` cleanup, copy `.current` to a local variable inside the effect body. ESLint's `react-hooks/exhaustive-deps` rule flags refs that may change between render and cleanup.
641+
20. **Refs from Custom Hooks in Dep Arrays** - When a `useRef` is returned from a custom hook, ESLint's `exhaustive-deps` can't trace its stability. Include it in the dep array (harmless, never triggers) rather than using `eslint-disable-next-line` which would mask real missing dependencies.
642+
21. **Every-Render Cleanup Effects** - `useChunkedCanvasRefs` and SpectrogramChannel's worker cleanup use `useEffect` with no dependency array intentionally. The virtualizer can unmount canvases between any render. Do NOT add `[]` or other deps — this would break stale ref pruning.
637643

638644
### Playlist Loading Detection
639645

packages/ui-components/src/components/Channel.tsx

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import React, { FunctionComponent, useLayoutEffect, useEffect, useCallback, useRef } from 'react';
1+
import React, { FunctionComponent, useLayoutEffect } from 'react';
22
import styled from 'styled-components';
33
import type { Peaks, Bits } from '@waveform-playlist/core';
44
import { WaveformColor, WaveformDrawMode, isWaveformGradient, waveformColorToCss } from '../wfpl-theme';
55
import { useVisibleChunkIndices } from '../contexts/ScrollViewport';
6+
import { useChunkedCanvasRefs } from '../hooks/useChunkedCanvasRefs';
67
import { MAX_CANVAS_WIDTH } from '../constants';
78

89
// Re-export WaveformColor for consumers
@@ -121,41 +122,16 @@ export const Channel: FunctionComponent<ChannelProps> = (props) => {
121122
transparentBackground = false,
122123
drawMode = 'inverted',
123124
} = props;
124-
const canvasesRef = useRef<HTMLCanvasElement[]>([]);
125+
const { canvasRef, canvasMapRef } = useChunkedCanvasRefs();
125126

126127
const visibleChunkIndices = useVisibleChunkIndices(length, MAX_CANVAS_WIDTH);
127128

128-
const canvasRef = useCallback(
129-
(canvas: HTMLCanvasElement | null) => {
130-
if (canvas !== null) {
131-
const index: number = parseInt(canvas.dataset.index!, 10);
132-
canvasesRef.current[index] = canvas;
133-
}
134-
},
135-
[]
136-
);
137-
138-
// Clean up stale refs for unmounted chunks
139-
useEffect(() => {
140-
const canvases = canvasesRef.current;
141-
for (let i = canvases.length - 1; i >= 0; i--) {
142-
if (canvases[i] && !canvases[i].isConnected) {
143-
delete canvases[i];
144-
}
145-
}
146-
});
147-
148129
// Draw waveform bars on visible canvas chunks.
149-
// visibleChunkKey changes only when chunks mount/unmount, not on every scroll pixel.
130+
// visibleChunkIndices changes only when chunks mount/unmount, not on every scroll pixel.
150131
useLayoutEffect(() => {
151-
const canvases = canvasesRef.current;
152132
const step = barWidth + barGap;
153133

154-
for (let i = 0; i < canvases.length; i++) {
155-
const canvas = canvases[i];
156-
if (!canvas) continue; // Skip unmounted chunks (sparse array from virtualization)
157-
158-
const canvasIdx = parseInt(canvas.dataset.index!, 10);
134+
for (const [canvasIdx, canvas] of canvasMapRef.current.entries()) {
159135
const globalPixelOffset = canvasIdx * MAX_CANVAS_WIDTH;
160136

161137
const ctx = canvas.getContext('2d');
@@ -232,6 +208,7 @@ export const Channel: FunctionComponent<ChannelProps> = (props) => {
232208
}
233209
}
234210
}, [
211+
canvasMapRef,
235212
data,
236213
bits,
237214
waveHeight,

packages/ui-components/src/components/SpectrogramChannel.tsx

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import React, { FunctionComponent, useLayoutEffect, useCallback, useRef, useEffect } from 'react';
1+
import React, { FunctionComponent, useLayoutEffect, useRef, useEffect } from 'react';
22
import styled from 'styled-components';
33
import type { SpectrogramData } from '@waveform-playlist/core';
44
import { useVisibleChunkIndices } from '../contexts/ScrollViewport';
5+
import { useChunkedCanvasRefs } from '../hooks/useChunkedCanvasRefs';
56
import { MAX_CANVAS_WIDTH } from '../constants';
67
const LINEAR_FREQUENCY_SCALE = (f: number, minF: number, maxF: number) => (f - minF) / (maxF - minF);
78

@@ -110,7 +111,7 @@ export const SpectrogramChannel: FunctionComponent<SpectrogramChannelProps> = ({
110111
onCanvasesReady,
111112
}) => {
112113
const channelIndex = channelIndexProp ?? index;
113-
const canvasesRef = useRef<HTMLCanvasElement[]>([]);
114+
const { canvasRef, canvasMapRef } = useChunkedCanvasRefs();
114115
const registeredIdsRef = useRef<string[]>([]);
115116
const transferredCanvasesRef = useRef<WeakSet<HTMLCanvasElement>>(new WeakSet());
116117
const workerApiRef = useRef(workerApi);
@@ -121,16 +122,6 @@ export const SpectrogramChannel: FunctionComponent<SpectrogramChannelProps> = ({
121122

122123
const visibleChunkIndices = useVisibleChunkIndices(length, MAX_CANVAS_WIDTH);
123124

124-
const canvasRef = useCallback(
125-
(canvas: HTMLCanvasElement | null) => {
126-
if (canvas !== null) {
127-
const idx = parseInt(canvas.dataset.index!, 10);
128-
canvasesRef.current[idx] = canvas;
129-
}
130-
},
131-
[]
132-
);
133-
134125
const lut = colorLUT ?? DEFAULT_COLOR_LUT;
135126
const maxF = maxFrequency ?? (data ? data.sampleRate / 2 : 22050);
136127
const scaleFn = frequencyScaleFn ?? LINEAR_FREQUENCY_SCALE;
@@ -154,18 +145,13 @@ export const SpectrogramChannel: FunctionComponent<SpectrogramChannelProps> = ({
154145
const currentWorkerApi = workerApiRef.current;
155146
if (!currentWorkerApi || !clipId) return;
156147

157-
const canvases = canvasesRef.current;
158148
const newIds: string[] = [];
159149
const newWidths: number[] = [];
160150

161-
for (let i = 0; i < canvases.length; i++) {
162-
const canvas = canvases[i];
163-
if (!canvas) continue;
164-
151+
for (const [canvasIdx, canvas] of canvasMapRef.current.entries()) {
165152
// Skip canvases that have already been transferred to the worker
166153
if (transferredCanvasesRef.current.has(canvas)) continue;
167154

168-
const canvasIdx = parseInt(canvas.dataset.index!, 10);
169155
const canvasId = `${clipId}-ch${channelIndex}-chunk${canvasIdx}`;
170156

171157
let offscreen: OffscreenCanvas;
@@ -194,7 +180,7 @@ export const SpectrogramChannel: FunctionComponent<SpectrogramChannelProps> = ({
194180
registeredIdsRef.current = [...registeredIdsRef.current, ...newIds];
195181
onCanvasesReadyRef.current?.(newIds, newWidths);
196182
}
197-
}, [isWorkerMode, clipId, channelIndex, length, visibleChunkIndices]);
183+
}, [canvasMapRef, isWorkerMode, clipId, channelIndex, length, visibleChunkIndices]);
198184

199185
// Clean up stale worker registrations for canvases that unmounted
200186
useEffect(() => {
@@ -209,7 +195,7 @@ export const SpectrogramChannel: FunctionComponent<SpectrogramChannelProps> = ({
209195
const match = id.match(/chunk(\d+)$/);
210196
if (!match) { remaining.push(id); continue; }
211197
const chunkIdx = parseInt(match[1], 10);
212-
const canvas = canvasesRef.current[chunkIdx];
198+
const canvas = canvasMapRef.current.get(chunkIdx);
213199
if (canvas && canvas.isConnected) {
214200
remaining.push(id);
215201
} else {
@@ -244,19 +230,14 @@ export const SpectrogramChannel: FunctionComponent<SpectrogramChannelProps> = ({
244230
useLayoutEffect(() => {
245231
if (isWorkerMode || !data) return;
246232

247-
const canvases = canvasesRef.current;
248233
const { frequencyBinCount, frameCount, hopSize, sampleRate, gainDb, rangeDb: rawRangeDb } = data;
249234
const rangeDb = rawRangeDb === 0 ? 1 : rawRangeDb;
250235

251236
// Pre-compute Y mapping: for each pixel row, which frequency bin(s) to sample
252237
const binToFreq = (bin: number) => (bin / frequencyBinCount) * (sampleRate / 2);
253238

254-
for (let i = 0; i < canvases.length; i++) {
255-
const canvas = canvases[i];
256-
if (!canvas) continue; // Skip unmounted chunks
257-
258-
const canvasIdx = parseInt(canvas.dataset.index!, 10);
259-
const globalPixelOffset = canvasIdx * MAX_CANVAS_WIDTH; // Compute from index
239+
for (const [canvasIdx, canvas] of canvasMapRef.current.entries()) {
240+
const globalPixelOffset = canvasIdx * MAX_CANVAS_WIDTH;
260241

261242
const ctx = canvas.getContext('2d');
262243
if (!ctx) continue;
@@ -348,7 +329,7 @@ export const SpectrogramChannel: FunctionComponent<SpectrogramChannelProps> = ({
348329

349330
}
350331

351-
}, [isWorkerMode, data, length, waveHeight, devicePixelRatio, samplesPerPixel, lut, minFrequency, maxF, scaleFn, hasCustomFrequencyScale, visibleChunkIndices]);
332+
}, [canvasMapRef, isWorkerMode, data, length, waveHeight, devicePixelRatio, samplesPerPixel, lut, minFrequency, maxF, scaleFn, hasCustomFrequencyScale, visibleChunkIndices]);
352333

353334
// Build visible canvas chunk elements
354335
const canvases = visibleChunkIndices.map((i) => {

packages/ui-components/src/components/TimeScale.tsx

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import React, { FunctionComponent, useRef, useEffect, useLayoutEffect, useContext, useMemo, useCallback } from 'react';
1+
import React, { FunctionComponent, useLayoutEffect, useContext, useMemo } from 'react';
22
import styled, { withTheme, DefaultTheme } from 'styled-components';
33
import { PlaylistInfoContext } from '../contexts/PlaylistInfo';
44
import { useDevicePixelRatio } from '../contexts/DevicePixelRatio';
55
import { useVisibleChunkIndices } from '../contexts/ScrollViewport';
6+
import { useChunkedCanvasRefs } from '../hooks/useChunkedCanvasRefs';
67
import { secondsToPixels } from '../utils/conversions';
78
import { MAX_CANVAS_WIDTH } from '../constants';
89

@@ -86,7 +87,7 @@ export const TimeScale: FunctionComponent<TimeScalePropsWithTheme> = (props) =>
8687
secondStep,
8788
renderTimestamp,
8889
} = props;
89-
const canvasRefsMap = useRef<Map<number, HTMLCanvasElement>>(new Map());
90+
const { canvasRef, canvasMapRef } = useChunkedCanvasRefs();
9091
const {
9192
sampleRate,
9293
samplesPerPixel,
@@ -95,13 +96,6 @@ export const TimeScale: FunctionComponent<TimeScalePropsWithTheme> = (props) =>
9596
} = useContext(PlaylistInfoContext);
9697
const devicePixelRatio = useDevicePixelRatio();
9798

98-
const canvasRefCallback = useCallback((canvas: HTMLCanvasElement | null) => {
99-
if (canvas !== null) {
100-
const idx = parseInt(canvas.dataset.index!, 10);
101-
canvasRefsMap.current.set(idx, canvas);
102-
}
103-
}, []);
104-
10599
const { widthX, canvasInfo, timeMarkersWithPositions } = useMemo(() => {
106100
const nextCanvasInfo = new Map<number, number>();
107101
const nextMarkers: Array<{ pix: number; element: React.ReactNode }> = [];
@@ -160,7 +154,7 @@ export const TimeScale: FunctionComponent<TimeScalePropsWithTheme> = (props) =>
160154
width={chunkWidth * devicePixelRatio}
161155
height={timeScaleHeight * devicePixelRatio}
162156
data-index={i}
163-
ref={canvasRefCallback}
157+
ref={canvasRef}
164158
/>
165159
);
166160
});
@@ -180,20 +174,10 @@ export const TimeScale: FunctionComponent<TimeScalePropsWithTheme> = (props) =>
180174
.map(({ element }) => element)
181175
: timeMarkersWithPositions.map(({ element }) => element);
182176

183-
// Clean up stale refs for unmounted chunks
184-
useEffect(() => {
185-
const currentMap = canvasRefsMap.current;
186-
for (const [idx, canvas] of currentMap.entries()) {
187-
if (!canvas.isConnected) {
188-
currentMap.delete(idx);
189-
}
190-
}
191-
});
192-
193177
// Draw tick marks on visible canvas chunks.
194-
// visibleChunkKey changes only when chunks mount/unmount, not on every scroll pixel.
178+
// visibleChunkIndices changes only when chunks mount/unmount, not on every scroll pixel.
195179
useLayoutEffect(() => {
196-
for (const [chunkIdx, canvas] of canvasRefsMap.current.entries()) {
180+
for (const [chunkIdx, canvas] of canvasMapRef.current.entries()) {
197181
const ctx = canvas.getContext('2d');
198182
if (!ctx) continue;
199183

@@ -215,7 +199,7 @@ export const TimeScale: FunctionComponent<TimeScalePropsWithTheme> = (props) =>
215199
ctx.fillRect(localX, scaleY, 1, scaleHeight);
216200
}
217201
}
218-
}, [duration, devicePixelRatio, timeColor, timeScaleHeight, canvasInfo, visibleChunkIndices]);
202+
}, [canvasMapRef, duration, devicePixelRatio, timeColor, timeScaleHeight, canvasInfo, visibleChunkIndices]);
219203

220204
return (
221205
<PlaylistTimeScaleScroll

packages/ui-components/src/contexts/ScrollViewport.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ export function useScrollViewportSelector<T>(
166166
* Only triggers a re-render when the set of visible chunks changes, not on every scroll pixel.
167167
*
168168
* @param totalWidth Total width in CSS pixels of the content being chunked.
169-
* @param chunkWidth Width of each chunk in CSS pixels. Defaults to MAX_CANVAS_WIDTH (1000).
169+
* @param chunkWidth Width of each chunk in CSS pixels (typically MAX_CANVAS_WIDTH, 1000).
170170
*/
171171
export function useVisibleChunkIndices(totalWidth: number, chunkWidth: number): number[] {
172172
const visibleChunkKey = useScrollViewportSelector((viewport) => {
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { useCallback, useEffect, useRef } from 'react';
2+
3+
/**
4+
* Manages canvas element refs for chunked virtual-scroll rendering.
5+
*
6+
* Provides a callback ref (for `ref={canvasRef}`) that stores canvases
7+
* by their `data-index` chunk index, and automatically cleans up refs
8+
* for canvases that have been unmounted by the virtualizer.
9+
*
10+
* Used by Channel, TimeScale, and SpectrogramChannel.
11+
*
12+
* @returns
13+
* - `canvasRef` — Callback ref to attach to each `<canvas data-index={i}>` element.
14+
* - `canvasMapRef` — Stable ref to a `Map<number, HTMLCanvasElement>` for iterating
15+
* over mounted canvases during draw effects.
16+
*/
17+
export function useChunkedCanvasRefs() {
18+
const canvasMapRef = useRef<Map<number, HTMLCanvasElement>>(new Map());
19+
20+
const canvasRef = useCallback((canvas: HTMLCanvasElement | null) => {
21+
if (canvas !== null) {
22+
const idx = parseInt(canvas.dataset.index!, 10);
23+
canvasMapRef.current.set(idx, canvas);
24+
}
25+
}, []);
26+
27+
// Clean up stale refs for unmounted chunks.
28+
// Intentionally has no dependency array — runs after every render because
29+
// the virtualizer can unmount canvases at any time, and drawing effects
30+
// need the map pruned before they iterate it.
31+
useEffect(() => {
32+
const map = canvasMapRef.current;
33+
for (const [idx, canvas] of map.entries()) {
34+
if (!canvas.isConnected) {
35+
map.delete(idx);
36+
}
37+
}
38+
});
39+
40+
return { canvasRef, canvasMapRef };
41+
}

0 commit comments

Comments
 (0)