Files
pmomusic/Blackboard/Todo/Frontend_Review.md
Eric Coissac 8be250b167 [FE] Révise la stabilité, réactivité et performance du frontend
- Corrige fuites mémoire SSE via nettoyage des listeners (P0)
- Remplace shallowRef<Map> par reactive(new Map()) pour réactivité native Vue (P1)
- Ajoute timeout/AbortController aux requêtes fetch dans l'API client (P4)
- Déboucle les watch() de useTabs avec debounce unique et try/finally sur isRestoringFromStorage (P7)
- Limite notifications à 5 + nettoyage des timers dans UI store (P15)
- Protège routes debug avec import dynamique uniquement en DEV + wildcard 403 (P8)
- Déplace SVG par défaut dans assets/default-cover.svg et import ?raw (P10)
- Ajoute @media prefers-reduced-motion aux animations CSS globales
- Factorise styles drawer-btns avec .drawer-icon-classe + ajoute --opacity-disabled (P13)
- Encode les clés de cache browse avec encodeURIComponent + ':' séparateur (P14)
- Implémente pagination infinite scroll dans browseContainer/loadMore
+ Supprime formatMsToShortTime alias (P9)
- Corrige truncate() pour éviter dépassement maxLength si suffix >=maxLength (P10)
- Valide structure des commandes PMOPlayer avant traitement
+ Met à jour version Cargo.toml et lock (0.3.39)
2026-04-06 09:53:10 +02:00

