Skip to content

Commit 6b3e2b2

Browse files
authored
Merge pull request #277 from naomiaro/chore-react-ts-audit-refactors
React/TypeScript audit: lint baseline, provider refactor, spectrogram fix
2 parents 914c22a + d888b03 commit 6b3e2b2

37 files changed

+763
-185
lines changed

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Agent Instructions
2+
3+
Primary project guidance is maintained in `CLAUDE.MD`.
4+
5+
- Read `CLAUDE.MD` before making changes.
6+
- Treat `CLAUDE.MD` as the source of truth for architecture and conventions.

CLAUDE.MD

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,50 @@ const LazyExample = createLazyExample(
401401

402402
**Documentation:** `website/docs/effects.md`
403403

404+
### Shared Animation Frame Loop Hook (Provider Refactor, 2026-02-13)
405+
406+
**Decision:** Centralize requestAnimationFrame lifecycle logic in a shared hook used by both playlist providers.
407+
408+
**Implementation:**
409+
- New hook: `packages/browser/src/hooks/useAnimationFrameLoop.ts`
410+
- Exported from: `packages/browser/src/hooks/index.ts`
411+
- Integrated into:
412+
- `packages/browser/src/WaveformPlaylistContext.tsx`
413+
- `packages/browser/src/MediaElementPlaylistContext.tsx`
414+
415+
**Why:**
416+
- Removes duplicated `requestAnimationFrame` / `cancelAnimationFrame` logic across providers
417+
- Ensures a single in-flight animation frame per provider
418+
- Standardizes cleanup on unmount and playback transitions
419+
420+
### SpectrogramChannel Hook Stability (2026-02-13)
421+
422+
**Decision:** Use stable default references for LUT/scale and remove hook dependency suppression.
423+
424+
**Implementation:** `packages/ui-components/src/components/SpectrogramChannel.tsx`
425+
- Hoisted stable defaults:
426+
- `DEFAULT_COLOR_LUT`
427+
- `LINEAR_FREQUENCY_SCALE`
428+
- Updated effect dependencies to include worker/callback references explicitly
429+
- Removed `react-hooks/exhaustive-deps` suppression
430+
431+
**Why:**
432+
- Prevents unnecessary redraw/recompute caused by new inline default references each render
433+
- Reduces stale-closure risk in worker canvas registration effect
434+
435+
### ESLint Baseline (2026-02-13)
436+
437+
**Decision:** Add a root flat ESLint config with TypeScript + React Hooks checks.
438+
439+
**Implementation:**
440+
- Config file: `eslint.config.mjs`
441+
- Root `package.json` devDependencies include:
442+
- `eslint`, `@eslint/js`
443+
- `@typescript-eslint/parser`, `@typescript-eslint/eslint-plugin`
444+
- `eslint-plugin-react-hooks`, `globals`
445+
446+
**Status:** TypeScript checks pass (`pnpm typecheck`). Lint execution is currently blocked in offline environments until dependencies are installable (`pnpm install` needed when network is available).
447+
404448
---
405449

406450
## Recording Architecture

eslint.config.mjs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import js from '@eslint/js';
2+
import tsParser from '@typescript-eslint/parser';
3+
import tsPlugin from '@typescript-eslint/eslint-plugin';
4+
import reactHooksPlugin from 'eslint-plugin-react-hooks';
5+
import globals from 'globals';
6+
7+
export default [
8+
{
9+
ignores: [
10+
'**/node_modules/**',
11+
'**/dist/**',
12+
'**/build/**',
13+
'**/.docusaurus/**',
14+
'**/playwright-report/**',
15+
'**/test-results/**',
16+
'**/*.d.ts',
17+
],
18+
},
19+
{
20+
files: ['packages/**/src/**/*.{ts,tsx}', 'e2e/**/*.ts', 'website/src/**/*.{ts,tsx}'],
21+
languageOptions: {
22+
parser: tsParser,
23+
parserOptions: {
24+
ecmaVersion: 'latest',
25+
sourceType: 'module',
26+
ecmaFeatures: {
27+
jsx: true,
28+
},
29+
},
30+
globals: {
31+
...globals.browser,
32+
...globals.node,
33+
},
34+
},
35+
plugins: {
36+
'@typescript-eslint': tsPlugin,
37+
'react-hooks': reactHooksPlugin,
38+
},
39+
rules: {
40+
...js.configs.recommended.rules,
41+
...tsPlugin.configs.recommended.rules,
42+
'no-undef': 'off',
43+
'no-unused-vars': 'off',
44+
'react-hooks/rules-of-hooks': 'error',
45+
'react-hooks/exhaustive-deps': 'error',
46+
'@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }],
47+
'@typescript-eslint/no-explicit-any': 'warn',
48+
},
49+
},
50+
{
51+
files: ['**/*.{js,mjs,cjs}'],
52+
languageOptions: {
53+
globals: {
54+
...globals.node,
55+
},
56+
},
57+
rules: {
58+
...js.configs.recommended.rules,
59+
},
60+
},
61+
{
62+
files: ['**/*.stories.{ts,tsx}'],
63+
rules: {
64+
'react-hooks/rules-of-hooks': 'off',
65+
'react-hooks/exhaustive-deps': 'off',
66+
},
67+
},
68+
];

package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,14 @@
2121
"version:bump": "node scripts/bump-version.js"
2222
},
2323
"devDependencies": {
24+
"@eslint/js": "^9.20.0",
2425
"@playwright/test": "^1.57.0",
26+
"@typescript-eslint/eslint-plugin": "^8.24.1",
27+
"@typescript-eslint/parser": "^8.24.1",
2528
"@types/node": "^20.10.6",
29+
"eslint": "^9.20.1",
30+
"eslint-plugin-react-hooks": "^5.2.0",
31+
"globals": "^15.15.0",
2632
"prettier": "^3.1.1",
2733
"rollup-plugin-visualizer": "^6.0.5",
2834
"typescript": "^5.3.3"

packages/annotations/src/hooks/useAnnotationControls.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useState, useCallback } from 'react';
2-
import type { AnnotationData, AnnotationListOptions } from '../types';
2+
import type { AnnotationData } from '../types';
33

