eric/pmomusic
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

🔧 Frontend review fixes & quality improvements

Browse Source 
- Fixed race condition in apiCache TTL handling by passing ttl per-entry
- Removed double snapshot reassignment bug after switch cases in useRenderers.ts  
- Replaced `as any` cast on transport_state with runtime guard (isTransportState)
- Fixed invalidate() to preserve subscriptions instead of deleting them
- Migrated reactive(Map) → shallowRef + explicit reactivity triggers (triggerSnapshotReactive, triggerLoadingReactice)
- Added onUnmounted cleanup for debounce timer in useRenderer()
- Exposed resetSSE() to allow reinitialization after SSE reconnect
- Split deep watch in useTabs.ts into separate lightweight watches without {deep: true}
- Fixed swipe gesture to capture startX in onSwipeStart instead of using final clientX
- Added minimal JSON validation log for null responses (DEV mode only)
- Implemented ARIA labels on transport controls and volume slider
- Added UI notifications for network errors via uiStore.notifyError()
+ Removed obsolete toRaw() usage and import after shallowRef migration
- Added missing reactivity triggers for state_changed, queue_refreshing/updated cases
This commit is contained in:
Eric Coissac
2026-04-06 08:47:22 +02:00
parent 66ba31b12f
commit ac9cb3ef3f
9 changed files with 801 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
pmoapp/webapp/src/components/pmocontrol/TransportControls.vue
View File