328 lines
13 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
** Ce travail devra être réalisé en suivant scrupuleusement les consignes listées dans le fichier [@Rules_optimal.md](file:///Users/coissac/Sync/maison/Petite_maisons/src/pmomusic/Blackboard/Rules_optimal.md) **
## Contexte
Ce document est le résultat d'une revue de code complète du frontend Vue.js/TypeScript du Control Point
(`pmoapp/webapp/src/`). L'application est fonctionnelle mais présente plusieurs classes de problèmes
qui peuvent causer des fuites mémoire, des incohérences de réactivité Vue 3, et des difficultés de
maintenance à mesure que l'app grandit.
**Périmètre** : uniquement `pmoapp/webapp/src/` (composants, services, composables, stores, utils, CSS).
---
## Problèmes identifiés
### P0 — Fuites mémoire via listeners SSE jamais nettoyés
**Fichiers** : `src/composables/useSSE.ts`, `src/composables/useRenderers.ts`,
`src/composables/useMediaServers.ts`
Les abonnements aux événements SSE sont créés lors du premier appel de chaque composable, mais
jamais nettoyés si le composable est réutilisé ou le composant détruit. Dans `useSSE.ts`, la
fonction `onRendererEvent()` retourne une fonction de cleanup, mais `useRendererEvents()` ignore
ce retour — l'abonnement reste actif indefiniment.
De plus, dans `imageCache.ts`, un `setInterval` de cleanup s'exécute toutes les 5 minutes sans
jamais être annulé si l'app est détruite.
### P1 — Réactivité Vue incohérente avec `shallowRef` + Maps
**Fichier** : `src/composables/useRenderers.ts` (L23-37)
`snapshots` et `loadingIds` sont déclarés en `shallowRef<Map<...>>()`. Vue ne détecte pas les
mutations d'objets à l'intérieur d'un `shallowRef`. La solution actuelle — `triggerSnapshotReactivity()`
qui crée une nouvelle Map à chaque appel — force une re-render complète de tous les composants
qui dépendent de `snapshots`, même si seul un renderer a changé.
### P2 — Race condition à l'initialisation (main.ts)
**Fichier** : `src/main.ts`
Le UIStore est initialisé après le montage de l'app et l'appel à `sse.connect()`. Des événements
SSE peuvent arriver avant que `useUIStore()` soit appelé dans les composants, et les notifications
correspondantes peuvent être perdues.
### P3 — SSE singleton sans garantie formelle
**Fichiers** : `src/composables/useRenderers.ts`, `src/composables/useMediaServers.ts`
Chaque composable maintient son propre flag `sseInitialized` pour éviter les double-abonnements.
Le mécanisme repose sur une convention implicite fragile : si deux composables s'abonnent au même
type d'événement SSE dans des contextes différents, les callbacks s'accumulent sans être
dédupliqués.
Dans `useSSE.ts`, `setupConnectionListener()` vérifie `connectionCallbacks.size === 0` mais
sans lock — deux appels simultanés peuvent installer deux listeners.
### P4 — Pas de timeout ni retry sur les requêtes `fetch`
**Fichier** : `src/services/pmocontrol/api.ts`
Toutes les requêtes `fetch()` sont émises sans `AbortController`. Si le serveur ne répond pas,
la promesse pend indéfiniment, bloquant potentiellement les composants qui attendent le résultat.
Il n'y a ni timeout configurable ni retry automatique au niveau du service.
### P5 — Validation absente des réponses API
**Fichiers** : `src/services/audioCache.ts` (L76), `src/services/coverCache.ts`,
`src/services/playlists.ts`, `src/services/pmocontrol/api.ts`
Les réponses JSON sont acceptées sans vérification de structure. Une assertion de type comme
`metadata as { origin_url?: unknown }` ne protège pas contre un changement d'API côté Rust. Si
l'API retourne une structure inattendue, le crash survient au runtime, pas à la compilation.
### P6 — Type assertions dangereuses dans PMOPlayer
**Fichier** : `src/services/pmosource.ts` / PMOPlayer (L197-214)
Les messages de commande sont typés `Record<string, unknown>`, puis les propriétés sont castées
directement : `msg.url as string`, `msg.timestamp as number`. Si une propriété est absente ou
d'un type différent, TypeScript ne le détecte pas.
### P7 — `useTabs` : watch multiples sans debounce, flag de restauration non-réinitialisé
**Fichier** : `src/composables/useTabs.ts` (L44, L350-361)
Trois `watch()` séparées écrivent dans `localStorage`. Sans debounce commun, si 3 onglets
changent d'état simultanément, `localStorage` est écrit 3 fois de suite.
Le flag `isRestoringFromStorage` (L44) empêche la boucle de sauvegarde pendant la restauration,
mais sans timeout : si `restoreFromLocalStorage()` lance une exception non-catchée, le flag reste
`true` et toutes les sauvegardes futures sont silencieusement ignorées.
### P8 — Routes de debug exposées en production, pas de lazy loading
**Fichier** : `src/router/index.ts`
Les routes debug (CoversCache, AudioCache, UPnP Explorer, etc.) sont accessibles en production
sans contrôle d'accès. Par ailleurs, tous les composants sont importés statiquement, augmentant
le bundle initial inutilement — les vues debug notamment ne sont jamais utilisées en prod.
### P9 — `formatMsToShortTime` est un alias inutile
**Fichier** : `src/utils/time.ts` (L58-59)
```typescript
// Actuellement
export function formatMsToShortTime(ms: number | null): string {
return formatMsToTime(ms);
}
```
Fonction identique à `formatMsToTime`. Tous les appelants peuvent utiliser directement
`formatMsToTime`.
### P10 — `truncate()` dans `string.ts` peut dépasser `maxLength`
**Fichier** : `src/utils/string.ts` (L46-48)
```typescript
// Actuellement
export function truncate(str: string, maxLength: number, suffix = '…'): string {
return str.length > maxLength ? str.slice(0, maxLength - suffix.length) + suffix : str;
}
```
Si `suffix.length >= maxLength`, `str.slice(0, maxLength - suffix.length)` retourne une chaîne
de longueur négative (comportement silencieux en JS, retourne `''`), et le résultat final est
plus long que `maxLength`.
### P11 — `DEFAULT_COVER_SVG` inline dans coverCache.ts
**Fichier** : `src/services/coverCache.ts` (L206-226)
Un SVG inline de ~20 lignes est inclus dans chaque bundle qui importe `coverCache`. Il devrait
être un fichier `src/assets/default-cover.svg` importé nativement par Vite (ce qui permet le
tree-shaking et le caching HTTP séparé).
### P12 — `animations` CSS sans `prefers-reduced-motion`
**Fichiers** : `src/assets/styles/glass-theme.css` (L384-401),
`src/assets/styles/pmocontrol.css` (L82)
Les animations `glassShimmer` (2s infini) et le `pulse` du badge de statut `Transitioning`
s'exécutent sans tenir compte de `prefers-reduced-motion: reduce`. Sur certains systèmes ou
pour des utilisateurs sensibles au mouvement, ces animations sont gênantes.
### P13 — CSS dupliqué dans drawers.css
**Fichier** : `src/assets/styles/drawers.css` (L105-149)
`drawer-close-btn` et `drawer-back-btn` partagent 90% des styles. Un TODO présent en L167
("remplacer par la classe globale .section-title") confirme cette dette. La variable
`var(--opacity-disabled)` est utilisée mais non définie dans `variables.css`.
### P14 — `browseContainer` : clés de cache fragiles et pas de pagination
**Fichier** : `src/composables/useMediaServers.ts` (L145, L239)
Les clés de cache sont construites comme `${serverId}/${containerId}`. Si un `containerId`
contient un slash (séparateur d'URL), la clé est ambigüe. Par exemple, `server1/a/b` peut
correspondre à serverId=`server1`, containerId=`a/b` ou serverId=`server1/a`, containerId=`b`.
La pagination n'est pas implémentée côté composable : `browseContainer` charge toujours
offset=0, limit=50. Pour les containers avec 500+ items, les items au-delà de 50 ne sont
jamais accessibles.
### P15 — Notifications sans limite de taille dans `ui.ts`
**Fichier** : `src/stores/ui.ts` (L50, L55-57)
Un bug ou une boucle d'erreur peut générer des centaines de notifications. Le tableau
`notifications` n'est pas limité. Chaque notification crée un `setTimeout` individuel, et
si le store est détruit avant l'expiration, ces callbacks persistent (ghosts).
---
## Plan d'exécution
Les corrections sont groupées par effort et impact. Les P0P3 concernent la fiabilité
(fuites mémoire, réactivité), les P4P8 la robustesse et maintenabilité, les P9P15 la
qualité et la dette technique.
### Étape 1 — Corriger les fuites mémoire SSE (P0)
Dans `useSSE.ts`, stocker et appeler les fonctions de cleanup retournées par `onRendererEvent` /
`onMediaServerEvent` :
```typescript
// useSSE.ts useRendererEvents()
onMounted(() => {
const cleanup = onRendererEvent(rendererId(), handler);
onUnmounted(cleanup); // ← actuellement ignoré
});
```
Dans `imageCache.ts`, exporter une fonction `destroyImageCache()` qui appelle `clearInterval`
sur le timer de cleanup, et l'appeler dans le `onUnmounted` de l'app root.
### Étape 2 — Stabiliser la réactivité des snapshots (P1)
Remplacer `shallowRef<Map<...>>` + `triggerSnapshotReactivity` par `reactive(new Map<...>)`.
Vue 3 rend les Maps réactives nativement. Les composants qui lisent `snapshots.get(id)`
seront notifiés uniquement si ce `id` change.
```typescript
// Avant
const snapshots = shallowRef<Map<string, FullRendererSnapshot>>(new Map());
function triggerSnapshotReactivity() {
snapshots.value = new Map(snapshots.value);
}
// Après
const snapshots = reactive(new Map<string, FullRendererSnapshot>());
// Les modifications directes (snapshots.set/delete) déclenchent la réactivité
```
### Étape 3 — Timeout fetch + AbortController (P4)
Ajouter un helper dans `api.ts` :
```typescript
function fetchWithTimeout(url: string, options?: RequestInit, timeoutMs = 10_000): Promise<Response> {
const controller = new AbortController();
const id = setTimeout(() => controller.abort(), timeoutMs);
return fetch(url, { ...options, signal: controller.signal })
.finally(() => clearTimeout(id));
}
```
Utiliser `fetchWithTimeout` pour toutes les requêtes dans le service API.
### Étape 4 — Corriger `useTabs` watchs et flag de restauration (P7)
Fusionner les trois `watch()` en un seul `watchEffect` avec un debounce unique (100ms).
Encadrer `isRestoringFromStorage` dans un bloc `try/finally` :
```typescript
async function restoreFromLocalStorage() {
isRestoringFromStorage = true;
try {
// ... logique de restauration
} catch (e) {
console.error('Tab restore failed:', e);
} finally {
isRestoringFromStorage = false;
}
}
```
### Étape 5 — Limit de notifications et nettoyage timers (P15)
```typescript
const MAX_NOTIFICATIONS = 5;
function addNotification(notif: Omit<Notification, 'id'>): void {
if (notifications.value.length >= MAX_NOTIFICATIONS) {
notifications.value.shift(); // supprimer la plus ancienne
}
const id = nextId++;
const timer = setTimeout(() => removeNotification(id), notif.duration ?? 5000);
notificationTimers.set(id, timer);
notifications.value.push({ ...notif, id });
}
function $dispose() {
notificationTimers.forEach(clearTimeout);
notificationTimers.clear();
}
```
### Étape 6 — Lazy loading des routes et protection debug (P8)
```typescript
// router/index.ts
const DebugView = () => import('../views/DebugView.vue');
const isDev = import.meta.env.DEV;
const routes = [
// ... routes normales
...(isDev ? [{ path: '/debug', component: DebugView }] : []),
{ path: '/:pathMatch(.*)*', redirect: '/' }, // wildcard 404
];
```
### Étape 7 — Corrections mineures (P9, P10, P11, P12, P13)
- **P9** : Supprimer `formatMsToShortTime`, remplacer tous les appels par `formatMsToTime`
- **P10** : Ajouter un guard dans `truncate` : `if (suffix.length >= maxLength) return str.slice(0, maxLength)`
- **P11** : Déplacer le SVG dans `src/assets/default-cover.svg` et l'importer avec `import defaultCover from '../assets/default-cover.svg?raw'`
- **P12** : Entourer les animations CSS avec `@media (prefers-reduced-motion: no-preference) { ... }`
- **P13** : Factoriser `drawer-close-btn` / `drawer-back-btn` avec une classe `.drawer-icon-btn`. Définir `--opacity-disabled: 0.4` dans `variables.css`
### Étape 8 — Clés de cache et pagination (P14)
Encoder les IDs dans les clés de cache :
```typescript
const cacheKey = `${encodeURIComponent(serverId)}:${encodeURIComponent(containerId)}`;
```
Utiliser `:` comme séparateur (absent de l'encoding) pour éviter toute ambigüité.
Pour la pagination, ajouter une propriété `hasMore: boolean` et `loadMore()` au résultat de
`browseContainer`, incrementant offset à chaque appel.
### Ordre d'exécution
1. Étape 1 — fuites SSE (P0) — fiabilité critique
2. Étape 2 — réactivité Map (P1) — fiabilité
3. Étape 3 — timeout fetch (P4) — robustesse réseau
4. Étape 4 — useTabs (P7) — fiabilité des onglets
5. Étape 5 — notifications (P15) — stabilité UI
6. Étape 6 — router (P8) — sécurité + performance bundle
7. Étape 7 — corrections mineures (P9P13)
8. Étape 8 — cache keys + pagination (P14)
## Règle après ces corrections
**Interdit** : créer un abonnement SSE (`onRendererEvent`, `onMediaServerEvent`) sans stocker
et appeler la fonction de cleanup retournée dans `onUnmounted`.
**Interdit** : utiliser `shallowRef<Map<...>>` avec mutation directe — utiliser `reactive(new Map())`
pour les Maps qui doivent déclencher la réactivité Vue sur leurs entrées.
**Obligatoire** : toute requête `fetch()` dans un service doit utiliser `fetchWithTimeout`
avec un AbortController.