44
const LINK_THRESHOLD = 0.01; // Consider edges "linked" if within 10ms
55

packages/browser/src/MediaElementPlaylistContext.tsx

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import React, {
1111
import { ThemeProvider } from 'styled-components';
1212
import {
1313
MediaElementPlayout,
14-
type MediaElementTrackOptions,
1514
} from '@waveform-playlist/media-element-playout';
1615
import { type WaveformDataObject } from '@waveform-playlist/core';
1716
import {
@@ -23,6 +22,7 @@ import { extractPeaksFromWaveformData } from './waveformDataLoader';
2322
import type WaveformData from 'waveform-data';
2423
import type { PeakData } from '@waveform-playlist/webaudio-peaks';
2524
import type { ClipPeaks, TrackClipPeaks } from './WaveformPlaylistContext';
25+
import { useAnimationFrameLoop } from './hooks/useAnimationFrameLoop';
2626

2727
// Configuration for a single media element track
2828
export interface MediaElementTrackConfig {
@@ -202,12 +202,12 @@ export const MediaElementPlaylistProvider: React.FC<
202202
// Refs
203203
const playoutRef = useRef<MediaElementPlayout | null>(null);
204204
const currentTimeRef = useRef<number>(0);
205-
const animationFrameRef = useRef<number | null>(null);
206205
const continuousPlayRef = useRef<boolean>(continuousPlay);
207206
const activeAnnotationIdRef = useRef<string | null>(null);
208207
const scrollContainerRef = useRef<HTMLDivElement | null>(null);
209208
const isAutomaticScrollRef = useRef<boolean>(automaticScroll);
210209
const samplesPerPixelRef = useRef<number>(initialSamplesPerPixel);
210+
const { startAnimationFrameLoop, stopAnimationFrameLoop } = useAnimationFrameLoop();
211211

212212
// Sync refs
213213
useEffect(() => {
@@ -259,10 +259,7 @@ export const MediaElementPlaylistProvider: React.FC<
259259

260260
// Set up playback complete callback
261261
playout.setOnPlaybackComplete(() => {
262-
if (animationFrameRef.current) {
263-
cancelAnimationFrame(animationFrameRef.current);
264-
animationFrameRef.current = null;
265-
}
262+
stopAnimationFrameLoop();
266263
setIsPlaying(false);
267264
setActiveAnnotationId(null);
268265
currentTimeRef.current = 0;
@@ -274,12 +271,10 @@ export const MediaElementPlaylistProvider: React.FC<
274271
onReady?.();
275272

276273
return () => {
277-
if (animationFrameRef.current) {
278-
cancelAnimationFrame(animationFrameRef.current);
279-
}
274+
stopAnimationFrameLoop();
280275
playout.dispose();
281276
};
282-
}, [track.source, track.waveformData, track.name, initialPlaybackRate, onReady]);
277+
}, [track.source, track.waveformData, track.name, initialPlaybackRate, onReady, stopAnimationFrameLoop, setActiveAnnotationId]);
283278

284279
// Generate peaks from waveform data
285280
useEffect(() => {
@@ -308,10 +303,6 @@ export const MediaElementPlaylistProvider: React.FC<
308303

309304
// Animation loop
310305
const startAnimationLoop = useCallback(() => {
311-
if (animationFrameRef.current) {
312-
cancelAnimationFrame(animationFrameRef.current);
313-
}
314-
315306
const updateTime = () => {
316307
const time = playoutRef.current?.getCurrentTime() ?? 0;
317308
currentTimeRef.current = time;
@@ -368,18 +359,13 @@ export const MediaElementPlaylistProvider: React.FC<
368359
container.scrollLeft = targetScrollLeft;
369360
}
370361

371-
animationFrameRef.current = requestAnimationFrame(updateTime);
362+
startAnimationFrameLoop(updateTime);
372363
};
373364

374-
animationFrameRef.current = requestAnimationFrame(updateTime);
375-
}, [setActiveAnnotationId, sampleRate, controls]);
365+
startAnimationFrameLoop(updateTime);
366+
}, [setActiveAnnotationId, sampleRate, controls, startAnimationFrameLoop]);
376367

377-
const stopAnimationLoop = useCallback(() => {
378-
if (animationFrameRef.current) {
379-
cancelAnimationFrame(animationFrameRef.current);
380-
animationFrameRef.current = null;
381-
}
382-
}, []);
368+
const stopAnimationLoop = stopAnimationFrameLoop;
383369

384370
// Playback controls
385371
const play = useCallback(

packages/browser/src/WaveformPlaylistContext.tsx

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { extractPeaksFromWaveformData } from './waveformDataLoader';
1010
import type WaveformData from 'waveform-data';
1111
import type { PeakData } from '@waveform-playlist/webaudio-peaks';
1212
import type { AnnotationData } from '@waveform-playlist/core';
13-
import { useTimeFormat, useZoomControls, useMasterVolume } from './hooks';
13+
import { useTimeFormat, useZoomControls, useMasterVolume, useAnimationFrameLoop } from './hooks';
1414

1515
// Types
1616
export interface ClipPeaks {
@@ -254,7 +254,6 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
254254
const playoutRef = useRef<TonePlayout | null>(null);
255255
const playStartPositionRef = useRef<number>(0);
256256
const currentTimeRef = useRef<number>(0);
257-
const animationFrameRef = useRef<number | null>(null);
258257
const trackStatesRef = useRef<TrackState[]>(trackStates);
259258
const playbackStartTimeRef = useRef<number>(0); // context.currentTime when playback started
260259
const audioStartPositionRef = useRef<number>(0); // Audio position when playback started
@@ -275,6 +274,7 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
275274
const zoom = useZoomControls({ initialSamplesPerPixel, zoomLevels });
276275
const samplesPerPixel = zoom.samplesPerPixel;
277276
const { masterVolume, setMasterVolume } = useMasterVolume({ playoutRef, initialVolume: 1.0 });
277+
const { animationFrameRef, startAnimationFrameLoop, stopAnimationFrameLoop } = useAnimationFrameLoop();
278278

279279
// Custom setter for continuousPlay that updates BOTH state and ref synchronously
280280
// This ensures the ref is updated immediately, before the animation loop can read it
@@ -384,10 +384,7 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
384384
// Stop current playback and animation before disposing
385385
if (playoutRef.current && wasPlaying) {
386386
playoutRef.current.stop();
387-
if (animationFrameRef.current) {
388-
cancelAnimationFrame(animationFrameRef.current);
389-
animationFrameRef.current = null;
390-
}
387+
stopAnimationFrameLoop();
391388
// Mark that we need to resume playback after playout is rebuilt
392389
pendingResumeRef.current = { position: resumePosition };
393390
}
@@ -524,14 +521,12 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
524521
loadAudio();
525522

526523
return () => {
527-
if (animationFrameRef.current) {
528-
cancelAnimationFrame(animationFrameRef.current);
529-
}
524+
stopAnimationFrameLoop();
530525
if (playoutRef.current) {
531526
playoutRef.current.dispose();
532527
}
533528
};
534-
}, [tracks, onReady, isPlaying]);
529+
}, [tracks, onReady, isPlaying, effects, stopAnimationFrameLoop]);
535530

