diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 30d0a9745..8c0f64f6b 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -92,10 +92,13 @@ Plans: 3. Mixer categorization doesn't dispatch when content unchanged 4. Loading states moved to components that use them 5. JKSessionScreen re-render count reduced by 50%+ -**Plans**: TBD +**Plans**: 4 plans Plans: -- [ ] 32-01: TBD +- [ ] 32-01-PLAN.md — Consolidate track sync and fix useSessionModel debounce recreation +- [ ] 32-02-PLAN.md — Add content comparison before mixer category dispatches +- [ ] 32-03-PLAN.md — Create JKResyncButton and JKVideoButton with colocated loading state +- [ ] 32-04-PLAN.md — Audit remaining state and verify re-render reduction ## Progress @@ -107,7 +110,7 @@ Plans: | 29. Context Optimization | v1.7 | 1/1 | ✓ Complete | 2026-03-05 | | 30. Component Memoization | v1.7 | 1/1 | ✓ Complete | 2026-03-05 | | 31. Selector Optimization | v1.7 | 1/1 | ✓ Complete | 2026-03-05 | -| 32. State Update Optimization | v1.7 | 0/TBD | Not started | - | +| 32. State Update Optimization | v1.7 | 0/4 | Planned | - | --- *v1.7 roadmap created 2026-03-03* diff --git a/.planning/phases/32-state-update-optimization/32-01-PLAN.md b/.planning/phases/32-state-update-optimization/32-01-PLAN.md new file mode 100644 index 000000000..f6ba486ae --- /dev/null +++ b/.planning/phases/32-state-update-optimization/32-01-PLAN.md @@ -0,0 +1,294 @@ +--- +phase: 32-state-update-optimization +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - jam-ui/src/components/client/JKSessionScreen.js + - jam-ui/src/hooks/useSessionModel.js + - jam-ui/src/hooks/useDebounceCallback.js +autonomous: true + +must_haves: + truths: + - "Track sync fires once per session join (not 3 times)" + - "Debounce timer in trackChanges is stable across state changes" + - "Debounced callbacks read fresh state values" + artifacts: + - path: "jam-ui/src/hooks/useDebounceCallback.js" + provides: "Reusable ref-based debounce hook" + exports: ["useDebounceCallback"] + - path: "jam-ui/src/components/client/JKSessionScreen.js" + provides: "Single debounced track sync" + contains: "syncTracksDebounced" + - path: "jam-ui/src/hooks/useSessionModel.js" + provides: "Stable trackChanges debounce" + contains: "useDebounceCallback" + key_links: + - from: "jam-ui/src/components/client/JKSessionScreen.js" + to: "syncTracksToServer" + via: "single debounced call" + pattern: "useMemo.*debounce.*syncTracksToServer" + - from: "jam-ui/src/hooks/useSessionModel.js" + to: "useDebounceCallback" + via: "import and usage" + pattern: "useDebounceCallback.*trackChanges" +--- + + +Fix redundant track sync calls and debounce instance recreation + +Purpose: Eliminate triple API calls on session join (STATE-01) and fix debounce timer reset on state changes (STATE-02). These are the two remaining redundant operation patterns identified in the investigation phase. + +Output: Single debounced track sync call, stable useDebounceCallback hook, refactored trackChanges handler + + + +@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md +@/Users/nuwan/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/32-state-update-optimization/32-RESEARCH.md + +# Source files to modify +@jam-ui/src/components/client/JKSessionScreen.js +@jam-ui/src/hooks/useSessionModel.js + + + + + + Task 1: Create useDebounceCallback hook + jam-ui/src/hooks/useDebounceCallback.js + +Create a new reusable hook that solves the stale closure problem with debounced callbacks: + +```javascript +import { useRef, useEffect, useMemo } from 'react'; +import { debounce } from 'lodash'; + +/** + * Hook that creates a stable debounced callback with fresh closure access. + * Solves the problem of debounce recreation when dependencies change. + * + * Pattern: useRef stores latest callback, useMemo creates debounce once. + * Source: https://www.developerway.com/posts/debouncing-in-react + * + * @param {Function} callback - The callback to debounce + * @param {number} delay - Debounce delay in ms (default: 500) + * @returns {Function} Debounced function that always uses latest callback + */ +export const useDebounceCallback = (callback, delay = 500) => { + const callbackRef = useRef(callback); + + // Always keep ref current with latest callback + useEffect(() => { + callbackRef.current = callback; + }, [callback]); + + // Create debounced function once - never recreated + const debouncedFn = useMemo(() => + debounce((...args) => { + callbackRef.current?.(...args); + }, delay), + [delay] // Only recreate if delay changes + ); + + // Cleanup on unmount + useEffect(() => { + return () => debouncedFn.cancel(); + }, [debouncedFn]); + + return debouncedFn; +}; + +export default useDebounceCallback; +``` + +Key design decisions: +- Uses useRef to store callback, keeping it always current +- Uses useMemo with [delay] deps to create debounce function once +- Handles cleanup on unmount via useEffect return +- Exports both named and default for flexibility + + +File exists at jam-ui/src/hooks/useDebounceCallback.js +Exports useDebounceCallback function +No ESLint errors: `cd jam-ui && npx eslint src/hooks/useDebounceCallback.js` + + +useDebounceCallback hook created with ref-based pattern, ready for import in useSessionModel + + + + + Task 2: Consolidate track sync to single debounced call + jam-ui/src/components/client/JKSessionScreen.js + +Replace the triple setTimeout pattern (lines ~462-485) with a single debounced call. + +1. Add import for useMemo at top (if not already present) + +2. Find the current track sync useEffect (around lines 453-485): +```javascript +// Track sync: Sync tracks to server when session joined (3-call pattern matching legacy) +useEffect(() => { + if (!hasJoined || !sessionId || !server?.clientId || !mixersReady) { + return; + } + const timer1 = setTimeout(() => { ... }, 1000); + const timer2 = setTimeout(() => { ... }, 1400); + const timer3 = setTimeout(() => { ... }, 6000); + return () => { clearTimeout(timer1); ... }; +}, [hasJoined, sessionId, mixersReady, dispatch]) +``` + +3. Replace with single debounced call pattern: +```javascript +// Create stable debounced sync function +const syncTracksDebounced = useMemo(() => + debounce((sid, cid, d) => { + d(syncTracksToServer(sid, cid)); + }, 1500), // 1.5s delay - adequate for mixer initialization + [] +); + +// Track sync: Single debounced call when session joined +// Replaces legacy 3-timer pattern (1s, 1.4s, 6s) with single 1.5s debounced call +useEffect(() => { + if (!hasJoined || !sessionId || !server?.clientId || !mixersReady) { + return; + } + + // console.log('[Track Sync] Mixers ready, scheduling single debounced sync'); + syncTracksDebounced(sessionId, server.clientId, dispatch); + + return () => syncTracksDebounced.cancel(); + // Note: server intentionally NOT in deps to avoid re-running on server reference changes + // eslint-disable-next-line react-hooks/exhaustive-deps +}, [hasJoined, sessionId, mixersReady, dispatch, syncTracksDebounced]); +``` + +4. Add lodash debounce import if not present: +```javascript +import { debounce } from 'lodash'; +``` + +Why 1.5s delay: Legacy used 1s, 1.4s, 6s. The first two calls at 1s and 1.4s were redundant. A single 1.5s debounced call covers the initial mixer setup window while preventing rapid repeated calls. The 6s call was likely for slow networks - if issues arise, this delay can be increased. + + +Check the track sync useEffect has single debounce call: +`grep -A 20 "Track sync:" jam-ui/src/components/client/JKSessionScreen.js` + +Verify no setTimeout pattern remains in that section: +`grep -A 20 "Track sync:" jam-ui/src/components/client/JKSessionScreen.js | grep -c setTimeout` should be 0 + +ESLint passes: `cd jam-ui && npx eslint src/components/client/JKSessionScreen.js --max-warnings=0` + + +Track sync useEffect uses single debounced call instead of 3 setTimeout calls + + + + + Task 3: Fix trackChanges debounce in useSessionModel + jam-ui/src/hooks/useSessionModel.js + +Replace the useCallback-wrapped debounce with useDebounceCallback hook. + +1. Add import at top: +```javascript +import { useDebounceCallback } from './useDebounceCallback'; +``` + +2. Find current trackChanges (around lines 604-614): +```javascript +// Track changes handler - debounced to prevent excessive session refreshes +const trackChanges = useCallback(debounce((header, payload) => { + if (currentTrackChanges < payload.track_changes_counter) { + logger.debug("track_changes_counter = stale. refreshing..."); + refreshCurrentSession(); + } else { + if (header.type !== 'HEARTBEAT_ACK') { + logger.info("track_changes_counter = fresh. skipping refresh...", header, payload); + } + } +}, 500), [currentTrackChanges, refreshCurrentSession]); +``` + +3. Replace with useDebounceCallback pattern: +```javascript +// Track changes handler - debounced to prevent excessive session refreshes +// Uses useDebounceCallback for stable timer (doesn't reset when deps change) +const trackChanges = useDebounceCallback((header, payload) => { + if (currentTrackChanges < payload.track_changes_counter) { + logger.debug("track_changes_counter = stale. refreshing..."); + refreshCurrentSession(); + } else { + if (header.type !== 'HEARTBEAT_ACK') { + logger.info("track_changes_counter = fresh. skipping refresh...", header, payload); + } + } +}, 500); +``` + +Key changes: +- Remove useCallback wrapper (useDebounceCallback handles memoization internally) +- Remove debounce import call (it's inside useDebounceCallback) +- Remove [currentTrackChanges, refreshCurrentSession] dependency array +- The callback now always reads fresh values via ref closure +- Timer is stable - won't reset on currentTrackChanges updates + +4. Remove lodash debounce import if no longer used elsewhere in file: +Check for other usages first: `grep -n "debounce" useSessionModel.js` +If trackChanges was the only use, remove: `import { debounce } from 'lodash';` +If other usages exist, keep the import. + + +trackChanges now uses useDebounceCallback: +`grep -B 2 -A 10 "trackChanges = " jam-ui/src/hooks/useSessionModel.js | head -15` + +No useCallback wrapper on trackChanges: +`grep "useCallback(debounce" jam-ui/src/hooks/useSessionModel.js | wc -l` should be 0 + +ESLint passes: `cd jam-ui && npx eslint src/hooks/useSessionModel.js --max-warnings=0` + + +trackChanges uses useDebounceCallback - timer stable across state changes, always reads fresh values + + + + + + +1. Track sync fires once per session join: + - Grep for setTimeout in track sync section should return 0 + - useMemo + debounce pattern present + +2. Debounce stability verified: + - useDebounceCallback hook exists with ref pattern + - trackChanges in useSessionModel uses useDebounceCallback + - No useCallback(debounce) pattern in useSessionModel + +3. All files pass ESLint: + ```bash + cd jam-ui && npx eslint src/hooks/useDebounceCallback.js src/hooks/useSessionModel.js src/components/client/JKSessionScreen.js --max-warnings=0 + ``` + + + +- [ ] useDebounceCallback.js created with ref-based debounce pattern +- [ ] JKSessionScreen track sync uses single debounced call (not 3 setTimeout) +- [ ] useSessionModel trackChanges uses useDebounceCallback (no dependency array recreation) +- [ ] All modified files pass ESLint +- [ ] No regressions - session join still works (manual verification if time permits) + + + +After completion, create `.planning/phases/32-state-update-optimization/32-01-SUMMARY.md` + diff --git a/.planning/phases/32-state-update-optimization/32-02-PLAN.md b/.planning/phases/32-state-update-optimization/32-02-PLAN.md new file mode 100644 index 000000000..79d9f48c8 --- /dev/null +++ b/.planning/phases/32-state-update-optimization/32-02-PLAN.md @@ -0,0 +1,244 @@ +--- +phase: 32-state-update-optimization +plan: 02 +type: execute +wave: 1 +depends_on: [] +files_modified: + - jam-ui/src/hooks/useMixerHelper.js +autonomous: true + +must_haves: + truths: + - "Mixer categorization doesn't dispatch when content unchanged" + - "Redux state only updates when mixer arrays meaningfully change" + - "Unchanged mixer list doesn't trigger downstream re-renders" + artifacts: + - path: "jam-ui/src/hooks/useMixerHelper.js" + provides: "Content comparison before dispatch" + contains: "mixerArraysEqual" + key_links: + - from: "jam-ui/src/hooks/useMixerHelper.js" + to: "setMetronomeTrackMixers" + via: "conditional dispatch" + pattern: "if.*!mixerArraysEqual.*dispatch" +--- + + +Prevent redundant Redux dispatches in mixer categorization + +Purpose: Stop dispatching mixer category arrays when their content hasn't changed (STATE-03). Currently, every mixer state change triggers 5 category dispatches even if the categories haven't changed, causing unnecessary re-renders. + +Output: useMixerHelper with content comparison before each category dispatch + + + +@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md +@/Users/nuwan/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/32-state-update-optimization/32-RESEARCH.md + +# Source file to modify +@jam-ui/src/hooks/useMixerHelper.js +@jam-ui/src/store/features/mixersSlice.js + + + + + + Task 1: Add mixer array comparison helper + jam-ui/src/hooks/useMixerHelper.js + +Add a helper function to compare mixer arrays by their IDs. Place this near the top of the file, after imports but before the hook function. + +```javascript +/** + * Compare two mixer arrays by their IDs to determine if content changed. + * Used to prevent unnecessary Redux dispatches when categorizing mixers. + * + * @param {Array} prev - Previous mixer array + * @param {Array} next - New mixer array + * @returns {boolean} True if arrays have same content (by ID) + */ +const mixerArraysEqual = (prev, next) => { + // Same reference = definitely equal + if (prev === next) return true; + + // Different lengths = definitely not equal + if (!prev || !next || prev.length !== next.length) return false; + + // Compare by mixer pair IDs (master.id + personal.id creates unique pair key) + const getIds = (arr) => arr + .map(pair => `${pair.master?.id || ''}-${pair.personal?.id || ''}`) + .sort() + .join(','); + + return getIds(prev) === getIds(next); +}; +``` + +Why ID-based comparison: +- Mixer objects have stable `id` fields +- Deep comparison with JSON.stringify is slow and fragile +- We only care if the set of mixers changed, not individual properties +- Master/personal pair creates unique identifier + + +Helper function exists: +`grep -A 10 "mixerArraysEqual" jam-ui/src/hooks/useMixerHelper.js | head -15` + + +mixerArraysEqual helper function added to useMixerHelper.js + + + + + Task 2: Add selectors for current category state + jam-ui/src/hooks/useMixerHelper.js + +Import the category selectors and add refs to track previous values. Find the existing selector imports and add the category selectors. + +1. Update imports from mixersSlice (around line 10-30): +```javascript +import { + // ... existing imports ... + selectMetronomeTrackMixers, + selectBackingTrackMixers, + selectJamTrackMixers, + selectRecordingTrackMixers, + selectAdhocTrackMixers, +} from '../store/features/mixersSlice'; +``` + +2. Inside the useMixerHelper function, add useRef to track previous values. Place this near other refs/selectors: +```javascript +// Track previous category values for comparison +const prevCategoriesRef = useRef({ + metronome: [], + backing: [], + jam: [], + recording: [], + adhoc: [] +}); +``` + +Note: We use a ref instead of selectors to avoid circular updates. The ref stores the last dispatched values so we can compare before dispatching again. + + +Category selectors imported: +`grep "selectMetronomeTrackMixers\|selectBackingTrackMixers" jam-ui/src/hooks/useMixerHelper.js | head -5` + +prevCategoriesRef defined: +`grep "prevCategoriesRef" jam-ui/src/hooks/useMixerHelper.js | head -3` + + +Category selectors imported and prevCategoriesRef added + + + + + Task 3: Add conditional dispatch to categorization useEffect + jam-ui/src/hooks/useMixerHelper.js + +Modify the categorization useEffect (around lines 236-297) to only dispatch when content changes. + +Find the current dispatch block: +```javascript +// Dispatch to Redux +dispatch(setMetronomeTrackMixers(metronomeTrackMixers)); +dispatch(setBackingTrackMixers(backingTrackMixers)); +dispatch(setJamTrackMixers(jamTrackMixers)); +dispatch(setRecordingTrackMixers(recordingTrackMixers)); +dispatch(setAdhocTrackMixers(adhocTrackMixers)); +``` + +Replace with conditional dispatches: +```javascript +// Dispatch to Redux ONLY if content changed +// This prevents unnecessary re-renders when mixer objects change but categories stay same + +if (!mixerArraysEqual(prevCategoriesRef.current.metronome, metronomeTrackMixers)) { + console.log('[useMixerHelper] Metronome mixers changed, dispatching'); + dispatch(setMetronomeTrackMixers(metronomeTrackMixers)); + prevCategoriesRef.current.metronome = metronomeTrackMixers; +} + +if (!mixerArraysEqual(prevCategoriesRef.current.backing, backingTrackMixers)) { + console.log('[useMixerHelper] Backing track mixers changed, dispatching'); + dispatch(setBackingTrackMixers(backingTrackMixers)); + prevCategoriesRef.current.backing = backingTrackMixers; +} + +if (!mixerArraysEqual(prevCategoriesRef.current.jam, jamTrackMixers)) { + console.log('[useMixerHelper] Jam track mixers changed, dispatching'); + dispatch(setJamTrackMixers(jamTrackMixers)); + prevCategoriesRef.current.jam = jamTrackMixers; +} + +if (!mixerArraysEqual(prevCategoriesRef.current.recording, recordingTrackMixers)) { + console.log('[useMixerHelper] Recording mixers changed, dispatching'); + dispatch(setRecordingTrackMixers(recordingTrackMixers)); + prevCategoriesRef.current.recording = recordingTrackMixers; +} + +if (!mixerArraysEqual(prevCategoriesRef.current.adhoc, adhocTrackMixers)) { + console.log('[useMixerHelper] Adhoc mixers changed, dispatching'); + dispatch(setAdhocTrackMixers(adhocTrackMixers)); + prevCategoriesRef.current.adhoc = adhocTrackMixers; +} +``` + +Note: Console logs are useful for debugging but can be commented out in production. They help verify that dispatches are being skipped when expected. + + +Conditional dispatch pattern present: +`grep -A 3 "mixerArraysEqual.*metronome" jam-ui/src/hooks/useMixerHelper.js | head -5` + +All 5 categories have conditional checks: +`grep -c "mixerArraysEqual.*prevCategoriesRef" jam-ui/src/hooks/useMixerHelper.js` should be 5 + +ESLint passes: +`cd jam-ui && npx eslint src/hooks/useMixerHelper.js --max-warnings=0` + + +Mixer categorization only dispatches when content changes, tracked via prevCategoriesRef + + + + + + +1. Helper function added: + ```bash + grep -c "mixerArraysEqual" jam-ui/src/hooks/useMixerHelper.js + ``` + Should return at least 6 (1 definition + 5 usages) + +2. Conditional dispatch pattern: + ```bash + grep "if (!mixerArraysEqual" jam-ui/src/hooks/useMixerHelper.js | wc -l + ``` + Should return 5 + +3. ESLint clean: + ```bash + cd jam-ui && npx eslint src/hooks/useMixerHelper.js --max-warnings=0 + ``` + + + +- [ ] mixerArraysEqual helper function created +- [ ] prevCategoriesRef tracks previously dispatched values +- [ ] All 5 category dispatches use conditional check +- [ ] prevCategoriesRef updated only when dispatch occurs +- [ ] ESLint passes with no warnings + + + +After completion, create `.planning/phases/32-state-update-optimization/32-02-SUMMARY.md` + diff --git a/.planning/phases/32-state-update-optimization/32-03-PLAN.md b/.planning/phases/32-state-update-optimization/32-03-PLAN.md new file mode 100644 index 000000000..96aa4e6b5 --- /dev/null +++ b/.planning/phases/32-state-update-optimization/32-03-PLAN.md @@ -0,0 +1,393 @@ +--- +phase: 32-state-update-optimization +plan: 03 +type: execute +wave: 1 +depends_on: [] +files_modified: + - jam-ui/src/components/client/JKSessionScreen.js + - jam-ui/src/components/client/JKResyncButton.js + - jam-ui/src/components/client/JKVideoButton.js +autonomous: true + +must_haves: + truths: + - "resyncLoading state lives in JKResyncButton (not JKSessionScreen)" + - "videoLoading state lives in JKVideoButton (not JKSessionScreen)" + - "Loading state changes don't re-render JKSessionScreen" + artifacts: + - path: "jam-ui/src/components/client/JKResyncButton.js" + provides: "Self-contained resync button with loading state" + exports: ["JKResyncButton"] + - path: "jam-ui/src/components/client/JKVideoButton.js" + provides: "Self-contained video button with loading state" + exports: ["JKVideoButton"] + - path: "jam-ui/src/components/client/JKSessionScreen.js" + provides: "Uses extracted button components" + contains: "JKResyncButton" + key_links: + - from: "jam-ui/src/components/client/JKSessionScreen.js" + to: "JKResyncButton" + via: "import and render" + pattern: " +Colocate loading states to button components + +Purpose: Move resyncLoading (COLOC-01) and videoLoading (COLOC-02) from JKSessionScreen to their respective button components. This follows state colocation principles - state should live in the component that uses it, preventing parent re-renders. + +Output: JKResyncButton and JKVideoButton components with colocated loading state, JKSessionScreen simplified + + + +@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md +@/Users/nuwan/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/32-state-update-optimization/32-RESEARCH.md + +# Source files +@jam-ui/src/components/client/JKSessionScreen.js + + + + + + Task 1: Create JKResyncButton component + jam-ui/src/components/client/JKResyncButton.js + +Create a new self-contained button component with colocated loading state. + +```javascript +import React, { useState, useCallback, memo } from 'react'; +import { Button, Spinner } from 'reactstrap'; +import { toast } from 'react-toastify'; +import PropTypes from 'prop-types'; + +/** + * Self-contained resync button with colocated loading state. + * Loading state changes only re-render this component, not the parent. + * + * State colocation: https://kentcdodds.com/blog/state-colocation-will-make-your-react-app-faster + */ +const JKResyncButton = memo(({ resyncAudio, className }) => { + const [loading, setLoading] = useState(false); + + const handleClick = useCallback(async (e) => { + e.preventDefault(); + if (loading) return; + + setLoading(true); + try { + await resyncAudio(); + // Silent success (matches legacy behavior) + } catch (error) { + if (error.message === 'timeout') { + toast.error('Audio resync timed out. Please try again.'); + } else { + toast.error('Audio resync failed: ' + (error.message || 'Unknown error')); + } + } finally { + setLoading(false); + } + }, [resyncAudio, loading]); + + return ( + + ); +}); + +JKResyncButton.displayName = 'JKResyncButton'; + +JKResyncButton.propTypes = { + resyncAudio: PropTypes.func.isRequired, + className: PropTypes.string +}; + +export default JKResyncButton; +``` + +Key design decisions: +- memo() wrapper prevents re-renders from parent prop stability +- Loading state is local - changes don't propagate up +- Same error handling as original handleResync +- displayName for React DevTools debugging +- PropTypes for documentation + + +File exists and exports component: +`ls -la jam-ui/src/components/client/JKResyncButton.js` + +Has useState for loading: +`grep "useState(false)" jam-ui/src/components/client/JKResyncButton.js` + +Has memo wrapper: +`grep "memo(" jam-ui/src/components/client/JKResyncButton.js` + + +JKResyncButton component created with colocated loading state + + + + + Task 2: Create JKVideoButton component + jam-ui/src/components/client/JKVideoButton.js + +Create a new self-contained video button component with colocated loading state. + +```javascript +import React, { useState, useCallback, memo } from 'react'; +import { Button, Spinner } from 'reactstrap'; +import { toast } from 'react-toastify'; +import PropTypes from 'prop-types'; +import videoIcon from '../../assets/images/icons8-video-call-50.png'; + +/** + * Self-contained video button with colocated loading state. + * Loading state changes only re-render this component, not the parent. + * + * State colocation: https://kentcdodds.com/blog/state-colocation-will-make-your-react-app-faster + */ +const JKVideoButton = memo(({ + canVideo, + getVideoUrl, + onUpgradePrompt, + className +}) => { + const [loading, setLoading] = useState(false); + + // Open external link in new window/tab + const openExternalLink = useCallback((url) => { + window.open(url, '_blank', 'noopener,noreferrer'); + }, []); + + const handleClick = useCallback(async () => { + if (!canVideo()) { + onUpgradePrompt(); + return; + } + + try { + setLoading(true); + + // Get video conferencing room URL from server + const response = await getVideoUrl(); + const videoUrl = `${response.url}&audiooff=true`; + + // Open video URL in new browser window/tab + openExternalLink(videoUrl); + + } catch (error) { + toast.error('Failed to start video session'); + } finally { + // Keep loading state for 10 seconds to prevent multiple clicks + setTimeout(() => setLoading(false), 10000); + } + }, [canVideo, getVideoUrl, onUpgradePrompt, openExternalLink]); + + return ( + + ); +}); + +JKVideoButton.displayName = 'JKVideoButton'; + +JKVideoButton.propTypes = { + canVideo: PropTypes.func.isRequired, + getVideoUrl: PropTypes.func.isRequired, + onUpgradePrompt: PropTypes.func.isRequired, + className: PropTypes.string +}; + +export default JKVideoButton; +``` + +Key design decisions: +- 10-second loading timeout preserved from original +- canVideo and onUpgradePrompt as props for flexibility +- Video icon imported directly (same path as original) +- memo() for render optimization + + +File exists and exports component: +`ls -la jam-ui/src/components/client/JKVideoButton.js` + +Has useState for loading: +`grep "useState(false)" jam-ui/src/components/client/JKVideoButton.js` + +Has 10-second timeout: +`grep "10000" jam-ui/src/components/client/JKVideoButton.js` + + +JKVideoButton component created with colocated loading state + + + + + Task 3: Refactor JKSessionScreen to use extracted components + jam-ui/src/components/client/JKSessionScreen.js + +Replace inline button implementations with the new components. + +1. Add imports at top: +```javascript +import JKResyncButton from './JKResyncButton'; +import JKVideoButton from './JKVideoButton'; +``` + +2. Remove useState declarations (around lines 202-205): +```javascript +// DELETE these lines: +const [videoLoading, setVideoLoading] = useState(false); +const [resyncLoading, setResyncLoading] = useState(false); +``` + +3. Remove handleResync function (around lines 1056-1075): +```javascript +// DELETE the entire handleResync function +// const handleResync = useCallback(async (e) => { ... }); +``` + +4. Remove handleVideoClick function (around lines 977-1002): +```javascript +// DELETE the entire handleVideoClick function +// const handleVideoClick = async () => { ... }; +``` + +5. Update the button JSX in the toolbar section (around lines 1341-1388). + +Find the Video button: +```jsx + +``` + +Replace with: +```jsx + getVideoConferencingRoomUrl(currentSession.id)} + onUpgradePrompt={showVideoUpgradePrompt} +/> +``` + +Find the Resync button: +```jsx + +``` + +Replace with: +```jsx + +``` + +6. Keep the following functions in JKSessionScreen (they're still needed): +- `canVideo` (permission check) +- `showVideoUpgradePrompt` (toast display) +- `resyncAudio` (from useMediaActions) + +7. Remove videoIcon import if no longer used elsewhere: +```javascript +// Check if videoIcon is used elsewhere, if not remove: +// import videoIcon from '../../assets/images/icons8-video-call-50.png'; +``` + + +New components imported: +`grep "JKResyncButton\|JKVideoButton" jam-ui/src/components/client/JKSessionScreen.js | head -5` + +Old state removed: +`grep "videoLoading\|resyncLoading" jam-ui/src/components/client/JKSessionScreen.js | wc -l` should be 0 + +Components used in JSX: +`grep " + +JKSessionScreen refactored to use JKResyncButton and JKVideoButton, loading state no longer in parent + + + + + + +1. New components exist: + ```bash + ls -la jam-ui/src/components/client/JKResyncButton.js jam-ui/src/components/client/JKVideoButton.js + ``` + +2. Loading state removed from JKSessionScreen: + ```bash + grep -c "videoLoading\|resyncLoading" jam-ui/src/components/client/JKSessionScreen.js + ``` + Should return 0 + +3. Components used: + ```bash + grep " + + +- [ ] JKResyncButton.js created with local loading state +- [ ] JKVideoButton.js created with local loading state +- [ ] JKSessionScreen imports and uses both new components +- [ ] videoLoading and resyncLoading useState removed from JKSessionScreen +- [ ] handleResync and handleVideoClick removed from JKSessionScreen +- [ ] All files pass ESLint + + + +After completion, create `.planning/phases/32-state-update-optimization/32-03-SUMMARY.md` + diff --git a/.planning/phases/32-state-update-optimization/32-04-PLAN.md b/.planning/phases/32-state-update-optimization/32-04-PLAN.md new file mode 100644 index 000000000..e808f834e --- /dev/null +++ b/.planning/phases/32-state-update-optimization/32-04-PLAN.md @@ -0,0 +1,223 @@ +--- +phase: 32-state-update-optimization +plan: 04 +type: execute +wave: 2 +depends_on: ["32-03"] +files_modified: + - jam-ui/src/components/client/JKSessionScreen.js +autonomous: false + +must_haves: + truths: + - "Local state audit completed with documented findings" + - "Any additional colocation candidates identified and addressed" + - "JKSessionScreen re-render count measurably reduced" + artifacts: + - path: ".planning/phases/32-state-update-optimization/32-04-SUMMARY.md" + provides: "Audit results and re-render measurements" + contains: "State Colocation Audit" +--- + + +Audit JKSessionScreen local state and verify re-render reduction + +Purpose: Complete COLOC-03 requirement by auditing remaining useState declarations in JKSessionScreen, identifying any additional colocation candidates, and measuring the re-render improvement from Phase 32 changes. + +Output: Audit documentation, any additional colocations if applicable, before/after re-render measurements + + + +@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md +@/Users/nuwan/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/32-state-update-optimization/32-RESEARCH.md + +# Prior plan summary needed for context +@.planning/phases/32-state-update-optimization/32-03-SUMMARY.md + +# Source file to audit +@jam-ui/src/components/client/JKSessionScreen.js + + + + + + Task 1: Audit remaining useState declarations + jam-ui/src/components/client/JKSessionScreen.js + +Analyze all remaining useState declarations in JKSessionScreen after Plan 03 changes. + +1. List all useState declarations: +```bash +grep -n "useState" jam-ui/src/components/client/JKSessionScreen.js +``` + +2. For each state variable, document: + - Name and purpose + - Where it's used (components that read it) + - Whether it could be colocated to a child component + - Decision: KEEP (used by multiple children or parent logic) or CANDIDATE (single child usage) + +Expected states after Plan 03 (with videoLoading/resyncLoading removed): +- volumeLevel - Used in volume modal, could be colocated to JKSessionVolumeModal +- leaveRating - Used in leave modal flow +- leaveComments - Used in leave modal flow +- leaveLoading - Used in leave modal button + +3. Analyze each candidate: + - volumeLevel: Shared between slider and display, modal handles both - KEEP (modal manages its own state) + - leaveRating/leaveComments/leaveLoading: All used within leave modal flow + - These could potentially move to JKSessionLeaveModal + - But leave modal may need values from parent for submit + - Decision: Document as FUTURE CANDIDATE - not critical path for this phase + +4. Record findings in task output for summary. + + +Review useState declarations present: +`grep -c "useState" jam-ui/src/components/client/JKSessionScreen.js` + +Verify videoLoading/resyncLoading removed (should be 0): +`grep -c "videoLoading\|resyncLoading" jam-ui/src/components/client/JKSessionScreen.js` + + +useState audit completed with documented decisions for each state variable + + + + + Task 2: Document re-render measurement approach + jam-ui/src/components/client/JKSessionScreen.js + +Add a temporary render counter for measurement purposes. This will be removed after verification. + +1. Add render counter at the beginning of the JKSessionScreen function body: +```javascript +// TEMPORARY: Render counter for Phase 32 verification +// Remove after verifying optimization +const renderCountRef = React.useRef(0); +renderCountRef.current += 1; +if (process.env.NODE_ENV === 'development') { + console.log(`[JKSessionScreen] Render #${renderCountRef.current}`); +} +``` + +2. Document in the summary how to measure: + - Open React DevTools Profiler + - Start recording + - Join a session (triggers track sync and mixer setup) + - Wait 10 seconds for stabilization + - Stop recording + - Check JKSessionScreen render count + - Compare with expected reduction (baseline was ~20-30 renders on join, target is 50%+ reduction) + +3. The success criteria from roadmap: "JKSessionScreen re-render count reduced by 50%+" + - This measurement is a checkpoint for the user to verify + - Document expected numbers in summary + + +Render counter present (for now): +`grep "renderCountRef" jam-ui/src/components/client/JKSessionScreen.js | head -3` + +Counter only logs in development: +`grep "NODE_ENV.*development" jam-ui/src/components/client/JKSessionScreen.js` + + +Render counter added for measurement, documentation of measurement approach prepared + + + + + +Phase 32 state update optimizations: +1. Single debounced track sync (replacing 3 setTimeout calls) +2. Stable useDebounceCallback hook for trackChanges +3. Conditional dispatch in mixer categorization +4. JKResyncButton with colocated loading state +5. JKVideoButton with colocated loading state +6. State colocation audit completed + + +1. **Functional verification:** + - Start jam-ui: `cd jam-ui && npm run start` + - Log in and join a session + - Verify session loads correctly + - Click Resync button - should show loading spinner + - Click Video button - should open video conferencing + +2. **Re-render measurement:** + - Open React DevTools in browser (F12 -> Profiler tab) + - Click "Start profiling" + - Join a new session (or refresh current session) + - Wait 10 seconds + - Click "Stop profiling" + - Look at JKSessionScreen render count + - Expected: Significantly fewer renders than before (target: 50%+ reduction) + - Check console for `[JKSessionScreen] Render #X` logs + +3. **Track sync verification:** + - Open Network tab in DevTools + - Join a session + - Filter by "track" in network requests + - Should see only ONE track sync call (not 3) + +4. **Console verification:** + - After joining session, check console for: + - `[Track Sync] Mixers ready, scheduling single debounced sync` (if uncommented) + - `[useMixerHelper] Metronome mixers changed, dispatching` (on first categorization) + - Subsequent mixer updates should NOT log dispatch messages + +5. **Remove render counter:** + - After verification, the render counter can be removed from JKSessionScreen + - Or keep for ongoing monitoring (your preference) + + +Type "approved" if: +- Session joins and works correctly +- Buttons function as expected +- Re-render count is measurably reduced +- Track sync fires once (not 3 times) + +Or describe any issues found. + + + + + + +1. Audit completed (Task 1): + - All useState documented with decisions + - No missed colocation opportunities for this phase + +2. Measurement approach documented (Task 2): + - Render counter added + - Steps for measuring improvement clear + +3. Human verification (Task 3): + - Functional tests pass + - Performance improvement confirmed + + + +- [ ] All useState declarations audited and documented +- [ ] Render counter added for measurement +- [ ] User verifies session join works +- [ ] User verifies buttons work +- [ ] User confirms re-render reduction (50%+ target) +- [ ] Track sync fires once per session join + + + +After completion, create `.planning/phases/32-state-update-optimization/32-04-SUMMARY.md` + +Include in summary: +- State audit results table +- Re-render measurements (before/after if available) +- Any additional candidates for future optimization +