refactor(theme): simplify ThemeProvider — remove legacy migration, hardcode storage keys, add system reactivity

Author: IzumiSy

packages/core/src/components/appshell.tsx

@@ -339,12 +339,7 @@ export const AppShell = (props: AppShellProps) => {
       <AppShellDataContext.Provider value={dataValue}>
         <BreadcrumbOverrideProvider>
           <CommandPaletteProvider searchSources={props.searchSources}>
-            <ThemeProvider
-              defaultTheme={props.defaultTheme}
-              defaultFont={props.defaultFont}
-              storageKey="appshell-ui-theme"
-              fontStorageKey="appshell-ui-font"
-            >
+            <ThemeProvider defaultTheme={props.defaultTheme} defaultFont={props.defaultFont}>
               <RouterContainer>
                 {props.children}
                 <BuiltInCommandPalette />

packages/core/src/contexts/theme-context.test.tsx

@@ -1,10 +1,7 @@
-import { cleanup, render, waitFor } from "@testing-library/react";
+import { cleanup, render, act, waitFor } from "@testing-library/react";
 import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
 
-import { ThemeProvider, useTheme, useFont } from "./theme-context";
-
-const storageKey = "theme-context-test-theme";
-const fontStorageKey = "theme-context-test-font";
+import { ThemeProvider, useTheme } from "./theme-context";
 
 /** happy-dom / Node can omit a full `localStorage`; ThemeProvider persists via it. */
 function installLocalStorageStub() {
@@ -27,8 +24,12 @@ function installLocalStorageStub() {
   return map;
 }
 
+type MatchMediaListener = (e: MediaQueryListEvent) => void;
+let matchMediaListeners: MatchMediaListener[] = [];
+
 /** `matchMedia` is not implemented in some test runtimes — stub to a controllable shape. */
 function installMatchMediaStub(matches: boolean) {
+  matchMediaListeners = [];
   Object.defineProperty(window, "matchMedia", {
     configurable: true,
     value: vi.fn().mockImplementation((query: string) => ({
@@ -37,8 +38,12 @@ function installMatchMediaStub(matches: boolean) {
       onchange: null,
       addListener: vi.fn(),
       removeListener: vi.fn(),
-      addEventListener: vi.fn(),
-      removeEventListener: vi.fn(),
+      addEventListener: (_: string, fn: MatchMediaListener) => {
+        matchMediaListeners.push(fn);
+      },
+      removeEventListener: (_: string, fn: MatchMediaListener) => {
+        matchMediaListeners = matchMediaListeners.filter((l) => l !== fn);
+      },
       dispatchEvent: vi.fn(),
     })),
   });
@@ -52,6 +57,7 @@ beforeAll(() => {
 
 beforeEach(() => {
   storageMap.clear();
+  matchMediaListeners = [];
   document.documentElement.removeAttribute("data-theme");
   document.documentElement.removeAttribute("data-font");
   document.documentElement.classList.remove("light", "dark");
@@ -72,62 +78,58 @@ function ThemeProbe() {
 }
 
 function FontProbe() {
-  const { font } = useFont();
+  const { font } = useTheme();
   return <span data-testid="font">{font}</span>;
 }
 
-describe("ThemeProvider — legacy id migration", () => {
-  it.each([
-    ["tailor-light", "cream"],
-    ["tailor-bloom", "bloom"],
-    ["tailor-dark", "dark"],
-  ])("maps stored %s → %s on first render", async (stored, expected) => {
-    storageMap.set(storageKey, stored);
+describe("ThemeProvider — storage validation", () => {
+  it("falls back to defaultTheme for an unrecognized stored value", () => {
+    storageMap.set("appshell-ui-theme", "totally-not-a-theme");
 
     const { getByTestId } = render(
-      <ThemeProvider storageKey={storageKey} fontStorageKey={fontStorageKey} defaultTheme="bloom">
+      <ThemeProvider defaultTheme="light">
         <ThemeProbe />
       </ThemeProvider>,
     );
 
-    expect(getByTestId("theme").textContent).toBe(expected);
-    await waitFor(() => {
-      expect(document.documentElement.dataset.theme).toBe(expected);
-    });
+    expect(getByTestId("theme").textContent).toBe("light");
   });
 
-  it("falls back to defaultTheme for an unrecognized stored value", () => {
-    storageMap.set(storageKey, "totally-not-a-theme");
+  it("falls back to defaultFont for an unrecognized stored font", () => {
+    storageMap.set("appshell-ui-font", "wingdings");
 
     const { getByTestId } = render(
-      <ThemeProvider storageKey={storageKey} fontStorageKey={fontStorageKey} defaultTheme="light">
-        <ThemeProbe />
+      <ThemeProvider defaultFont="inter">
+        <FontProbe />
       </ThemeProvider>,
     );
 
-    expect(getByTestId("theme").textContent).toBe("light");
+    expect(getByTestId("font").textContent).toBe("inter");
   });
 
-  it("falls back to defaultFont for an unrecognized stored font", () => {
-    storageMap.set(fontStorageKey, "wingdings");
+  it("reads a valid stored theme", async () => {
+    storageMap.set("appshell-ui-theme", "cream");
 
     const { getByTestId } = render(
-      <ThemeProvider storageKey={storageKey} fontStorageKey={fontStorageKey} defaultFont="inter">
-        <FontProbe />
+      <ThemeProvider>
+        <ThemeProbe />
       </ThemeProvider>,
     );
 
-    expect(getByTestId("font").textContent).toBe("inter");
+    expect(getByTestId("theme").textContent).toBe("cream");
+    await waitFor(() => {
+      expect(document.documentElement.dataset.theme).toBe("cream");
+    });
   });
 });
 
 describe("ThemeProvider — system resolution", () => {
   it("resolves system → dark when prefers-color-scheme: dark matches", async () => {
     installMatchMediaStub(true);
-    storageMap.set(storageKey, "system");
+    storageMap.set("appshell-ui-theme", "system");
 
     const { getByTestId } = render(
-      <ThemeProvider storageKey={storageKey} fontStorageKey={fontStorageKey}>
+      <ThemeProvider>
         <ThemeProbe />
       </ThemeProvider>,
     );
@@ -142,10 +144,10 @@ describe("ThemeProvider — system resolution", () => {
 
   it("resolves system → light when prefers-color-scheme: dark does not match", async () => {
     installMatchMediaStub(false);
-    storageMap.set(storageKey, "system");
+    storageMap.set("appshell-ui-theme", "system");
 
     const { getByTestId } = render(
-      <ThemeProvider storageKey={storageKey} fontStorageKey={fontStorageKey}>
+      <ThemeProvider>
         <ThemeProbe />
       </ThemeProvider>,
     );
@@ -156,19 +158,37 @@ describe("ThemeProvider — system resolution", () => {
       expect(document.documentElement.classList.contains("light")).toBe(true);
     });
   });
+
+  it("reacts to OS color-scheme change while theme is system", async () => {
+    installMatchMediaStub(false);
+    storageMap.set("appshell-ui-theme", "system");
+
+    const { getByTestId } = render(
+      <ThemeProvider>
+        <ThemeProbe />
+      </ThemeProvider>,
+    );
+
+    expect(getByTestId("resolved").textContent).toBe("light");
+
+    // Simulate OS dark mode toggle
+    act(() => {
+      for (const listener of matchMediaListeners) {
+        listener({ matches: true } as MediaQueryListEvent);
+      }
+    });
+
+    expect(getByTestId("resolved").textContent).toBe("dark");
+    await waitFor(() => {
+      expect(document.documentElement.dataset.theme).toBe("dark");
+    });
+  });
 });
 
-describe("useTheme / useFont — provider guard", () => {
+describe("useTheme — provider guard", () => {
   it("throws when useTheme is called outside ThemeProvider", () => {
-    // Silence React's expected error log for this assertion.
     const spy = vi.spyOn(console, "error").mockImplementation(() => {});
     expect(() => render(<ThemeProbe />)).toThrow(/useTheme must be used within a ThemeProvider/);
     spy.mockRestore();
   });
-
-  it("throws when useFont is called outside ThemeProvider", () => {
-    const spy = vi.spyOn(console, "error").mockImplementation(() => {});
-    expect(() => render(<FontProbe />)).toThrow(/useFont must be used within a ThemeProvider/);
-    spy.mockRestore();
-  });
 });

packages/core/src/contexts/theme-context.tsx

@@ -10,6 +10,7 @@ const ALL_THEMES: readonly Theme[] = ["light", "dark", "cream", "bloom", "system
 
 /** Dropdown / switcher entries: order matches selectable themes; labels are user-facing. */
 export type ThemeOption = { readonly value: Theme; readonly label: string };
+export type FontOption = { readonly value: Font; readonly label: string };
 
 export const THEME_OPTIONS: readonly ThemeOption[] = [
   { value: "bloom", label: "Bloom" },
@@ -24,67 +25,28 @@ export type Font = "geist" | "inter";
 
 const ALL_FONTS: readonly Font[] = ["geist", "inter"] as const;
 
-export type FontOption = { readonly value: Font; readonly label: string };
-
 export const FONT_OPTIONS: readonly FontOption[] = [
   { value: "geist", label: "Geist" },
   { value: "inter", label: "Inter" },
 ] as const;
 
-/** Migrate stored values from legacy `tailor-*` ids before the public rename. */
-const LEGACY_THEME_IDS: Partial<Record<string, Theme>> = {
-  "tailor-light": "cream",
-  "tailor-bloom": "bloom",
-  "tailor-dark": "dark",
-};
-
-function parseStoredTheme(value: string | null, fallback: Theme): Theme {
-  if (!value) return fallback;
-  const legacy = LEGACY_THEME_IDS[value];
-  if (legacy) return legacy;
-  if ((ALL_THEMES as readonly string[]).includes(value)) return value as Theme;
-  return fallback;
-}
-
-function parseStoredFont(value: string | null, fallback: Font): Font {
-  if (!value) return fallback;
-  if ((ALL_FONTS as readonly string[]).includes(value)) return value as Font;
-  return fallback;
-}
+const THEME_STORAGE_KEY = "appshell-ui-theme";
+const FONT_STORAGE_KEY = "appshell-ui-font";
 
-function readStored<T extends string>(
-  storageKey: string,
-  fallback: T,
-  parse: (value: string | null, fallback: T) => T,
-): T {
+function readStored<T extends string>(key: string, allowList: readonly T[], fallback: T): T {
   if (typeof window === "undefined") return fallback;
-  const ls = window.localStorage;
-  const getItem = ls && typeof ls.getItem === "function" ? ls.getItem.bind(ls) : null;
-  if (!getItem) return fallback;
   try {
-    return parse(getItem(storageKey), fallback);
+    const v = localStorage.getItem(key);
+    return v && (allowList as readonly string[]).includes(v) ? (v as T) : fallback;
   } catch {
     return fallback;
   }
 }
 
-function writeStored<T extends string>(storageKey: string, value: T) {
-  if (typeof window === "undefined") return;
-  const ls = window.localStorage;
-  if (!ls || typeof ls.setItem !== "function") return;
-  try {
-    ls.setItem(storageKey, value);
-  } catch {
-    /* storage full or forbidden */
-  }
-}
-
 type ThemeProviderProps = {
   children: React.ReactNode;
   defaultTheme?: Theme;
   defaultFont?: Font;
-  storageKey: string;
-  fontStorageKey: string;
 };
 
 type ThemeProviderState = {
@@ -97,27 +59,36 @@ type ThemeProviderState = {
 
 const ThemeProviderContext = createContext<ThemeProviderState | undefined>(undefined);
 
-function resolveTheme(theme: Theme): ResolvedTheme {
-  if (theme !== "system") return theme;
-  if (typeof window === "undefined") return "light";
-  return window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light";
-}
-
 export function ThemeProvider({
   children,
-  storageKey,
-  fontStorageKey,
   defaultTheme = "bloom",
   defaultFont = "geist",
 }: ThemeProviderProps) {
   const [theme, setThemeState] = useState<Theme>(() =>
-    readStored(storageKey, defaultTheme, parseStoredTheme),
+    readStored(THEME_STORAGE_KEY, ALL_THEMES, defaultTheme),
   );
   const [font, setFontState] = useState<Font>(() =>
-    readStored(fontStorageKey, defaultFont, parseStoredFont),
+    readStored(FONT_STORAGE_KEY, ALL_FONTS, defaultFont),
   );
 
-  const resolvedTheme = useMemo(() => resolveTheme(theme), [theme]);
+  // Track OS color-scheme for live "system" theme resolution
+  const [systemDark, setSystemDark] = useState(() =>
+    typeof window !== "undefined"
+      ? window.matchMedia("(prefers-color-scheme: dark)").matches
+      : false,
+  );
+
+  useEffect(() => {
+    const mql = window.matchMedia("(prefers-color-scheme: dark)");
+    const handler = (e: MediaQueryListEvent) => setSystemDark(e.matches);
+    mql.addEventListener("change", handler);
+    return () => mql.removeEventListener("change", handler);
+  }, []);
+
+  const resolvedTheme: ResolvedTheme = (() => {
+    if (theme !== "system") return theme;
+    return systemDark ? "dark" : "light";
+  })();
 
   useEffect(() => {
     const root = window.document.documentElement;
@@ -130,21 +101,23 @@ export function ThemeProvider({
     window.document.documentElement.dataset.font = font;
   }, [font]);
 
-  const setTheme = useCallback(
-    (newTheme: Theme) => {
-      writeStored(storageKey, newTheme);
-      setThemeState(newTheme);
-    },
-    [storageKey],
-  );
-
-  const setFont = useCallback(
-    (newFont: Font) => {
-      writeStored(fontStorageKey, newFont);
-      setFontState(newFont);
-    },
-    [fontStorageKey],
-  );
+  const setTheme = useCallback((newTheme: Theme) => {
+    try {
+      localStorage.setItem(THEME_STORAGE_KEY, newTheme);
+    } catch {
+      /* storage full or forbidden */
+    }
+    setThemeState(newTheme);
+  }, []);
+
+  const setFont = useCallback((newFont: Font) => {
+    try {
+      localStorage.setItem(FONT_STORAGE_KEY, newFont);
+    } catch {
+      /* storage full or forbidden */
+    }
+    setFontState(newFont);
+  }, []);
 
   const value = useMemo<ThemeProviderState>(
     () => ({ theme, resolvedTheme, setTheme, font, setFont }),
@@ -157,15 +130,5 @@ export function ThemeProvider({
 export const useTheme = () => {
   const context = useContext(ThemeProviderContext);
   if (context === undefined) throw new Error("useTheme must be used within a ThemeProvider");
-
-  const { theme, resolvedTheme, setTheme } = context;
-  return useMemo(() => ({ theme, resolvedTheme, setTheme }), [theme, resolvedTheme, setTheme]);
-};
-
-export const useFont = () => {
-  const context = useContext(ThemeProviderContext);
-  if (context === undefined) throw new Error("useFont must be used within a ThemeProvider");
-
-  const { font, setFont } = context;
-  return useMemo(() => ({ font, setFont }), [font, setFont]);
+  return context;
 };

packages/core/src/index.ts

@@ -26,7 +26,6 @@ export { WithGuard, type WithGuardProps } from "./components/with-guard";
 export { useAppShell, useAppShellConfig, useAppShellData } from "./contexts/appshell-context";
 export {
   useTheme,
-  useFont,
   THEME_OPTIONS,
   FONT_OPTIONS,
   type ResolvedTheme,