ThemeToggle: lint react-hooks/set-state-in-effect (hydratation localStorage) #90

New issue

Closed

opened 2026-05-30 18:22:31 +00:00 by maximus · 0 comments

maximus commented

2026-05-30 18:22:31 +00:00

Owner

Contexte

web/src/components/ThemeToggle.tsx:13-16 declenche l'erreur eslint react-hooks/set-state-in-effect : lecture de localStorage au mount puis setTheme(stored) synchrone dans un useEffect, ce qui force un double-render.

const [theme, setTheme] = useState<Theme>("system");
useEffect(() => {
  const stored = localStorage.getItem("sl-theme") as Theme | null;
  if (stored) setTheme(stored);   // <- ligne 15, flaggee
}, []);

Analyse (suite a /analyze 90)

Cartographie du systeme de theme (sous-agent Explore) :

ThemeScript.tsx (inline <script> dans <head>, monte dans app/layout.tsx:34) pose la classe dark sur document.documentElement avant hydratation : lit sl-theme, resout system via matchMedia. Le flash de fond de page (FOUC) est donc deja couvert.
<html suppressHydrationWarning> (layout.tsx:31) masque deja le mismatch d'hydratation.
Cle sl-theme coherente partout (ThemeScript + ThemeToggle). Aucun mismatch de cle.
Le 2e effect de ThemeToggle (ecrit localStorage + toggle la classe sur [theme]) n'est pas flagge : c'est l'usage autorise du rule (synchroniser un systeme externe avec l'etat React). Seul le 1er effect (read -> setState) est en cause.
Une seule occurrence de ce pattern dans tout web/src/ -> pas de candidat hook partage, fix local.
Aucun precedent dans le repo : pas de useSyncExternalStore, pas de useMounted/useIsClient, pas de garde typeof window.
Pas de sync cross-tab aujourd'hui (aucun listener storage), non requis.
Aucun harness de test dans web/ (scripts : dev/build/start/lint).

Note : framing initial corrige

L'issue disait que useSyncExternalStore "supprime le double-render". Inexact : le serveur ne peut pas lire localStorage, donc getServerSnapshot retourne "system" -> une correction au montage reste de toute facon, et le flash d'icone Monitor -> Moon n'est pas garanti supprime. Le seul vrai fix UX du flash serait un theme lisible au SSR (cookie). Conclusion : ni le disable ni useSyncExternalStore n'eliminent le micro-flash d'icone ; seul un theme en cookie le ferait.

Decision : Option A — eslint-disable documente

Choisie via /analyze (2026-05-30). Le FOUC de page est deja resolu par ThemeScript ; le residuel est uniquement l'icone du toggle, acceptable pour cette dette incidente. useSyncExternalStore (option B) n'ameliore pas franchement l'UX pour le cout et n'a aucun precedent ; le theme-en-cookie (option C) a un scope trop large (migration du theming entier) pour cette issue.

Options B et C documentees ci-dessus si on veut revenir dessus plus tard.

Travail a faire

Dans ThemeToggle.tsx, 1er useEffect, ajouter un // eslint-disable-next-line react-hooks/set-state-in-effect cible sur la ligne setTheme + un commentaire justificatif, p.ex. :

useEffect(() => {
  const stored = localStorage.getItem("sl-theme") as Theme | null;
  // localStorage is unavailable during SSR, so the stored theme can only be
  // read post-mount. ThemeScript applies the `dark` class before hydration, so
  // there is no page FOUC; only the toggle icon corrects on mount. The rule is
  // a false positive for this hydration-from-storage case.
  // eslint-disable-next-line react-hooks/set-state-in-effect
  if (stored) setTheme(stored);
}, []);

eslint web/ vert (0 erreur)
tsc --noEmit clean
Verif manuelle : cycle light/dark/system OK, pas de crash SSR, pas de warning d'hydratation en console

Fichiers concernes

web/src/components/ThemeToggle.tsx — seul fichier (1er useEffect)

Surface de test

Tests existants touchant ThemeToggle : aucun (pas de harness dans web/)
Test de regression : non requis (type:refactor) -> verification manuelle visuelle
Gates automatiques : eslint + tsc --noEmit

Criteres d'acceptation

eslint de web/ : 0 erreur
Cycle de theme light/dark/system fonctionnel, pas de regression
Pas de crash SSR ni warning d'hydratation
Le disable est cible (next-line + nom de regle exact) avec justification — pas de disable de fichier ni de bloc

Complexite

Simple.