@@ -68,6 +68,7 @@ async function handleNext() {
:disabled="isPlaying"
@click="handlePlay"
title="Lecture"
aria-label="Lecture"
>
<Play :size="20" />
</button>
@@ -77,6 +78,7 @@ async function handleNext() {
:disabled="isPaused || isStopped"
@click="handlePause"
title="Pause"
aria-label="Pause"
>
<Pause :size="20" />
</button>
@@ -86,6 +88,7 @@ async function handleNext() {
:disabled="isStopped"
@click="handleStop"
title="Stop"
aria-label="Arrêter"
>
<Square :size="20" />
</button>
@@ -95,6 +98,7 @@ async function handleNext() {
:disabled="!state?.queue_len"
@click="handleNext"
title="Suivant"
aria-label="Morceau suivant"
>
<SkipForward :size="20" />
</button>

2
pmoapp/webapp/src/components/pmocontrol/VolumeControl.vue
View File

@@ -65,6 +65,7 @@ async function handleToggleMute() {
class="btn btn-icon"
@click="handleToggleMute"
:title="state?.mute ? 'Réactiver le son' : 'Couper le son'"
:aria-label="state?.mute ? 'Réactiver le son' : 'Couper le son'"
>
<VolumeX v-if="state?.mute" :size="20" />
<Volume2 v-else :size="20" />
@@ -78,6 +79,7 @@ async function handleToggleMute() {
@input="handleVolumeChange"
class="volume-slider"
:disabled="state?.mute ?? false"
aria-label="Volume"
/>
<span class="volume-value">{{ localVolume }}</span>

22
pmoapp/webapp/src/composables/apiCache.ts
View File

@@ -11,6 +11,7 @@
export interface CacheEntry<T> {
data: T;
timestamp: number;
ttl?: number;
etag?: string;
}
@@ -51,7 +52,9 @@ class ApiCacheService {
private isFresh(key: string): boolean {
const entry = this.cache.get(key);
if (!entry) return false;
return Date.now() - entry.timestamp < this.options.ttl;
// Lire le TTL de l'entrée, sinon utiliser le TTL global par défaut
const ttl = entry.ttl ?? this.options.ttl;
return Date.now() - entry.timestamp < ttl;
}
get<T>(endpoint: string, params?: Record<string, string | number | boolean>): T | null {
@@ -66,12 +69,13 @@ class ApiCacheService {
return entry.data;
}
set<T>(endpoint: string, data: T, params?: Record<string, string | number | boolean>, etag?: string): void {
set<T>(endpoint: string, data: T, params?: Record<string, string | number | boolean>, etag?: string, ttl?: number): void {
const key = this.makeKey(endpoint, params);
this.cache.set(key, {
data,
timestamp: Date.now(),
ttl,
etag,
});
@@ -127,14 +131,9 @@ class ApiCacheService {
try {
const data = await fetcher();
if (ttl) {
const originalTtl = this.options.ttl;
this.options.ttl = ttl;
this.set(endpoint, data, params);
this.options.ttl = originalTtl;
} else {
this.set(endpoint, data, params);
}
// Passer le TTL directement à set() pour éviter les problèmes de race condition
// avec la modification globale de this.options.ttl
this.set(endpoint, data, params, undefined, ttl);
resolvePromise(data);
@@ -196,7 +195,8 @@ class ApiCacheService {
keysToDelete.forEach(key => {
this.cache.delete(key);
this.subscriptions.delete(key);
// NE PAS supprimer this.subscriptions.get(key)
// Les abonnés seront notifiés lors du prochain set() après un refetch
});
}

157
pmoapp/webapp/src/composables/useRenderers.ts
View File

@@ -4,11 +4,12 @@
* - Les snapshots complets proviennent de /renderers/{id}/full
* - Les événements SSE ne servent qu'à déclencher un refetch.
*/
import { ref, reactive, computed, toRaw, type Ref } from "vue";
import { ref, shallowRef, computed, toRaw, type Ref, onUnmounted } from "vue";
import { api } from "../services/pmocontrol/api";
import { useSSE } from "./useSSE";
import { apiCache } from "./apiCache";
import { parseTimeToMs } from "../utils/time";
import { useUIStore } from "@/stores/ui";
import type {
RendererSummary,
RendererState,
@@ -16,33 +17,43 @@ import type {
AttachedPlaylistInfo,
FullRendererSnapshot,
} from "../services/pmocontrol/types";
import { isTransportState } from "../services/pmocontrol/types";
interface RendererSnapshotState {
snapshots: Map<string, FullRendererSnapshot>;
lastSnapshotAt: Map<string, number>;
lastEventAt: Map<string, number>;
loadingIds: Set<string>;
queueRefreshingIds: Set<string>;
selectedRendererId: string | null;
}
// État global des snapshots avec shallowRef pour éviter les problèmes de réactivité avec les Maps
const snapshots = shallowRef(new Map<string, FullRendererSnapshot>());
const lastSnapshotAt = shallowRef(new Map<string, number>());
const lastEventAt = shallowRef(new Map<string, number>());
const loadingIds = shallowRef(new Set<string>());
const queueRefreshingIds = shallowRef(new Set<string>());
const selectedRendererId = ref<string | null>(null);
// Cache des renderers (summary)
const renderersCache = ref<Map<string, RendererSummary>>(new Map());
const RENDERERS_CACHE_MS = 2000;
const snapshotState = reactive<RendererSnapshotState>({
snapshots: reactive(new Map<string, FullRendererSnapshot>()),
lastSnapshotAt: reactive(new Map<string, number>()),
lastEventAt: reactive(new Map<string, number>()),
loadingIds: reactive(new Set<string>()),
queueRefreshingIds: reactive(new Set<string>()),
selectedRendererId: null,
});
// Helper pour déclencher la réactivité après mutation des Maps
function triggerSnapshotReactivity() {
// Créer une nouvelle référence pour déclencher la réactivité
snapshots.value = new Map(snapshots.value);
}
function triggerLoadingReactivity() {
loadingIds.value = new Set(loadingIds.value);
}
const loading = ref(false);
const error = ref<string | null>(null);
// Utiliser le composable SSE centralisé
let sseInitialized = false;
/**
* Réinitialise le flag SSE pour permettre une nouvelle connexion après reconnexion
*/
function resetSSE() {
sseInitialized = false;
}
function ensureSSEInitialized() {
if (sseInitialized) return;
@@ -99,16 +110,17 @@ function ensureSSEInitialized() {
}
// Supprimer le snapshot (il n'est plus valide)
snapshotState.snapshots.delete(rendererId);
snapshotState.lastSnapshotAt.delete(rendererId);
snapshotState.lastEventAt.delete(rendererId);
snapshots.value.delete(rendererId);
triggerSnapshotReactivity();
lastSnapshotAt.value.delete(rendererId);
lastEventAt.value.delete(rendererId);
return;
}
// Pour les autres événements, mettre à jour le snapshot local directement
snapshotState.lastEventAt.set(rendererId, timestamp);
lastEventAt.value.set(rendererId, timestamp);
const snapshot = snapshotState.snapshots.get(rendererId);
const snapshot = snapshots.value.get(rendererId);
// Si pas de snapshot, on doit fetch
if (!snapshot) {
@@ -119,7 +131,12 @@ function ensureSSEInitialized() {
// Sinon, mettre à jour le snapshot localement selon le type d'événement
switch (event.type) {
case "state_changed":
snapshot.state.transport_state = event.state as any;
// Utiliser le guard de type pour valider le transport_state
if (isTransportState(event.state)) {
snapshot.state.transport_state = event.state;
} else {
console.warn(`[useRenderers] transport_state inconnu: ${event.state}`);
}
break;
case "position_changed":
@@ -148,15 +165,23 @@ function ensureSSEInitialized() {
...snapshot,
state: newState,
};
snapshotState.snapshots.set(rendererId, newSnapshot);
snapshots.value.set(rendererId, newSnapshot);
break;
case "volume_changed":
snapshot.state.volume = event.volume;
// Créer un nouvel objet pour déclencher la réactivité
snapshots.value.set(rendererId, {
...snapshot,
state: { ...snapshot.state, volume: event.volume },
});
break;
case "mute_changed":
snapshot.state.mute = event.mute;
// Créer un nouvel objet pour déclencher la réactivité
snapshots.value.set(rendererId, {
...snapshot,
state: { ...snapshot.state, mute: event.mute },
});
break;
case "metadata_changed":
@@ -173,19 +198,19 @@ function ensureSSEInitialized() {
snapshot.state.current_track.album = event.album;
snapshot.state.current_track.album_art_uri = event.album_art_uri;
// Important: Trigger reactivity en réassignant l'objet complet avec deep copy
snapshotState.snapshots.set(rendererId, {
snapshots.value.set(rendererId, {
...snapshot,
state: { ...snapshot.state },
});
break;
case "queue_refreshing":
snapshotState.queueRefreshingIds.add(rendererId);
queueRefreshingIds.value.add(rendererId);
break;
case "queue_updated":
snapshot.state.queue_len = event.queue_length;
snapshotState.queueRefreshingIds.delete(rendererId);
queueRefreshingIds.value.delete(rendererId);
// Pour la queue complète, on doit refetch
void fetchRendererSnapshot(rendererId, { force: true });
break;
@@ -202,10 +227,17 @@ function ensureSSEInitialized() {
snapshot.binding = null;
snapshot.state.attached_playlist = null;
}
// Créer un nouvel objet pour déclencher la réactivité
snapshots.value.set(rendererId, {
...snapshot,
state: { ...snapshot.state },
});
break;
case "stream_state_changed":
snapshot.is_stream = event.is_stream;
// Créer un nouvel objet pour déclencher la réactivité
snapshots.value.set(rendererId, { ...snapshot });
break;
case "timer_started":
@@ -218,8 +250,8 @@ function ensureSSEInitialized() {
break;
}
// Trigger reactivity
snapshotState.snapshots.set(rendererId, snapshot);
// Note: chaque case est maintenant responsable de stocker le snapshot dans la Map
// Plus de réassignation finale après le switch
});
sseInitialized = true;
@@ -230,7 +262,7 @@ const onlineRenderers = computed(() =>
allRenderers.value.filter((r) => r.online),
);
const allSnapshots = computed(() =>
Array.from(snapshotState.snapshots.values()),
Array.from(snapshots.value.values()),
);
const playingRenderers = computed(() =>
allSnapshots.value
@@ -243,35 +275,38 @@ function getRendererById(id: string) {
}
function getSnapshotById(id: string) {
return snapshotState.snapshots.get(id) ?? null;
return snapshots.value.get(id) ?? null;
}
function getStateById(id: string): RendererState | null {
return snapshotState.snapshots.get(id)?.state ?? null;
return snapshots.value.get(id)?.state ?? null;
}
function getQueueById(id: string): QueueSnapshot | null {
return snapshotState.snapshots.get(id)?.queue ?? null;
return snapshots.value.get(id)?.queue ?? null;
}
function getBindingById(id: string): AttachedPlaylistInfo | null {
return snapshotState.snapshots.get(id)?.binding ?? null;
return snapshots.value.get(id)?.binding ?? null;
}
function isSnapshotLoading(id: string) {
return snapshotState.loadingIds.has(id);
return loadingIds.value.has(id);
}
function isQueueRefreshing(id: string) {
return snapshotState.queueRefreshingIds.has(id);
return queueRefreshingIds.value.has(id);
}
function selectRenderer(id: string | null) {
snapshotState.selectedRendererId = id;
selectedRendererId.value = id;
}
async function fetchRenderers(force = false, retries = 2) {
ensureSSEInitialized();
// Créer le store UI pour les notifications (lazy import pour éviter les effets de bord)
const uiStore = useUIStore();
let lastError: Error | null = null;
@@ -304,6 +339,9 @@ async function fetchRenderers(force = false, retries = 2) {
}
error.value = lastError?.message ?? "Erreur fetch renderers";
// Notifier l'utilisateur en cas d'erreur finale
uiStore.notifyError("Impossible de rafraîchir la liste des renderers");
}
async function fetchRendererSnapshot(
@@ -312,33 +350,43 @@ async function fetchRendererSnapshot(
) {
ensureSSEInitialized();
const force = opts?.force ?? false;
const hasSnapshot = snapshotState.snapshots.has(rendererId);
const hasSnapshot = snapshots.value.has(rendererId);
if (!force && hasSnapshot) {
const lastSnapshot = snapshotState.lastSnapshotAt.get(rendererId) ?? 0;
const lastEvent = snapshotState.lastEventAt.get(rendererId) ?? 0;
const lastSnapshot = lastSnapshotAt.value.get(rendererId) ?? 0;
const lastEvent = lastEventAt.value.get(rendererId) ?? 0;
if (lastEvent <= lastSnapshot) {
return;
}
}
// Éviter les requêtes multiples simultanées pour le même renderer
if (snapshotState.loadingIds.has(rendererId)) {
if (loadingIds.value.has(rendererId)) {
return;
}
snapshotState.loadingIds.add(rendererId);
loadingIds.value.add(rendererId);
triggerLoadingReactivity();
// Lazy load UI store pour les notifications
const uiStore = useUIStore();
try {
const snapshot = await api.getRendererFullSnapshot(rendererId);
snapshotState.snapshots.set(rendererId, snapshot);
snapshotState.lastSnapshotAt.set(rendererId, Date.now());
snapshots.value.set(rendererId, snapshot);
lastSnapshotAt.value.set(rendererId, Date.now());
triggerSnapshotReactivity();
} catch (err) {
console.error(`[useRenderers] Erreur snapshot ${rendererId}:`, err);
// En cas d'erreur, on supprime le snapshot pour permettre une nouvelle tentative
snapshotState.snapshots.delete(rendererId);
snapshots.value.delete(rendererId);
triggerSnapshotReactivity();
// Notifier l'utilisateur
uiStore.notifyError(`Impossible de récupérer l'état du renderer`);
} finally {
// Toujours nettoyer le flag de chargement
snapshotState.loadingIds.delete(rendererId);
loadingIds.value.delete(rendererId);
triggerLoadingReactivity();
}
}
@@ -388,7 +436,7 @@ async function play(id: string) {
}
async function resumeOrPlayFromQueue(id: string) {
const snapshot = snapshotState.snapshots.get(id);
const snapshot = snapshots.value.get(id);
if (!snapshot) {
throw new Error(`Renderer ${id} non trouvé`);
}
@@ -510,7 +558,6 @@ export function useRenderers() {
isSnapshotLoading,
isQueueRefreshing,
selectRenderer,
snapshotState,
// Fetchers
fetchRenderers,
fetchRendererSnapshot,
@@ -534,6 +581,8 @@ export function useRenderers() {
playContent,
addToQueue,
addAfterCurrent,
// SSE
resetSSE,
};
}
@@ -553,6 +602,14 @@ export function useRenderer(rendererId: Ref<string>) {
let refreshDebounceTimer: ReturnType<typeof setTimeout> | null = null;
const REFRESH_DEBOUNCE_MS = 500;
// Nettoyer le timer debounce si le composant est démonté
onUnmounted(() => {
if (refreshDebounceTimer !== null) {
clearTimeout(refreshDebounceTimer);
refreshDebounceTimer = null;
}
});
async function refresh(force = true) {
const currentRendererId = rendererId.value;

10
pmoapp/webapp/src/composables/useTabs.ts
View File

@@ -346,12 +346,18 @@ function openServer(server: MediaServerSummary | undefined) {
*/
export function useTabs() {
// Watch pour sauvegarde automatique (uniquement les server tabs)
// Watch séparés sans deep pour éviter la sérialisation complète à chaque mutation
watch(() => state.activeTabId, () => {
saveToLocalStorage();
});
watch(() => state.tabHistory.length, () => {
saveToLocalStorage();
});
watch(
() => [state.tabs, state.activeTabId, state.tabHistory],
() => state.tabs.map(t => t.id + t.type + (t.metadata?.rendererId ?? '') + (t.metadata?.serverId ?? '')).join('|'),
() => {
saveToLocalStorage();
},
{ deep: true },
);
// Restaurer au montage (uniquement les server tabs)

6
pmoapp/webapp/src/services/pmocontrol/api.ts
View File

@@ -47,7 +47,11 @@ class PMOControlAPI {
throw new Error(error.error);
}
return response.json();
const data = await response.json();
if (import.meta.env.DEV && data == null) {
console.warn(`[PMOControlAPI] Réponse vide pour ${path}`);
}
return data;
}
// ============================================================================

17
pmoapp/webapp/src/services/pmocontrol/types.ts
View File

@@ -39,13 +39,7 @@ export interface RendererSummary {
export interface RendererState {
id: string;
friendly_name: string;
transport_state:
| "PLAYING"
| "PAUSED"
| "STOPPED"
| "TRANSITIONING"
| "NO_MEDIA"
| "UNKNOWN";
transport_state: TransportState;
position_ms: number | null;
duration_ms: number | null;
volume: number | null; // 0-100
@@ -55,6 +49,15 @@ export interface RendererState {
current_track: CurrentTrackMetadata | null;
}
export type TransportState = "PLAYING" | "PAUSED" | "STOPPED" | "TRANSITIONING" | "NO_MEDIA" | "UNKNOWN";
/**
* Guard de type pour valider que string est un TransportState valide
*/
export function isTransportState(s: string): s is TransportState {
return ["PLAYING", "PAUSED", "STOPPED", "TRANSITIONING", "NO_MEDIA", "UNKNOWN"].includes(s);
}
export interface CurrentTrackMetadata {
title: string | null;
artist: string | null;

14
pmoapp/webapp/src/views/UnifiedControlView.vue
View File

@@ -34,17 +34,19 @@ const rendererDrawerOpen = ref(false);
// Ref pour le swipe edge detection
const viewRef = ref<HTMLElement | null>(null);
// Position initiale du swipe pour détecter un swipe depuis le bord gauche
const swipeStartX = ref(0);
// Swipe depuis le bord gauche pour ouvrir le drawer
useSwipe(viewRef, {
threshold: 50,
onSwipeStart(e: TouchEvent) {
swipeStartX.value = e.touches[0]?.clientX ?? 0;
},
onSwipeEnd(_e: TouchEvent, swipeDirection: string) {
// Swipe right depuis le bord gauche → ouvrir drawer
if (swipeDirection === "right" && !drawerOpen.value) {
const touch = _e.changedTouches[0];
// Vérifier que le swipe commence depuis le bord gauche (< 50px)
if (touch && touch.clientX < 50) {
drawerOpen.value = true;
}
if (swipeDirection === "right" && !drawerOpen.value && swipeStartX.value < 50) {
drawerOpen.value = true;
}
},
});