Problem
src/app/setting/page.tsx adds a system-preference listener and cleans it up with a different function:
const mediaQuery = window.matchMedia('(prefers-color-scheme: dark)');
mediaQuery.addEventListener('change', () => {
if (theme === 'system') { applyTheme('system'); }
});
return () => mediaQuery.removeEventListener('change', () => {});removeEventListener requires the same function reference; the cleanup registers a new anonymous function, so the original change listener is never removed. Consequences:
- The listener leaks on every theme change: each
theme state change re-runs the effect, adding another change listener that is never removed — the leak grows with every theme toggle, and each leaked listener closes over the old theme value (stale closure: after switching to dark, the leaked system-era listeners still check the old theme).
- Unmount never cleans up: navigating away from settings leaves the listener attached to
window.matchMedia for the page's lifetime.
- The cleanup pattern is a trap: the
() => {} placeholder signals the author knew cleanup was required but wired the wrong reference — exactly the class of bug a lint rule (react-hooks/exhaustive-deps won't catch it) or a test should.
Root cause
The cleanup was written with a placeholder handler instead of the registered one.
Why this is architecturally hard
- The fix is to hold the handler in a variable and pass the same reference to both add and remove — but the handler must read the current
theme (a ref, not the closure), or the listener should be re-created per theme change with correct cleanup.
- The theme logic is inline in the page; a
useTheme hook (with the media-query handling and localStorage persistence) would centralize it and make the lifecycle testable — the durable fix is extraction, not just the reference fix.
- Testing listener leaks requires mocking
matchMedia and asserting add/remove call counts — a test harness the page currently lacks.
Proposed design
Extract the theme handling into a useTheme hook (or fix the reference and use a themeRef for the handler), ensuring add/remove use the same reference and unmount removes the listener. Add a test asserting one listener per mount and zero after unmount.
Acceptance criteria
Service
Tests
Out of scope
The locale/theme preference sync to the server (companion issues).
Getting started
Good first files to read: src/app/setting/page.tsx.
Problem
src/app/setting/page.tsxadds a system-preference listener and cleans it up with a different function:removeEventListenerrequires the same function reference; the cleanup registers a new anonymous function, so the originalchangelistener is never removed. Consequences:themestate change re-runs the effect, adding anotherchangelistener that is never removed — the leak grows with every theme toggle, and each leaked listener closes over the oldthemevalue (stale closure: after switching to dark, the leakedsystem-era listeners still check the oldtheme).window.matchMediafor the page's lifetime.() => {}placeholder signals the author knew cleanup was required but wired the wrong reference — exactly the class of bug a lint rule (react-hooks/exhaustive-depswon't catch it) or a test should.Root cause
The cleanup was written with a placeholder handler instead of the registered one.
Why this is architecturally hard
theme(a ref, not the closure), or the listener should be re-created per theme change with correct cleanup.useThemehook (with the media-query handling and localStorage persistence) would centralize it and make the lifecycle testable — the durable fix is extraction, not just the reference fix.matchMediaand asserting add/remove call counts — a test harness the page currently lacks.Proposed design
Extract the theme handling into a
useThemehook (or fix the reference and use athemeReffor the handler), ensuring add/remove use the same reference and unmount removes the listener. Add a test asserting one listener per mount and zero after unmount.Acceptance criteria
Service
Tests
matchMediaand asserts balanced add/remove.Out of scope
The locale/theme preference sync to the server (companion issues).
Getting started
npm testGood first files to read:
src/app/setting/page.tsx.