chore(web): document set-state-in-effect on ThemeToggle localStorage read (#90) #93

Merged

maximus merged 1 commit from issue-90-themetoggle-lint-disable into master

2026-05-30 19:25:21 +00:00

maximus commented

2026-05-30 18:58:06 +00:00

Owner

Fixes #90

Suite à /analyze 90 (décision : Option A — eslint-disable documenté).

Changement (comment-only)

ThemeToggle.tsx, 1er useEffect : ajout d'un // eslint-disable-next-line react-hooks/set-state-in-effect ciblé sur la ligne setTheme(stored), avec justification. Aucun changement de logique.

localStorage indisponible en SSR → le read doit être post-mount.
ThemeScript.tsx pose déjà la classe dark avant hydratation (pas de FOUC de page).
Seule occurrence du pattern dans web/ (fix local, pas de hook partagé).
useSyncExternalStore ne supprimerait pas le flash d'icône (le SSR ne lit pas localStorage) ; le cookie aurait un scope trop large. Options B/C détaillées dans #90.

Vérification

eslint web/ : 0 problème (était 1)
tsc --noEmit : OK
Diff : 1 fichier, +5 (commentaires uniquement)
Pas de harness de test dans web/ ; changement comment-only → zéro impact runtime (cycle de thème inchangé).

Fixes #90 Suite à /analyze 90 (décision : Option A — eslint-disable documenté). ## Changement (comment-only) `ThemeToggle.tsx`, 1er `useEffect` : ajout d'un `// eslint-disable-next-line react-hooks/set-state-in-effect` ciblé sur la ligne `setTheme(stored)`, avec justification. Aucun changement de logique. ## Pourquoi un disable (pas useSyncExternalStore / cookie) - `localStorage` indisponible en SSR → le read doit être post-mount. - `ThemeScript.tsx` pose déjà la classe `dark` avant hydratation (pas de FOUC de page). - Seule occurrence du pattern dans `web/` (fix local, pas de hook partagé). - `useSyncExternalStore` ne supprimerait pas le flash d'icône (le SSR ne lit pas localStorage) ; le cookie aurait un scope trop large. Options B/C détaillées dans #90. ## Vérification - `eslint` web/ : **0 problème** (était 1) - `tsc --noEmit` : OK - Diff : 1 fichier, +5 (commentaires uniquement) - Pas de harness de test dans `web/` ; changement comment-only → zéro impact runtime (cycle de thème inchangé).

maximus added 1 commit 2026-05-30 18:58:07 +00:00

chore(web): document set-state-in-effect on ThemeToggle localStorage read (#90 ) e9ddf27eab

ThemeToggle's mount effect reads localStorage then setTheme, a pattern
react-hooks/set-state-in-effect flags. It is safe here: localStorage is
SSR-unavailable so the read must happen post-mount, and ThemeScript
already applies the dark class before hydration (no page FOUC). Add a
targeted eslint-disable-next-line with justification.

web/ lint is now green (0 errors).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

maximus added the

status:review

type:refactor

labels 2026-05-30 18:58:07 +00:00

maximus commented

2026-05-30 19:11:45 +00:00

Author

Owner

Review — APPROVE

Verdict : APPROVE

Changement comment-only validé. Le directive eslint-disable-next-line react-hooks/set-state-in-effect cible la bonne ligne et supprime une erreur réelle (pas un faux positif de directive inutilisée).

Vérification locale (sur la branche `issue-90-themetoggle-lint-disable` via worktree isolé)

eslint web/ → exit 0, 0 problème (master en avait 1 sur cette ligne). La directive fonctionne.
eslint --report-unused-disable-directives → exit 0 : le disable est bien consommé, pas orphelin.
tsc --noEmit → exit 0.
Diff confirmé : 1 fichier, +5 lignes (commentaires uniquement), logique de cycle de thème inchangée.

Bien-fondé de la justification

ThemeScript.tsx lit effectivement localStorage.getItem('sl-theme') et pose la classe dark avant le premier paint → pas de FOUC de page, seule l'icône du toggle se corrige au mount. Claim exact.
localStorage indisponible en SSR → read forcément post-mount. Le choix d'un disable documenté plutôt que useSyncExternalStore/cookie est cohérent avec l'analyse de #90 (le SSR ne lirait pas localStorage de toute façon).

Checklist

Sécurité : N/A (comment-only, aucun secret, aucune surface d'injection).
Tests : N/A (pas de harness dans web/, impact runtime nul — aucun test de régression requis).
i18n : N/A (aucune chaîne utilisateur).
Data : N/A (aucune migration).
Commit conventionnel : OK (chore(web): document set-state-in-effect on ThemeToggle localStorage read (#90)), référence Fixes #90.

Aucun point bloquant.

## Review — APPROVE **Verdict : APPROVE** Changement comment-only validé. Le directive `eslint-disable-next-line react-hooks/set-state-in-effect` cible la bonne ligne et supprime une erreur réelle (pas un faux positif de directive inutilisée). ### Vérification locale (sur la branche `issue-90-themetoggle-lint-disable` via worktree isolé) - `eslint` web/ → **exit 0, 0 problème** (master en avait 1 sur cette ligne). La directive fonctionne. - `eslint --report-unused-disable-directives` → **exit 0** : le disable est bien consommé, pas orphelin. - `tsc --noEmit` → **exit 0**. - Diff confirmé : 1 fichier, +5 lignes (commentaires uniquement), logique de cycle de thème inchangée. ### Bien-fondé de la justification - `ThemeScript.tsx` lit effectivement `localStorage.getItem('sl-theme')` et pose la classe `dark` avant le premier paint → pas de FOUC de page, seule l'icône du toggle se corrige au mount. Claim exact. - `localStorage` indisponible en SSR → read forcément post-mount. Le choix d'un disable documenté plutôt que `useSyncExternalStore`/cookie est cohérent avec l'analyse de #90 (le SSR ne lirait pas localStorage de toute façon). ### Checklist - Sécurité : N/A (comment-only, aucun secret, aucune surface d'injection). - Tests : N/A (pas de harness dans `web/`, impact runtime nul — aucun test de régression requis). - i18n : N/A (aucune chaîne utilisateur). - Data : N/A (aucune migration). - Commit conventionnel : OK (`chore(web): document set-state-in-effect on ThemeToggle localStorage read (#90)`), référence `Fixes #90`. Aucun point bloquant.

maximus merged commit 51429045e6 into master

2026-05-30 19:25:21 +00:00

maximus referenced this pull request from a commit

2026-05-30 19:25:22 +00:00

Merge pull request 'chore(web): document set-state-in-effect on ThemeToggle localStorage read (#90)' (#93) from issue-90-themetoggle-lint-disable into master

No reviewers