Decouvert lors du nettoyage lint de web/ (suite a #70). Les 3 lint triviaux corriges dans PR #91.

## Contexte `web/src/components/ThemeToggle.tsx:13-16` declenche l'erreur eslint `react-hooks/set-state-in-effect` : lecture de `localStorage` au mount puis `setTheme(stored)` synchrone dans un `useEffect`, ce qui force un double-render. ```tsx const [theme, setTheme] = useState<Theme>("system"); useEffect(() => { const stored = localStorage.getItem("sl-theme") as Theme | null; if (stored) setTheme(stored); // <- ligne 15, flaggee }, []); ``` ## Analyse (suite a /analyze 90) Cartographie du systeme de theme (sous-agent Explore) : - **`ThemeScript.tsx`** (inline `<script>` dans `<head>`, monte dans `app/layout.tsx:34`) pose la classe `dark` sur `document.documentElement` **avant hydratation** : lit `sl-theme`, resout `system` via `matchMedia`. Le **flash de fond de page (FOUC) est donc deja couvert**. - **`<html suppressHydrationWarning>`** (`layout.tsx:31`) masque deja le mismatch d'hydratation. - Cle `sl-theme` **coherente** partout (ThemeScript + ThemeToggle). Aucun mismatch de cle. - Le **2e effect** de ThemeToggle (ecrit localStorage + toggle la classe sur `[theme]`) n'est **pas** flagge : c'est l'usage autorise du rule (synchroniser un systeme externe avec l'etat React). Seul le **1er effect** (read -> setState) est en cause. - **Une seule occurrence** de ce pattern dans tout `web/src/` -> pas de candidat hook partage, fix **local**. - **Aucun precedent** dans le repo : pas de `useSyncExternalStore`, pas de `useMounted`/`useIsClient`, pas de garde `typeof window`. - **Pas de sync cross-tab** aujourd'hui (aucun listener `storage`), non requis. - **Aucun harness de test** dans `web/` (scripts : dev/build/start/lint). ### Note : framing initial corrige L'issue disait que `useSyncExternalStore` "supprime le double-render". Inexact : le serveur ne peut pas lire `localStorage`, donc `getServerSnapshot` retourne `"system"` -> une correction au montage reste de toute facon, et le **flash d'icone Monitor -> Moon n'est pas garanti supprime**. Le seul vrai fix UX du flash serait un theme lisible au SSR (cookie). Conclusion : ni le disable ni `useSyncExternalStore` n'eliminent le micro-flash d'icone ; seul un theme en cookie le ferait. ## Decision : Option A — eslint-disable documente Choisie via /analyze (2026-05-30). Le FOUC de page est deja resolu par ThemeScript ; le residuel est uniquement l'icone du toggle, acceptable pour cette dette incidente. `useSyncExternalStore` (option B) n'ameliore pas franchement l'UX pour le cout et n'a aucun precedent ; le theme-en-cookie (option C) a un scope trop large (migration du theming entier) pour cette issue. Options B et C documentees ci-dessus si on veut revenir dessus plus tard. ## Travail a faire - [ ] Dans `ThemeToggle.tsx`, 1er `useEffect`, ajouter un `// eslint-disable-next-line react-hooks/set-state-in-effect` **cible sur la ligne `setTheme`** + un commentaire justificatif, p.ex. : ```tsx useEffect(() => { const stored = localStorage.getItem("sl-theme") as Theme | null; // localStorage is unavailable during SSR, so the stored theme can only be // read post-mount. ThemeScript applies the `dark` class before hydration, so // there is no page FOUC; only the toggle icon corrects on mount. The rule is // a false positive for this hydration-from-storage case. // eslint-disable-next-line react-hooks/set-state-in-effect if (stored) setTheme(stored); }, []); ``` - [ ] `eslint` web/ vert (0 erreur) - [ ] `tsc --noEmit` clean - [ ] Verif manuelle : cycle light/dark/system OK, pas de crash SSR, pas de warning d'hydratation en console ## Fichiers concernes - `web/src/components/ThemeToggle.tsx` — seul fichier (1er useEffect) ## Surface de test - Tests existants touchant ThemeToggle : aucun (pas de harness dans `web/`) - Test de regression : non requis (`type:refactor`) -> verification manuelle visuelle - Gates automatiques : `eslint` + `tsc --noEmit` ## Criteres d'acceptation - [ ] `eslint` de `web/` : 0 erreur - [ ] Cycle de theme light/dark/system fonctionnel, pas de regression - [ ] Pas de crash SSR ni warning d'hydratation - [ ] Le disable est **cible** (next-line + nom de regle exact) avec justification — pas de disable de fichier ni de bloc ## Complexite Simple. _Decouvert lors du nettoyage lint de `web/` (suite a #70). Les 3 lint triviaux corriges dans PR #91._