feat: use persisted theme css to fix flashes on header #1784
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds persisted Pinia state for the theme store (new THEME_STORAGE_KEY), registers pinia-plugin-persistedstate on a shared global Pinia instance, refactors theme store wiring to support hydration/sanitization, updates component calls, expands tests to exercise hydration and persistence, and adds the plugin dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as App / Component
participant Wrapper as useThemeStore (wrapper)
participant Store as theme store (base)
participant GlobalPinia as globalPinia + plugin
participant LS as localStorage
Component->>Wrapper: request theme store (optional pinia)
Wrapper->>GlobalPinia: resolve Pinia (arg or shared)
Wrapper->>Store: instantiate or return store
GlobalPinia->>LS: plugin reads `THEME_STORAGE_KEY`
alt persisted state exists
LS-->>GlobalPinia: return persisted state
GlobalPinia->>Store: hydrate store (afterHydrate)
Store->>Store: sanitize/map hydrated publicTheme
Store->>Document: apply theme (classList, CSS vars)
else
Store->>Store: use defaults
end
Component->>Store: setTheme(...)
Store->>GlobalPinia: state mutation triggers persist
GlobalPinia->>LS: write `THEME_STORAGE_KEY`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to inspect closely:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
elibosley
force-pushed
the
codex/cache-theme-css-for-faster-load
branch
from
3b7b043 to
e6397e9
Compare
elibosley
changed the title
💡 Codex ReviewLines 63 to 74 in 3b7b043  Fix persisted-state config to avoid storing hasServerTheme
The persisted-state plugin is wired with ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1784 +/- ##
==========================================
+ Coverage 52.23% 52.27% +0.03%
==========================================
Files 872 872
Lines 50219 50267 +48
Branches 5009 5019 +10
==========================================
+ Hits 26234 26278 +44
- Misses 23909 23914 +5
+ Partials 76 75 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/store/theme.ts (1)
33-52: Remove unsupported manual plugin invocation; use standardpinia.use()registrationThe manual per-store invocation of
createPersistedStatewith a hand-craftedPiniaPluginContextis not a documented or supported pattern inpinia-plugin-persistedstate@4.7.1. The official approach is to register the plugin viapinia.use(createPersistedState(...)), and the non-null assertion onappdoesn't address the underlying architectural problem.The
appparameter is not required for client-side localStorage/sessionStorage usage, so your persistence should work fine with the standard registration approach. Replace the customensureThemePersistencelogic inuseThemeStorewith standard plugin registration in your Pinia setup, and remove the manual context construction inweb/src/store/theme.ts:54–81.
🧹 Nitpick comments (2)
web/src/components/DevThemeSwitcher.standalone.vue (1)
54-56: Updated setTheme calls align with new API; consider relying on store’s CSS var watcherRemoving the old boolean flag and calling
themeStore.setTheme({ name: ... })matches the new(data, options?)signature and keeps this component focused on its own URL/localStorage handling. Given the store nowwatchesthemewith{ immediate: true }and callssetCssVarsitself, the explicitthemeStore.setCssVars()calls here are redundant; you could drop them later if you want to cut a bit of duplicated work.Also applies to: 103-105
web/__test__/store/theme.test.ts (1)
30-52: Test setup uses sharedglobalPiniainstance; official Pinia v3 docs recommend fresh instances per testThe Pinia v3 testing documentation recommends calling setActivePinia(createPinia()) so useX() picks a fresh instance in each test rather than reusing a shared instance. While manual cleanup (deleting state, disposing the store, clearing localStorage) works and provides some isolation, creating a fresh Pinia instance in
beforeEachwould better align with official best practices and reduce the risk of cross-test contamination.Consider refactoring
beforeEachto use:beforeEach(() => { const pinia = createPinia(); setActivePinia(pinia); store = undefined; // ... rest of setup });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
web/__test__/store/theme.test.ts(10 hunks)web/package.json(1 hunks)web/src/components/DevThemeSwitcher.standalone.vue(2 hunks)web/src/store/theme.ts(8 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to function correctly.
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
📚 Learning: 2025-02-21T18:40:10.810Z
Learnt from: elibosley
Repo: unraid/api PR: 1181
File: web/store/theme.ts:210-216
Timestamp: 2025-02-21T18:40:10.810Z
Learning: When updating theme-related CSS variables via `cssText`, preserve existing non-theme styles by filtering out only theme-related rules (those starting with '--') and combining them with the new theme styles.
Applied to files:
web/src/components/DevThemeSwitcher.standalone.vueweb/src/store/theme.ts
📚 Learning: 2025-02-20T15:52:56.733Z
Learnt from: elibosley
Repo: unraid/api PR: 1155
File: web/store/theme.ts:49-50
Timestamp: 2025-02-20T15:52:56.733Z
Learning: CSS variable names in the theme store should be concise and follow established patterns. For example, prefer '--gradient-start' over '--color-customgradient-start' to maintain consistency with other variable names.
Applied to files:
web/src/components/DevThemeSwitcher.standalone.vue
📚 Learning: 2025-02-05T14:43:25.062Z
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/package.json:1-8
Timestamp: 2025-02-05T14:43:25.062Z
Learning: The repository uses Renovate for automated dependency updates, making strict version pinning in package.json less critical as updates are handled automatically through PRs.
Applied to files:
web/package.json
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Applied to files:
web/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to function correctly.
Applied to files:
web/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Applied to files:
web/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Applied to files:
web/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-01-22T18:34:06.925Z
Learnt from: elibosley
Repo: unraid/api PR: 1068
File: api/src/unraid-api/auth/api-key.service.ts:122-137
Timestamp: 2025-01-22T18:34:06.925Z
Learning: The store in app/store is implemented using Redux's configureStore, where dispatch operations for config updates are synchronous in-memory state updates that cannot fail, making transaction-like patterns unnecessary.
Applied to files:
web/__test__/store/theme.test.ts
📚 Learning: 2025-02-20T15:52:58.297Z
Learnt from: elibosley
Repo: unraid/api PR: 1155
File: web/store/theme.ts:161-172
Timestamp: 2025-02-20T15:52:58.297Z
Learning: The banner gradient implementation in web/store/theme.ts doesn't require explicit error handling for hexToRgba as CSS gracefully handles invalid values by ignoring them.
Applied to files:
web/__test__/store/theme.test.ts
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/store/theme.ts
📚 Learning: 2024-12-17T14:59:32.458Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/store/theme.ts:46-49
Timestamp: 2024-12-17T14:59:32.458Z
Learning: In the `web/store/theme.ts` file of the Unraid web application, the header is intentionally designed to have a light background with dark text in dark mode, and a dark background with light text in light mode.
Applied to files:
web/src/store/theme.ts
📚 Learning: 2025-02-24T14:51:21.328Z
Learnt from: elibosley
Repo: unraid/api PR: 1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
Applied to files:
web/src/store/theme.ts
🧬 Code graph analysis (1)
web/__test__/store/theme.test.ts (1)
web/src/store/theme.ts (2)
useThemeStore(330-337)THEME_STORAGE_KEY(33-33)
🪛 GitHub Actions: CI - Main (API)
web/src/store/theme.ts
[error] 64-64: Type 'App | undefined' is not assignable to type 'App'.
🔇 Additional comments (4)
web/package.json (1)
134-136: pinia-plugin-persistedstate dependency looks appropriate; confirm version compatibilityAdding
pinia-plugin-persistedstateas a runtime dependency alongsidepiniamakes sense for the new persistence behavior. Please just double‑check that4.7.1is explicitly compatible withpinia@3.0.3in this repo’s environment (ESM, Vite, Nuxt module usage, etc.), and bump if needed via your normal Renovate flow.web/__test__/store/theme.test.ts (1)
239-276: Persistence tests correctly exercise hydration and caching behaviorThe new tests that (1) preload
localStorageunderTHEME_STORAGE_KEYand verify the store hydrates that theme, and (2) assert that server‑sourced themes end up serialized as{ theme: serverTheme }under the same key, are well aligned with the store’spick: ['theme']configuration. This should give good protection against regressions in both cache shape and persistence flow.web/src/store/theme.ts (2)
82-98: Theme sanitization and source-aware setTheme logic look solid
sanitizeThemeprovides a strict, typed normalization layer for both server and persisted data, andmapPublicThemecleanly translates the GraphQL shape into that internalThemestructure. The newThemeSource‑awaresetThemeimplementation correctly:
- Marks server themes via
hasServerTheme,- Prevents subsequent “local” updates from overriding a server theme unless
devOverrideis enabled, and- Always writes a fully sanitized
Themeinto state.This should significantly reduce the risk of malformed data (from cache or API) breaking CSS or class computations.
Also applies to: 124-147, 179-198
309-315: All external setTheme call sites have been properly updated to the new signatureVerification confirms that the entire codebase has successfully migrated to the new
setTheme(data?: Partial<Theme>, options?: { source?: ThemeSource })signature. A search across 21 call sites found zero instances of the old boolean second argument pattern. All callers now properly use either no arguments, a data object, or the new options object format.
pujitm
left a comment
pujitm
left a comment
There was a problem hiding this comment.
qq about Object.assign, lgtm otherwise
web/src/store/theme.ts
Outdated
| setDevOverride, | ||
| }; | ||
| }); | ||
| Object.assign(useThemeStore, baseUseThemeStore); |
There was a problem hiding this comment.
I haven't seen this application of Object.assign before -- what's supposed to happen here?
There was a problem hiding this comment.
Honestly was a lot of code remnants from AI refactor + the fact we used to pass pinia in manually. Removed all of this logic in favor of much simplified flows.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/__test__/store/theme.test.ts (1)
5-6: Good alignment of test harness withglobalPiniaand DOM stateUsing
createApp+app.use(globalPinia)together withsetActivePinia(globalPinia)ensures tests exercise the same shared Pinia instance as production. ClearinglocalStorageand deletingglobalPinia.state.value.themebefore each run, then disposing the store and unmounting the app afterward, avoids stale persisted state or watchers leaking across tests. The extradelete globalPinia.state.value.themeon top of$dispose()is a bit redundant but makes the reset explicit and is fine to keep.Also applies to: 12-15, 31-37, 38-48, 64-69, 79-85
web/src/store/theme.ts (1)
170-272: CSS variable/class application is well-structured; consider a safe fallback for unknown themesCentralizing DOM updates in
setCssVarswith a singlerequestAnimationFramecallback and targeting bothdocumentElementand.unapiroots is a good fit for your web-component setup. Dynamic variables andhas-custom-*classes are applied/cleaned predictably, and the tests asserting calls todocument.documentElement.style.setPropertyand activeColorVariables confirm the behavior.One minor robustness tweak: if a new theme name is introduced on the server before
defaultColorsis updated,defaultColors[selectedTheme]could beundefined, and spreading it would throw. You can cheaply guard against that by falling back to a known palette (e.g., the default/white theme):- // Store active variables for reference (from defaultColors for compatibility) - const customTheme = { ...defaultColors[selectedTheme] }; - activeColorVariables.value = customTheme; + // Store active variables for reference (from defaultColors for compatibility) + const baseTheme = defaultColors[selectedTheme] ?? defaultColors.white; + const customTheme = { ...baseTheme }; + activeColorVariables.value = customTheme;The double application of the
'dark'class (viathemeClassesand then explicitly) is mostly cosmetic; you could simplify it later if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/__test__/store/theme.test.ts(10 hunks)web/src/store/globalPinia.ts(1 hunks)web/src/store/theme.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to function correctly.
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Applied to files:
web/src/store/globalPinia.tsweb/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Applied to files:
web/src/store/globalPinia.tsweb/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to function correctly.
Applied to files:
web/src/store/globalPinia.tsweb/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
Repo: unraid/api PR: 0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Applied to files:
web/src/store/globalPinia.tsweb/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
Repo: unraid/api PR: 1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/store/globalPinia.tsweb/__test__/store/theme.test.tsweb/src/store/theme.ts
📚 Learning: 2025-01-22T18:34:06.925Z
Learnt from: elibosley
Repo: unraid/api PR: 1068
File: api/src/unraid-api/auth/api-key.service.ts:122-137
Timestamp: 2025-01-22T18:34:06.925Z
Learning: The store in app/store is implemented using Redux's configureStore, where dispatch operations for config updates are synchronous in-memory state updates that cannot fail, making transaction-like patterns unnecessary.
Applied to files:
web/__test__/store/theme.test.ts
📚 Learning: 2025-02-20T15:52:58.297Z
Learnt from: elibosley
Repo: unraid/api PR: 1155
File: web/store/theme.ts:161-172
Timestamp: 2025-02-20T15:52:58.297Z
Learning: The banner gradient implementation in web/store/theme.ts doesn't require explicit error handling for hexToRgba as CSS gracefully handles invalid values by ignoring them.
Applied to files:
web/__test__/store/theme.test.ts
📚 Learning: 2025-02-24T14:51:21.328Z
Learnt from: elibosley
Repo: unraid/api PR: 1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
Applied to files:
web/src/store/theme.ts
📚 Learning: 2024-12-17T14:59:32.458Z
Learnt from: elibosley
Repo: unraid/api PR: 972
File: web/store/theme.ts:46-49
Timestamp: 2024-12-17T14:59:32.458Z
Learning: In the `web/store/theme.ts` file of the Unraid web application, the header is intentionally designed to have a light background with dark text in dark mode, and a dark background with light text in light mode.
Applied to files:
web/src/store/theme.ts
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
Repo: unraid/api PR: 975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/store/theme.ts
🧬 Code graph analysis (2)
web/__test__/store/theme.test.ts (2)
web/src/store/theme.ts (2)
useThemeStore(307-310)THEME_STORAGE_KEY(32-32)web/src/store/globalPinia.ts (1)
globalPinia(6-6)
web/src/store/theme.ts (1)
web/src/store/globalPinia.ts (1)
globalPinia(6-6)
🔇 Additional comments (7)
web/__test__/store/theme.test.ts (2)
79-85:createStorehelper simplifies store reuse and verifies persistence wiringThe
createStorehelper ensures each test reuses a singleuseThemeStoreinstance per run and centralizes initialization logic. Asserting thattypeof store.$persist === 'function'is a nice way to lock in the contract that the persistedstate plugin is correctly attached to the theme store when used viaglobalPinia.Also applies to: 88-92, 106-107, 121-122, 148-149, 165-166, 181-182, 212-213
244-281: Server theme precedence test is needed to complete coverageThe two tests at lines 244–281 correctly exercise cache hydration and server persistence. However, they don't verify the precedence behavior when both a cached theme and server
publicThemeexist—per the store implementation, the server theme should override the cache, but this scenario isn't tested.The review suggestion to mock
result.value.publicThemeand seedlocalStoragesimultaneously is valid: it would confirm that server data takes precedence during initialization. Because the mock currently returnsresult: ref(null), the initialif (result.value?.publicTheme)block in the store never fires with real data in any test.You should add a test that:
- Seeds a cached theme in
localStorage- Mocks
useQueryto return a non-nullresult.value.publicTheme- Verifies the store resolves to the server theme, not the cached one
web/src/store/theme.ts (4)
32-63: Theme storage key andsanitizeThemeprovide a solid normalization layerExposing
THEME_STORAGE_KEYand normalizing all theme inputs throughsanitizeThemeis a clean way to keepThemefully populated and well-typed, regardless of whether the source is persisted JSON or GraphQL. The field-by-field fallbacks toDEFAULT_THEMEtighten robustness against partial or malformed data.
129-165: Getters andsetThemesemantics align with previous behavior and new sourcesThe
darkModeandbannerGradientgetters remain straightforward and consistent with the existing UI semantics. The updatedsetThemenow cleanly distinguishes between'server'and'local'sources, usessanitizeThemeto merge partial updates, and respectshasServerTheme/devOverrideto prevent accidental overwrites of server-provided themes. This matches the intended flow without introducing surprising side effects.
275-305: Persist config anduseThemeStorewrapper correctly tie intoglobalPiniaThe
persistblock withpick: ['theme']andafterHydratehook ensures hydrated data flows throughsetTheme()normalization and CSS logic. The wrapper's fallback chainpinia ?? getActivePinia() ?? globalPiniaprovides flexibility while safely defaulting to the globally registered instance. All current callers use the no-argument form, avoiding the potential persistence gap when a custom Pinia withoutpiniaPluginPersistedstatewould be passed—though this remains a valid concern for future API misuse.
75-128: No precedence issue exists; server theme protection is already in place via early-return logicThe code correctly prevents the hydrate-override scenario through the gate at lines 171–173 of
setTheme. When a server theme is applied,hasServerThemeis set to true; subsequent calls fromafterHydrate(which use the defaultsource: 'local') return early without overwriting. This ensures server themes always take precedence, regardless of initialization order.While a combined cache+server test would provide explicit documentation of this behavior, the existing separate tests for cache and server scenarios, combined with the protective logic, make the implementation sound.
web/src/store/globalPinia.ts (1)
3-7: Persistedstate plugin registration onglobalPiniais correct and verifiedThe code properly wires
pinia-plugin-persistedstateon a single sharedglobalPiniainstance before callingsetActivePinia(globalPinia)at module load (line 11, confirmed by script output). This ensures all stores—including the theme store—persist without extra setup. All ~30useThemeStore()calls across the codebase route through this shared instance, and no production code creates custom Pinia instances.
- Updated the theme store to export the useThemeStore directly, simplifying its usage. - Removed the baseUseThemeStore function and associated global Pinia references, enhancing clarity and maintainability.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.26.0](v4.25.3...v4.26.0) (2025-11-17) ### Features * add cpu power query & subscription ([#1745](#1745)) ([d7aca81](d7aca81)) * add schema publishing to apollo studio ([#1772](#1772)) ([7e13202](7e13202)) * add workflow_dispatch trigger to schema publishing workflow ([818e7ce](818e7ce)) * apollo studio readme link ([c4cd0c6](c4cd0c6)) * **cli:** make `unraid-api plugins remove` scriptable ([#1774](#1774)) ([64eb9ce](64eb9ce)) * use persisted theme css to fix flashes on header ([#1784](#1784)) ([854b403](854b403)) ### Bug Fixes * **api:** decode html entities before parsing notifications ([#1768](#1768)) ([42406e7](42406e7)) * **connect:** disable api plugin if unraid plugin is absent ([#1773](#1773)) ([c264a18](c264a18)) * detection of flash backup activation state ([#1769](#1769)) ([d18eaf2](d18eaf2)) * re-add missing header gradient styles ([#1787](#1787)) ([f8a6785](f8a6785)) * respect OS safe mode in plugin loader ([#1775](#1775)) ([92af3b6](92af3b6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Testing
Codex Task
Summary by CodeRabbit