docs(27): research backing track sync domain
Phase 27: Backing Track Sync - Standard stack identified (existing React/Redux patterns) - Architecture patterns documented (openBackingTrack action, ignore flag) - Pitfalls catalogued (bypassing Redux, race conditions) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
3d847ece7f
commit
12ce12420f
|
|
@ -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
|
||||
Loading…
Reference in New Issue