diff --git a/.planning/phases/27-backing-track-sync/27-RESEARCH.md b/.planning/phases/27-backing-track-sync/27-RESEARCH.md new file mode 100644 index 000000000..38d4494b6 --- /dev/null +++ b/.planning/phases/27-backing-track-sync/27-RESEARCH.md @@ -0,0 +1,404 @@ +# Phase 27: Backing Track Sync - Research + +**Researched:** 2026-02-26 +**Domain:** React/Redux state management, media track synchronization +**Confidence:** HIGH + +## Summary + +This phase addresses two targeted fixes for backing track functionality in the JamKazam React frontend (jam-ui): + +1. **BT-01**: The backing track should appear in the session screen when opened, similar to how JamTrack stems appear. Currently, `handleBackingTrackSelected` in JKSessionScreen.js directly calls `jamClient.SessionOpenBackingTrackFile()` instead of using the Redux-based `openBackingTrack()` action from useMediaActions, which would enable proper track sync to the session screen. + +2. **BT-02**: The duration fetch `useEffect` in JKSessionBackingTrackPlayer.js can cause "state update on unmounted component" warnings when the backing track popup is closed quickly before the async duration fetch completes. This requires an ignore/cleanup flag pattern. + +The fixes are localized to two files: `JKSessionScreen.js` and `JKSessionBackingTrackPlayer.js`. Both are standard React/Redux patterns already used elsewhere in the codebase (see JamTrack handling for reference). + +**Primary recommendation:** Use existing `openBackingTrack()` from useMediaActions hook (already imported but unused for this purpose) and add a mounted/ignore flag pattern to the duration fetch useEffect. + +## Standard Stack + +This phase uses the existing stack - no new libraries required. + +### Core +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| React | 14.21.3 | UI framework | Already in use in jam-ui | +| Redux Toolkit | existing | State management | Already used for mediaSlice, activeSessionSlice | +| useMediaActions | existing hook | Media action dispatching | Provides `openBackingTrack` action | + +### Supporting +| Library | Version | Purpose | When to Use | +|---------|---------|---------|-------------| +| trackSyncService | existing | Server sync | Called automatically by openBackingTrack | +| mixersSlice | existing | Media summary state | Updated by openBackingTrack action | + +### Alternatives Considered +| Instead of | Could Use | Tradeoff | +|------------|-----------|----------| +| Mounted flag | AbortController | AbortController works for fetch, but jamClient calls are not standard fetch - mounted flag is simpler | +| useRef for ignore flag | useState | useRef avoids re-renders and works correctly in cleanup functions | + +## Architecture Patterns + +### Pattern 1: Redux Action for Media Operations (BT-01) + +**What:** Use Redux thunks for all media operations to ensure proper state synchronization +**When to use:** Opening any media (backing tracks, JamTracks, metronome) +**Current location of correct pattern:** JamTrack handling in JKSessionScreen.js (lines 1166-1207) + +**Current INCORRECT flow in handleBackingTrackSelected:** +```javascript +// JKSessionScreen.js - lines 1141-1158 +const handleBackingTrackSelected = async (result) => { + try { + // PROBLEM: Direct jamClient call bypasses Redux + await jamClient.SessionOpenBackingTrackFile(result.file, false); + + // Sets data for popup only, doesn't trigger track sync + dispatch(setBackingTrackData({...})); + dispatch(openModal('backingTrack')); + } catch (error) { + toast.error('Failed to open backing track'); + } +}; +``` + +**CORRECT flow (use openBackingTrack from useMediaActions):** +```javascript +// JKSessionScreen.js - corrected pattern +const handleBackingTrackSelected = async (result) => { + try { + // Use Redux action which: + // 1. Calls jamClient.SessionOpenBackingTrackFile + // 2. Updates mediaSummary (backingTrackOpen: true) + // 3. Triggers trackSyncService.syncTracksToServer() + await openBackingTrack(result.file); + + // Set data for popup + dispatch(setBackingTrackData({ + backingTrack: result.file, + session: currentSession, + currentUser: currentUser + })); + dispatch(openModal('backingTrack')); + } catch (error) { + toast.error('Failed to open backing track'); + } +}; +``` + +**Reference: openBackingTrack in useMediaActions.js (lines 47-66):** +```javascript +const openBackingTrack = useCallback(async (file) => { + try { + await dispatch(openBackingTrackThunk({ file, jamClient })).unwrap(); + + // Update media summary + dispatch(updateMediaSummary({ + backingTrackOpen: true, + userNeedsMediaControls: true + })); + + // Sync tracks to server after opening backing track + if (sessionId && jamClient) { + dispatch(syncTracksToServer(sessionId, jamClient)); + } + } catch (error) { + console.error('Error opening backing track:', error); + throw error; + } +}, [dispatch, jamClient, sessionId]); +``` + +### Pattern 2: Ignore Flag for Async Cleanup (BT-02) + +**What:** Prevent state updates on unmounted components in useEffect with async operations +**When to use:** Any useEffect that performs async operations and updates state +**Reference:** [Sentry article on this pattern](https://sentry.io/answers/how-to-fix-react-warning-can-t-perform-a-react-state-update-on-an-unmounted-component/) + +**Current PROBLEMATIC code in JKSessionBackingTrackPlayer.js (lines 90-143):** +```javascript +useEffect(() => { + const shouldInitialize = (isOpen || isPopup) && backingTrack && jamClient; + + if (shouldInitialize) { + setIsPlaying(false); + setCurrentTime('0:00'); + setCurrentPositionMs(0); + + const fetchDuration = async () => { + setIsLoadingDuration(true); + try { + // ... async jamClient calls ... + const durationInMs = await jamClient.SessionGetTracksPlayDurationMs(); + // PROBLEM: If component unmounts during await, these updates cause warnings + setDurationMs(validDuration); + setDuration(formatTime(validDuration)); + setIsLoadingDuration(false); + } catch (error) { + // PROBLEM: State updates can happen after unmount + setDuration('0:00'); + setDurationMs(0); + setIsLoadingDuration(false); + } + }; + + fetchDuration(); + } +}, [isOpen, isPopup, backingTrack, jamClient, handleError, formatTime, clearError]); +``` + +**CORRECTED pattern with ignore flag:** +```javascript +useEffect(() => { + let ignore = false; // Flag to track if effect should be ignored + + const shouldInitialize = (isOpen || isPopup) && backingTrack && jamClient; + + if (shouldInitialize) { + setIsPlaying(false); + setCurrentTime('0:00'); + setCurrentPositionMs(0); + + const fetchDuration = async () => { + setIsLoadingDuration(true); + try { + if (!jamClient) { + if (!ignore) { + handleError('general', 'Audio engine not available', null); + setIsLoadingDuration(false); + } + return; + } + + const durationInMs = await jamClient.SessionGetTracksPlayDurationMs(); + const validDuration = parseInt(durationInMs, 10) || 0; + + // Only update state if not ignored (component still mounted) + if (!ignore) { + if (validDuration === 0 || isNaN(validDuration)) { + handleError('file', 'Failed to load track duration. The file may be invalid or unsupported.', null); + setDuration('0:00'); + setDurationMs(0); + setIsLoadingDuration(false); + return; + } + + setDurationMs(validDuration); + setDuration(formatTime(validDuration)); + clearError(); + setIsLoadingDuration(false); + } + } catch (error) { + if (!ignore) { + handleError('file', 'Failed to load track duration', error); + setDuration('0:00'); + setDurationMs(0); + setIsLoadingDuration(false); + } + } + }; + + fetchDuration(); + } + + // Cleanup function - sets ignore flag when component unmounts or deps change + return () => { + ignore = true; + }; +}, [isOpen, isPopup, backingTrack, jamClient, handleError, formatTime, clearError]); +``` + +### Anti-Patterns to Avoid + +- **Direct jamClient calls for media operations:** Always use Redux actions to ensure proper state sync +- **Async state updates without cleanup:** Always check if component is still mounted before updating state +- **Using useState for mounted flag:** Use a local variable (`let ignore = false`) or useRef, not useState + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Media state sync | Custom jamClient + manual dispatch | `openBackingTrack()` from useMediaActions | Already handles all sync steps | +| Track sync to server | Manual API calls | `syncTracksToServer()` (auto-called by openBackingTrack) | Part of the media action flow | +| Unmount detection | useIsMounted hook | Local ignore flag in useEffect | Simpler, standard React pattern | + +**Key insight:** The codebase already has the correct patterns implemented for JamTrack - this phase is about using the same patterns for backing track. + +## Common Pitfalls + +### Pitfall 1: Forgetting to Use Redux Action +**What goes wrong:** Backing track opens in popup but doesn't appear in session screen +**Why it happens:** Direct `jamClient.SessionOpenBackingTrackFile()` call bypasses Redux state updates +**How to avoid:** Always use `openBackingTrack()` from useMediaActions hook +**Warning signs:** Track plays in popup but no track appears in session screen; missing in mixerHelper.backingTracks + +### Pitfall 2: Race Condition on Quick Close +**What goes wrong:** Console warning: "Can't perform a React state update on an unmounted component" +**Why it happens:** User closes backing track popup before async duration fetch completes +**How to avoid:** Add ignore flag to useEffect with cleanup function +**Warning signs:** Console warnings when opening and quickly closing backing track + +### Pitfall 3: Missing Track Sync After Open +**What goes wrong:** Backing track opens locally but doesn't sync to server/other participants +**Why it happens:** `syncTracksToServer()` not called after opening track +**How to avoid:** `openBackingTrack()` action automatically calls syncTracksToServer +**Warning signs:** Other session participants don't see the backing track + +## Code Examples + +### BT-01: Use openBackingTrack Action + +**File:** `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionScreen.js` +**Location:** `handleBackingTrackSelected` function (around line 1137) + +```javascript +// BEFORE (incorrect - direct jamClient call) +const handleBackingTrackSelected = async (result) => { + try { + await jamClient.SessionOpenBackingTrackFile(result.file, false); + dispatch(setBackingTrackData({ + backingTrack: result.file, + session: currentSession, + currentUser: currentUser + })); + dispatch(openModal('backingTrack')); + } catch (error) { + toast.error('Failed to open backing track'); + } +}; + +// AFTER (correct - using Redux action) +const handleBackingTrackSelected = async (result) => { + try { + // Use the openBackingTrack action from useMediaActions + // This is already imported at line 153: const { openBackingTrack, ... } = useMediaActions(); + await openBackingTrack(result.file); + + // Set popup data (same as before) + dispatch(setBackingTrackData({ + backingTrack: result.file, + session: currentSession, + currentUser: currentUser + })); + dispatch(openModal('backingTrack')); + } catch (error) { + toast.error('Failed to open backing track'); + } +}; +``` + +### BT-02: Add Ignore Flag to Duration Fetch + +**File:** `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionBackingTrackPlayer.js` +**Location:** Duration fetch useEffect (around line 90) + +```javascript +// BEFORE (can cause unmounted component warnings) +useEffect(() => { + const shouldInitialize = (isOpen || isPopup) && backingTrack && jamClient; + if (shouldInitialize) { + // ... state updates ... + const fetchDuration = async () => { + // ... async operations that update state ... + }; + fetchDuration(); + } +}, [isOpen, isPopup, backingTrack, jamClient, handleError, formatTime, clearError]); + +// AFTER (with ignore flag for cleanup) +useEffect(() => { + let ignore = false; + + const shouldInitialize = (isOpen || isPopup) && backingTrack && jamClient; + if (shouldInitialize) { + setIsPlaying(false); + setCurrentTime('0:00'); + setCurrentPositionMs(0); + + const fetchDuration = async () => { + // Guard early exit state updates + if (ignore) return; + setIsLoadingDuration(true); + + try { + if (!jamClient) { + if (!ignore) { + handleError('general', 'Audio engine not available', null); + setIsLoadingDuration(false); + } + return; + } + + const durationInMs = await jamClient.SessionGetTracksPlayDurationMs(); + + // Check ignore before all state updates + if (ignore) return; + + const validDuration = parseInt(durationInMs, 10) || 0; + if (validDuration === 0 || isNaN(validDuration)) { + handleError('file', 'Failed to load track duration. The file may be invalid or unsupported.', null); + setDuration('0:00'); + setDurationMs(0); + setIsLoadingDuration(false); + return; + } + + setDurationMs(validDuration); + setDuration(formatTime(validDuration)); + clearError(); + setIsLoadingDuration(false); + } catch (error) { + if (!ignore) { + handleError('file', 'Failed to load track duration', error); + setDuration('0:00'); + setDurationMs(0); + setIsLoadingDuration(false); + } + } + }; + + fetchDuration(); + } + + return () => { + ignore = true; + }; +}, [isOpen, isPopup, backingTrack, jamClient, handleError, formatTime, clearError]); +``` + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| Direct jamClient calls | Redux thunks (mediaSlice) | Already in codebase | Enables state sync | +| No unmount handling | Ignore flag pattern | React best practice | Prevents warnings | + +**Note on React 18+:** The "state update on unmounted component" warning was removed in React 18 as it caused more confusion than benefit. However, jam-ui uses React 14.21.3, so the fix is still relevant. + +## Open Questions + +None - the requirements are clear and the patterns are already established in the codebase. + +## Sources + +### Primary (HIGH confidence) +- `/Users/nuwan/Code/jam-cloud/jam-ui/src/hooks/useMediaActions.js` - openBackingTrack implementation +- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionScreen.js` - current handleBackingTrackSelected +- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionBackingTrackPlayer.js` - duration fetch useEffect +- `/Users/nuwan/Code/jam-cloud/jam-ui/src/store/features/mediaSlice.js` - openBackingTrack thunk + +### Secondary (MEDIUM confidence) +- [Sentry: Fix state update on unmounted component](https://sentry.io/answers/how-to-fix-react-warning-can-t-perform-a-react-state-update-on-an-unmounted-component/) - ignore flag pattern +- [React 18 Discussion on warning removal](https://github.com/reactwg/react-18/discussions/82) - context for why warning exists + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH - using existing codebase patterns +- Architecture: HIGH - patterns already exist for JamTrack, just applying to backing track +- Pitfalls: HIGH - based on actual codebase analysis and React best practices + +**Research date:** 2026-02-26 +**Valid until:** N/A - these are stable patterns in the existing codebase