536531
// Regenerate peaks when zoom or mono changes (without reloading audio)
537532
useEffect(() => {
@@ -617,12 +612,6 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
617612

618613
// Animation loop
619614
const startAnimationLoop = useCallback(() => {
620-
// Cancel any existing animation frame before starting a new one
621-
if (animationFrameRef.current) {
622-
cancelAnimationFrame(animationFrameRef.current);
623-
animationFrameRef.current = null;
624-
}
625-
626615
const updateTime = () => {
627616
// Calculate current position based on context.currentTime timing
628617
const elapsed = getContext().currentTime - playbackStartTimeRef.current;
@@ -720,7 +709,7 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
720709
playoutRef.current?.play(timeNow, loopStartRef.current);
721710

722711
// Continue animation loop
723-
animationFrameRef.current = requestAnimationFrame(updateTime);
712+
startAnimationFrameLoop(updateTime);
724713
return;
725714
}
726715
}
@@ -736,17 +725,12 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
736725
setActiveAnnotationId(null);
737726
return;
738727
}
739-
animationFrameRef.current = requestAnimationFrame(updateTime);
728+
startAnimationFrameLoop(updateTime);
740729
};
741-
animationFrameRef.current = requestAnimationFrame(updateTime);
742-
}, [duration, audioBuffers, samplesPerPixel, continuousPlay]);
730+
startAnimationFrameLoop(updateTime);
731+
}, [duration, audioBuffers, controls.show, controls.width, setActiveAnnotationId, startAnimationFrameLoop]);
743732

