Skip to content

useTheme doesn't subscribe to OS theme changes when set to 'system', unlike the sibling useMediaQuery hook that already solves this correctly #31

Description

@abayomicornelius

Problem

src/hooks/useTheme.ts, in full:

import { useLocalStorage } from './useLocalStorage'
import { useEffect } from 'react'
type Theme = 'light' | 'dark' | 'system'
export function useTheme() {
  const [theme, setTheme] = useLocalStorage<Theme>('ss-theme', 'system')
  useEffect(() => {
    const root = document.documentElement
    if (theme === 'dark') root.classList.add('dark')
    else if (theme === 'light') root.classList.remove('dark')
    else root.classList.toggle('dark', matchMedia('(prefers-color-scheme: dark)').matches)
  }, [theme])
  return { theme, setTheme }
}

When theme === 'system', the effect reads matchMedia('(prefers-color-scheme: dark)').matches once, at the moment the effect runs, and never again unless theme itself changes. There's no addEventListener('change', ...) on the media query, so if the user's OS switches between light and dark mode while this app is open (a genuinely common case — many OSes auto-switch at sunset/sunrise, or the user manually toggles system dark mode without touching this app at all) with theme still 'system', the <html> element's dark class is never updated, and the app stays visually stuck on whichever mode was active when the page loaded or when theme was last set.

This is a real, avoidable gap because the exact fix already exists one file away in the same hooks/ directory:

// src/hooks/useMediaQuery.ts
export function useMediaQuery(query: string): boolean {
  const [matches, setMatches] = useState(() => window.matchMedia(query).matches)
  useEffect(() => {
    const mq = window.matchMedia(query)
    const handler = (e: MediaQueryListEvent) => setMatches(e.matches)
    mq.addEventListener("change", handler)
    return () => mq.removeEventListener("change", handler)
  }, [query])
  return matches
}
export const useIsDarkMode = () => useMediaQuery("(prefers-color-scheme: dark)")

useIsDarkMode() — built specifically for this — correctly subscribes to live OS theme changes via addEventListener('change', ...), but useTheme doesn't call it and instead re-implements a one-shot, non-reactive version of the same check inline.

Why it matters

  • Any user who selects "System" as their theme preference (the documented Theme type includes 'system' as a first-class option, and useLocalStorage('ss-theme', 'system') defaults to it) gets a theme that's only correct at the moment they last touched the app's own theme setting — it silently stops tracking the OS after that, which is the opposite of what "System" is supposed to mean.
  • This is exactly the kind of bug that's invisible in normal manual QA (testers rarely flip their OS dark-mode toggle mid-session while a specific page is open) but very visible to real users on OSes with time-of-day-based auto dark mode, who will see the app "stuck" in the wrong theme for potentially hours until they navigate in a way that happens to remount this hook or otherwise churn theme.

Reproduction

  • Set theme to 'system', ensure the OS is currently in light mode (so document.documentElement has no dark class). While the app is open and mounted, switch the OS to dark mode (or, in a test, fire a change event on a mocked matchMedia media query list). Observe document.documentElement's dark class does not update — it requires some unrelated state change to touch theme again before the effect re-runs.

Suggested fix

  • Replace the inline one-shot matchMedia(...).matches check with the existing useIsDarkMode() hook from useMediaQuery.ts, and include its return value in the effect's dependency array so the effect re-runs (and the dark class gets toggled) whenever the OS preference changes, not just when theme changes:
    const systemIsDark = useIsDarkMode()
    useEffect(() => {
      const root = document.documentElement
      if (theme === 'dark') root.classList.add('dark')
      else if (theme === 'light') root.classList.remove('dark')
      else root.classList.toggle('dark', systemIsDark)
    }, [theme, systemIsDark])

Edge cases

  • Rapid OS theme toggling (unlikely but possible via some OS testing tools) should not cause flicker/thrash beyond what the underlying matchMedia change event naturally provides — no additional debouncing should be necessary since the OS itself doesn't fire this rapidly in practice, but worth a sanity check once fixed.

Testing strategy

  • Unit test useTheme with theme fixed at 'system' and a mocked matchMedia whose change handler is manually invoked mid-test (flipping matches from false to true), asserting document.documentElement.classList.contains('dark') updates accordingly without any change to theme itself.
  • Regression-test that explicit 'light'/'dark' selections are unaffected by OS-level changes (only 'system' mode should react to the media query).

Metadata

Metadata

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26accessibilityAccessibility (a11y)bugSomething isn't workingvery hardVery difficult / senior-level bounty issue

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions