From 1a91d3608ff3962aa56bf7a44af2a37e73d01f34 Mon Sep 17 00:00:00 2001 From: Miodec Date: Mon, 26 Oct 2026 16:47:22 +0100 Subject: [PATCH 1/3] refactor: replace config-bus subscribers outside the test with store effects (C2) Theme, sound, quotes and challenge controllers and the commandline theme list react via effects on config store keys instead of configEvent. Theme resolves one configured theme; picking a theme turns auto switch off via overrideConfig, so whole-config apply sets autoSwitchTheme last. Effects only run on changed values: theme pickers clear the random theme themselves. Co-Authored-By: Claude Opus 5.5 --- docs/specs/solid-monkeytype/roadmap.md | 2 +- .../commandline/lists/themes.spec.ts | 34 +++++ .../controllers/challenge-controller.spec.ts | 40 +++++ .../controllers/quotes-controller.spec.ts | 21 +++ .../controllers/sound-controller.spec.ts | 60 ++++++++ .../controllers/theme-controller.spec.ts | 67 ++++++++ .../__tests__/root/config-metadata.spec.ts | 30 ++++ frontend/__tests__/root/config.spec.ts | 9 ++ .../commandline/lists/custom-themes-list.ts | 1 + frontend/src/ts/commandline/lists/themes.ts | 40 ++--- .../pages/settings/custom-setting/Theme.tsx | 12 +- frontend/src/ts/config/lifecycle.ts | 1 + frontend/src/ts/config/metadata.tsx | 17 ++- .../ts/controllers/challenge-controller.ts | 47 +++--- .../src/ts/controllers/quotes-controller.ts | 15 +- .../src/ts/controllers/sound-controller.ts | 24 ++- .../src/ts/controllers/theme-controller.ts | 143 +++++++----------- 17 files changed, 415 insertions(+), 148 deletions(-) create mode 100644 frontend/__tests__/commandline/lists/themes.spec.ts create mode 100644 frontend/__tests__/controllers/challenge-controller.spec.ts create mode 100644 frontend/__tests__/controllers/quotes-controller.spec.ts create mode 100644 frontend/__tests__/controllers/sound-controller.spec.ts create mode 100644 frontend/__tests__/controllers/theme-controller.spec.ts diff --git a/docs/specs/solid-monkeytype/roadmap.md b/docs/specs/solid-monkeytype/roadmap.md index f8c6e49dee6e..47a1deb32b70 100644 --- a/docs/specs/solid-monkeytype/roadmap.md +++ b/docs/specs/solid-monkeytype/roadmap.md @@ -99,7 +99,7 @@ Serial; each step needs the previous one. Verify each in the browser (`__mt.chec | Done | ID | Size | Step | Blocked by | Notes | |---|---|---|---|---|---| | [x] | C1 | S · commit | Config setter writes the store **before** dispatching the config event (and before full-config-change finished) | none | Done in [bd2cae5](https://github.com/monkeytypegame/monkeytype/commit/bd2cae518): `setConfig`/`toggleFunbox` write store → persist → event; whole-config apply resets the store with `Config`, writes per key inside one `batch` (effects run once), all before finished. Leftover for C3: funbox `activate()` writes `Config.funbox` without the store | -| [ ] | C2 | S · commit each | Replace config-bus subscribers outside the test with effects on store keys: theme controller, sound controller, quotes controller, challenge controller, commandline theme list | C1 | One PR each, all parallel | +| [ ] | C2 | S · commit each | Replace config-bus subscribers outside the test with effects on store keys: theme controller, sound controller, quotes controller, challenge controller, commandline theme list | C1 | One PR each, all parallel. Effects only run on a changed value: re-picking a setting no longer re-runs its reaction (challenge stays loaded, quote queue not reshuffled); theme pickers clear the random theme themselves. Theme reactions resolve one configured theme; picking a theme turns auto switch off via `overrideConfig`, so whole-config apply sets `autoSwitchTheme` last | | [ ] | C3 | M · commit per area; PR for test modules | Move mutable `Config.x` reads to the store, by area: format singleton; commandline lists; controllers; input; test modules | C1 | Parallel by area; the test-module part follows T3/T6 where they overlap | | [ ] | C4 | L · PR | Delete the mutable config object, the config event bus and the full-config-change events; whole-config apply becomes one batched reconcile (the `batch` already exists since C1); convert remaining test subscribers (pace caret, tts, practise words, funbox, test-logic) to effects | C2, C3, T6, L2 | Last step of C | | [ ] | C5 | M · PR | Merge commandline metadata into config metadata; command icons become icon-component types | none | Can be L-track work | diff --git a/frontend/__tests__/commandline/lists/themes.spec.ts b/frontend/__tests__/commandline/lists/themes.spec.ts new file mode 100644 index 000000000000..c581985335ca --- /dev/null +++ b/frontend/__tests__/commandline/lists/themes.spec.ts @@ -0,0 +1,34 @@ +import { describe, it, expect } from "vitest"; +import { Config, setConfigStore } from "../../../src/ts/config/store"; +import commands from "../../../src/ts/commandline/lists/themes"; +import * as ThemeController from "../../../src/ts/controllers/theme-controller"; +import { __testing } from "../../../src/ts/config/testing"; +import { getTheme } from "../../../src/ts/states/theme"; + +describe("themes commandline list config effects", () => { + const firstTheme = (): string | undefined => + commands[0]?.subgroup?.list[0]?.configValue as string | undefined; + + it("puts new favorites first", () => { + Config.favThemes = ["nord"]; + setConfigStore("favThemes", ["nord"]); + expect(firstTheme()).toBe("nord"); + + Config.favThemes = ["nord", "8008"]; + setConfigStore("favThemes", ["nord", "8008"]); + expect(firstTheme()).toBe("8008"); + }); + + it("drops the random theme when the configured theme is re-picked", async () => { + __testing.replaceConfig({ theme: "serika_dark", randomTheme: "on" }); + await ThemeController.randomizeTheme(); + expect(ThemeController.randomTheme).not.toBeNull(); + + commands[0]?.subgroup?.list + .find((command) => command.configValue === "serika_dark") + ?.exec?.({}); + + expect(ThemeController.randomTheme).toBeNull(); + expect(getTheme().name).toBe("serika_dark"); + }); +}); diff --git a/frontend/__tests__/controllers/challenge-controller.spec.ts b/frontend/__tests__/controllers/challenge-controller.spec.ts new file mode 100644 index 000000000000..014574183a78 --- /dev/null +++ b/frontend/__tests__/controllers/challenge-controller.spec.ts @@ -0,0 +1,40 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { Config as ConfigType } from "@monkeytype/schemas/configs"; +import { Config, setConfigStore } from "../../src/ts/config/store"; +import { __testing } from "../../src/ts/config/testing"; +import "../../src/ts/controllers/challenge-controller"; +import { + getLoadedChallenge, + setLoadedChallenge, +} from "../../src/ts/states/test"; + +const { replaceConfig } = __testing; + +function set(key: K, value: ConfigType[K]): void { + Config[key] = value; + setConfigStore(key, value); +} + +describe("challenge controller config effects", () => { + beforeEach(() => { + replaceConfig({}); + setLoadedChallenge({ name: "oneHourWarrior" } as Parameters< + typeof setLoadedChallenge + >[0]); + }); + + it("clears the challenge on a test setting change", () => { + set("mode", "words"); + expect(getLoadedChallenge()).toBeNull(); + }); + + it("clears the challenge on a funbox change", () => { + set("funbox", ["nausea"]); + expect(getLoadedChallenge()).toBeNull(); + }); + + it("keeps the challenge on other changes", () => { + set("soundVolume", 0.3); + expect(getLoadedChallenge()).not.toBeNull(); + }); +}); diff --git a/frontend/__tests__/controllers/quotes-controller.spec.ts b/frontend/__tests__/controllers/quotes-controller.spec.ts new file mode 100644 index 000000000000..085965079144 --- /dev/null +++ b/frontend/__tests__/controllers/quotes-controller.spec.ts @@ -0,0 +1,21 @@ +import { describe, it, expect, vi } from "vitest"; +import { Config as ConfigType } from "@monkeytype/schemas/configs"; +import { Config, setConfigStore } from "../../src/ts/config/store"; +import quoteController from "../../src/ts/controllers/quotes-controller"; + +function set(key: K, value: ConfigType[K]): void { + Config[key] = value; + setConfigStore(key, value); +} + +describe("quotes controller config effects", () => { + const updateQuoteQueueMock = vi.spyOn(quoteController, "updateQuoteQueue"); + + it("updates the queue on quote length changes", () => { + set("quoteLength", [0, 1]); + expect(updateQuoteQueueMock).toHaveBeenLastCalledWith([0, 1]); + + set("quoteLength", [0, 1, 2]); + expect(updateQuoteQueueMock).toHaveBeenLastCalledWith([0, 1, 2]); + }); +}); diff --git a/frontend/__tests__/controllers/sound-controller.spec.ts b/frontend/__tests__/controllers/sound-controller.spec.ts new file mode 100644 index 000000000000..3e3b10ffb612 --- /dev/null +++ b/frontend/__tests__/controllers/sound-controller.spec.ts @@ -0,0 +1,60 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { Config as ConfigType } from "@monkeytype/schemas/configs"; +import { Config, setConfigStore } from "../../src/ts/config/store"; +import "../../src/ts/controllers/sound-controller"; + +const { volumeMock, howlMock } = vi.hoisted(() => ({ + volumeMock: vi.fn(), + howlMock: vi.fn(), +})); +vi.mock("howler", () => ({ + Howler: { volume: volumeMock }, + Howl: class { + constructor(options: unknown) { + howlMock(options); + } + seek(): void { + // + } + }, +})); + +function set(key: K, value: ConfigType[K]): void { + Config[key] = value; + setConfigStore(key, value); +} + +async function flush(): Promise { + await new Promise((resolve) => setTimeout(resolve)); +} + +describe("sound controller config effects", () => { + beforeEach(() => { + volumeMock.mockClear(); + howlMock.mockClear(); + }); + + it("does nothing at boot", async () => { + await flush(); + expect(volumeMock).not.toHaveBeenCalled(); + expect(howlMock).not.toHaveBeenCalled(); + }); + + it("loads the click sounds when turned on", async () => { + set("playSoundOnClick", "2"); + await flush(); + expect(howlMock).toHaveBeenCalled(); + }); + + it("loads nothing when turned off", async () => { + set("playSoundOnClick", "off"); + await flush(); + expect(howlMock).not.toHaveBeenCalled(); + }); + + it("sets the volume", async () => { + set("soundVolume", 0.3); + await flush(); + expect(volumeMock).toHaveBeenCalledWith(0.3); + }); +}); diff --git a/frontend/__tests__/controllers/theme-controller.spec.ts b/frontend/__tests__/controllers/theme-controller.spec.ts new file mode 100644 index 000000000000..bfb9300b376e --- /dev/null +++ b/frontend/__tests__/controllers/theme-controller.spec.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { Config as ConfigType } from "@monkeytype/schemas/configs"; +import * as ThemeController from "../../src/ts/controllers/theme-controller"; +import { Config, setConfigStore } from "../../src/ts/config/store"; +import { __testing } from "../../src/ts/config/testing"; +import { getTheme } from "../../src/ts/states/theme"; +import { getThemeIndicator, setThemeIndicator } from "../../src/ts/states/core"; + +const { replaceConfig } = __testing; + +function set(key: K, value: ConfigType[K]): void { + Config[key] = value; + setConfigStore(key, value); +} + +describe("theme controller config effects", () => { + beforeEach(() => { + replaceConfig({ theme: "serika_dark" }); + }); + + it("applies the theme", () => { + set("theme", "8008"); + expect(getTheme().name).toBe("8008"); + }); + + it("applies the custom theme and its colors", () => { + set("customTheme", true); + expect(getTheme().name).toBe("custom"); + + const colors = [ + ...Config.customThemeColors, + ] as ConfigType["customThemeColors"]; + colors[0] = "#123456"; + set("customThemeColors", colors); + expect(getTheme().bg).toBe("#123456"); + }); + + it("applies the light theme on auto switch", () => { + set("themeLight", "nord_light"); + set("autoSwitchTheme", true); + expect(getTheme().name).toBe("nord_light"); + + set("autoSwitchTheme", false); + expect(getTheme().name).toBe("serika_dark"); + }); + + it("keeps the random theme over a custom theme", async () => { + set("customTheme", true); + set("randomTheme", "on"); + + await ThemeController.randomizeTheme(); + await new Promise((resolve) => setTimeout(resolve)); + + expect(ThemeController.randomTheme).not.toBeNull(); + expect(getTheme().name).toBe(ThemeController.randomTheme); + expect(Config.customTheme).toBe(false); + }); + + it("updates the indicator on favorite changes", () => { + setThemeIndicator({ text: "", isFavorite: false }); + set("favThemes", ["serika_dark"]); + expect(getThemeIndicator()).toEqual({ + text: "serika dark", + isFavorite: true, + }); + }); +}); diff --git a/frontend/__tests__/root/config-metadata.spec.ts b/frontend/__tests__/root/config-metadata.spec.ts index 80b725a2274e..d0a13b195af7 100644 --- a/frontend/__tests__/root/config-metadata.spec.ts +++ b/frontend/__tests__/root/config-metadata.spec.ts @@ -310,6 +310,36 @@ describe("ConfigMeta", () => { given: { customTheme: true }, expected: { customTheme: false }, }, + { + value: "8008", + given: { autoSwitchTheme: true }, + expected: { autoSwitchTheme: false }, + }, + ], + customTheme: [ + { + value: true, + given: { autoSwitchTheme: true }, + expected: { autoSwitchTheme: false }, + }, + ], + customThemeColors: [ + { + value: [ + "#000", + "#111", + "#222", + "#333", + "#444", + "#555", + "#666", + "#777", + "#888", + "#999", + ], + given: { autoSwitchTheme: true }, + expected: { autoSwitchTheme: false }, + }, ], keymapLayout: [ { diff --git a/frontend/__tests__/root/config.spec.ts b/frontend/__tests__/root/config.spec.ts index e230b75b8005..5a88ead8efe0 100644 --- a/frontend/__tests__/root/config.spec.ts +++ b/frontend/__tests__/root/config.spec.ts @@ -527,6 +527,15 @@ describe("Config", () => { expect(getConfigStore.funbox).toEqual([]); }); + it("applyConfig keeps auto switch theme next to a theme", async () => { + await Lifecycle.applyConfig({ + theme: "8008", + customTheme: true, + autoSwitchTheme: true, + }); + expect(getConfig().autoSwitchTheme).toBe(true); + }); + it("applyConfig runs store effects once", async () => { replaceConfig({ mode: "time", punctuation: false, numbers: false }); const runs: unknown[] = []; diff --git a/frontend/src/ts/commandline/lists/custom-themes-list.ts b/frontend/src/ts/commandline/lists/custom-themes-list.ts index f5f9e574278b..c4ace2a90669 100644 --- a/frontend/src/ts/commandline/lists/custom-themes-list.ts +++ b/frontend/src/ts/commandline/lists/custom-themes-list.ts @@ -40,6 +40,7 @@ export function update(): void { ThemeController.preview("custom", theme.colors); }, exec: (): void => { + void ThemeController.clearRandom(); setConfig("customTheme", true); setConfig("customThemeColors", theme.colors); }, diff --git a/frontend/src/ts/commandline/lists/themes.ts b/frontend/src/ts/commandline/lists/themes.ts index 157be86f1868..b6d287475ad2 100644 --- a/frontend/src/ts/commandline/lists/themes.ts +++ b/frontend/src/ts/commandline/lists/themes.ts @@ -1,11 +1,12 @@ -import { Config } from "../../config/store"; +import { createRoot } from "solid-js"; +import { Config, getConfig } from "../../config/store"; import { setConfig } from "../../config/setters"; import { capitalizeFirstLetterOfEachWord } from "../../utils/strings"; import * as ThemeController from "../../controllers/theme-controller"; import { Command, CommandsSubgroup } from "../types"; import { ThemesList, ThemeWithName } from "../../constants/themes"; import { not } from "@monkeytype/util/predicates"; -import { configEvent } from "../../events/config"; +import { createEffectOn } from "../../hooks/effects"; import * as getErrorMessage from "../../utils/error"; const isFavorite = (theme: ThemeWithName): boolean => @@ -33,6 +34,7 @@ const createThemeCommand = (theme: ThemeWithName): Command => { ThemeController.preview(theme.name); }, exec: (): void => { + void ThemeController.clearRandom(); setConfig("theme", theme.name); }, }; @@ -75,21 +77,25 @@ export function update(themes: ThemeWithName[]): void { ); } -// subscribe to theme-related config events to update the theme command list -configEvent.subscribe(({ key }) => { - if (key === "favThemes") { - // update themes list when favorites change - try { - update(ThemesList); - } catch (e: unknown) { - console.error( - getErrorMessage.createErrorMessage( - e, - "Failed to update themes commands", - ), - ); - } - } +// a root so the effect is owned, it lives as long as the page +createRoot(() => { + // rebuild with the new favorites first + createEffectOn( + () => [...getConfig.favThemes], + () => { + try { + update(ThemesList); + } catch (e: unknown) { + console.error( + getErrorMessage.createErrorMessage( + e, + "Failed to update themes commands", + ), + ); + } + }, + { defer: true }, + ); }); export default commands; diff --git a/frontend/src/ts/components/pages/settings/custom-setting/Theme.tsx b/frontend/src/ts/components/pages/settings/custom-setting/Theme.tsx index 8136b64d7280..54c752716dd6 100644 --- a/frontend/src/ts/components/pages/settings/custom-setting/Theme.tsx +++ b/frontend/src/ts/components/pages/settings/custom-setting/Theme.tsx @@ -18,6 +18,7 @@ import { ThemeWithName, } from "../../../../constants/themes"; import { + clearRandom, convertCustomColorsToTheme, convertThemeToCustomColors, } from "../../../../controllers/theme-controller"; @@ -258,12 +259,18 @@ export function Theme(): JSXElement { inputs={