Initialize dark mode from system theme#429
Conversation
|
@Kroczak185 is attempting to deploy a commit to the ritesh-oss Team on Vercel. A member of the Team first needs to authorize it. |
geekskai
left a comment
There was a problem hiding this comment.
Problems / improvements
- Good: computing next before setIsDarkMode fixes the stale-read bug.
- Guard window/localStorage for SSR: while useEffect runs client-side, toggleDarkMode and any direct localStorage access should still defensively check typeof window !== 'undefined' and wrap JSON.parse/localStorage calls in try/catch to avoid crashes in restricted environments or browsers with storage disabled.
- Avoid unnecessary writes: the effect currently writes localStorage on every mount even if a saved value exists. Only write when no saved preference exists (or value actually changed).
- JSON.parse safety: malformed saved values will throw. Use try/catch around parsing.
- System preference changes: add a matchMedia change listener so the UI updates when the user toggles the OS theme.
- Cross-tab sync: listen for the window "storage" event so theme changes in one tab update other tabs.
- Stable toggle identity: consider useCallback for toggleDarkMode to keep identity stable across renders.
- Apply theme to DOM: the hook currently returns isDarkMode but doesn’t apply a class to document.documentElement. If you expect the hook to control the UI theme, add a side-effect to toggle a class (e.g., 'dark') so CSS can react.
- Expose setter optionally: returning setIsDarkMode (or a setDarkMode) can be useful for programmatic changes.
- Types & constants: ensure STORAGE_KEY is defined nearby and documented.
Suggested revised implementation (concise, safe, with listeners)
- This example includes SSR guards, try/catch, matchMedia listener, storage event, class application, and a stable toggle:
const STORAGE_KEY = 'app:dark-mode';
export function useDarkMode() {
const [isDarkMode, setIsDarkMode] = useState(() => {
if (typeof window === 'undefined') return false;
try {
const saved = localStorage.getItem(STORAGE_KEY);
if (saved !== null) return JSON.parse(saved);
return !!(window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches);
} catch {
return false;
}
});
useEffect(() => {
if (typeof window === 'undefined') return;
// sync initial value, but avoid overwriting an existing saved value
try {
const saved = localStorage.getItem(STORAGE_KEY);
const inferred =
saved !== null
? JSON.parse(saved)
: !!(window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches);
setIsDarkMode(inferred);
if (saved === null) localStorage.setItem(STORAGE_KEY, JSON.stringify(inferred));
} catch {
/* ignore parsing/storage errors */
}
// listen for system changes
const mql = window.matchMedia('(prefers-color-scheme: dark)');
const onPrefChange = (e: MediaQueryListEvent) => setIsDarkMode(e.matches);
if (mql.addEventListener) mql.addEventListener('change', onPrefChange);
else mql.addListener(onPrefChange);
// listen for cross-tab updates
const onStorage = (e: StorageEvent) => {
if (e.key === STORAGE_KEY) {
try {
setIsDarkMode(e.newValue ? JSON.parse(e.newValue) : false);
} catch {}
}
};
window.addEventListener('storage', onStorage);
return () => {
if (mql.removeEventListener) mql.removeEventListener('change', onPrefChange);
else mql.removeListener(onPrefChange);
window.removeEventListener('storage', onStorage);
};
}, []);
// apply class to document element so CSS can use it
useEffect(() => {
if (typeof document === 'undefined') return;
document.documentElement.classList.toggle('dark', isDarkMode);
}, [isDarkMode]);
const toggleDarkMode = useCallback(() => {
setIsDarkMode(prev => {
const next = !prev;
try {
if (typeof window !== 'undefined') localStorage.setItem(STORAGE_KEY, JSON.stringify(next));
} catch {}
return next;
});
}, []);
return { isDarkMode, toggleDarkMode, setIsDarkMode };
}
When to change the current patch
- If you only want the minimal fix, the patch you provided is fine (it fixes the toggle bug and uses system preference). If you want robustness for real-world apps, add the guards, listeners and DOM-class application above.
Default dark mode setting now follows the system’s color scheme on first load.