fix(useIsDark): never throw when localStorage access is blocked - #521
Open
kevbarns wants to merge 1 commit into
Open
fix(useIsDark): never throw when localStorage access is blocked#521kevbarns wants to merge 1 commit into
kevbarns wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change safely prevents initialization crashes when storage access is blocked and is backed by targeted runtime tests for both the client logic and the ASAP script.
Pull request overview
This PR fixes crashes in the useIsDark initialization path when window.localStorage access is blocked (notably Chromium throwing SecurityError on read), ensuring the client-side theme logic still initializes and falls back to defaults instead of breaking rendering.
Changes:
- Add a
safeLocalStoragewrapper that never throws and behaves like an empty storage when access fails. - Replace direct
localStorageaccesses in the client-sideuseIsDarklogic withsafeLocalStorage. - Harden the pre-hydration “run ASAP” inline script against blocked storage and add runtime tests covering both working and blocked storage scenarios.
File summaries
| File | Description |
|---|---|
| src/tools/safeLocalStorage.ts | Introduces a safe localStorage wrapper that swallows read/write failures and returns null on reads when unavailable. |
| src/useIsDark/client.ts | Switches persisted scheme reads/writes to safeLocalStorage so initialization never crashes when storage is blocked. |
| src/useIsDark/scriptToRunAsap.ts | Inlines a safe storage wrapper inside the ASAP script to prevent pre-hydration theme setup from crashing. |
| test/runtime/lib/safeLocalStorage.test.ts | Adds runtime tests for normal delegation, blocked-storage getter behavior, and write errors like QuotaExceededError. |
| test/runtime/lib/scriptToRunAsap.test.ts | Adds runtime tests executing the generated script with both functional and blocked localStorage. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #442
Problème
Quand le stockage est bloqué pour le document (iframe tierce avec cookies tiers bloqués, WebView avec stockage désactivé, politique d'entreprise…), Chromium lève une
SecurityErrordès la lecture dewindow.localStorage.startClientSideIsDarkLogicn'était pas protégé :start()plantait dansDsfrProvider,$clientSideIsDarkn'était jamais initialisé et le premieruseIsDark()levait ensuite « react-dsfr not initialized… ». Le rendu client de la page était perdu.Données de production et stacktrace complète dans #442 (comment) (~870 événements Sentry depuis mars 2026 sur un seul site, quasi exclusivement Chromium).
Correctif
src/tools/safeLocalStorage.ts:getItem/setItem/removeItemenveloppés dans untry/catch. En cas d'échec, le stockage se comporte comme s'il était vide (getItem→null, écritures ignorées). Le code appelant retombe alors naturellement surcolorSchemeExplicitlyProvidedAsParameterpuisprefers-color-scheme, sans changement de logique.src/useIsDark/client.ts: les 6 accès directs passent par le wrapper.src/useIsDark/scriptToRunAsap.ts: même wrapper inliné dans le script exécuté avant hydratation (5 accès), pour que la prévention du flash blanc ne plante pas non plus dans ces environnements.Non modifié, volontairement :
consentManagement/createConsentManagement.tsaccède aussi àlocalStoragesans garde, mais c'est un autre périmètre fonctionnel (le consentement ne peut pas être « oublié » silencieusement sans décision produit). Je peux ouvrir une PR séparée si souhaité.Tests
test/runtime/lib/safeLocalStorage.test.ts: délégation quand le stockage fonctionne ; comportement « stockage vide » quand le getterwindow.localStoragelève uneSecurityError(simulation du comportement Chromium) ; écriture qui lève unQuotaExceededError.test/runtime/lib/scriptToRunAsap.test.ts: exécute le script généré contre un DOM minimal avec unlocalStoragefonctionnel puis bloqué, et vérifie quedata-fr-theme/data-fr-schemesont bien posés dans les deux cas.scriptToRunAsap.test.ts) et queyarn build && yarn testpasse avec (114 tests),yarn lint:checketyarn format:checksans erreur sur les fichiers touchés,tsc -p srcsans erreur.