744-
const stopAnimationLoop = useCallback(() => {
745-
if (animationFrameRef.current) {
746-
cancelAnimationFrame(animationFrameRef.current);
747-
animationFrameRef.current = null;
748-
}
749-
}, []);
733+
const stopAnimationLoop = stopAnimationFrameLoop;
750734

751735
// Restart animation loop and reschedule playout when continuousPlay changes during playback
752736
// This ensures the loop always has the current continuousPlay value
@@ -786,7 +770,7 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
786770
};
787771

788772
reschedulePlayback();
789-
}, [continuousPlay, isPlaying, startAnimationLoop, stopAnimationLoop]);
773+
}, [continuousPlay, isPlaying, startAnimationLoop, stopAnimationLoop, animationFrameRef]);
790774

791775
// Resume playback after tracks change (e.g., after splitting a clip during playback)
792776
useEffect(() => {
@@ -878,7 +862,7 @@ export const WaveformPlaylistProvider: React.FC<WaveformPlaylistProviderProps> =
878862
currentTimeRef.current = playStartPositionRef.current;
879863
setCurrentTime(playStartPositionRef.current);
880864
setActiveAnnotationId(null);
881-
}, [stopAnimationLoop]);
865+
}, [stopAnimationLoop, setActiveAnnotationId]);
882866

883867
// Seek to a specific time - works whether playing or stopped
884868
const seekTo = useCallback((time: number) => {
@@ -1154,4 +1138,3 @@ export const usePlaylistData = () => {
11541138
}
11551139
return context;
11561140
};
1157-

packages/browser/src/components/MediaElementPlaylist.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export const MediaElementPlaylist: React.FC<MediaElementPlaylistProps> = ({
6363
const theme = useTheme() as import('@waveform-playlist/ui-components').WaveformPlaylistTheme;
6464

6565
// MediaElement context hooks
66-
const { isPlaying, currentTimeRef } = useMediaElementAnimation();
66+
const { isPlaying } = useMediaElementAnimation();
6767
const { annotations, activeAnnotationId } = useMediaElementState();
6868
const annotationIntegration = useContext(AnnotationIntegrationContext);
6969
const { play, seekTo, setActiveAnnotationId, setAnnotations, setScrollContainer } = useMediaElementControls();

packages/browser/src/components/PlaylistVisualization.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export const PlaylistVisualization: React.FC<PlaylistVisualizationProps> = ({
8787
renderTrackControls,
8888
renderTimestamp,
8989
renderPlayhead,
90-
annotationControls,
90+
annotationControls: _annotationControls,
9191
getAnnotationBoxLabel,
9292
className,
9393
showClipHeaders = false,
@@ -106,7 +106,7 @@ export const PlaylistVisualization: React.FC<PlaylistVisualizationProps> = ({
106106
annotations,
107107
activeAnnotationId,
108108
annotationsEditable,
109-
linkEndpoints,
109+
linkEndpoints: _linkEndpoints,
110110
continuousPlay,
111111
selectedTrackId,
112112
loopStart,
@@ -115,7 +115,7 @@ export const PlaylistVisualization: React.FC<PlaylistVisualizationProps> = ({
115115
} = usePlaylistState();
116116
const annotationIntegration = useContext(AnnotationIntegrationContext);
117117
const {
118-
setAnnotations,
118+
setAnnotations: _setAnnotations,
119119
setActiveAnnotationId,
120120
setTrackMute,
121121
setTrackSolo,

0 commit comments

Comments
 (0)