Enhances theme handling with seasonal themes#2
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughMigrates theming to a centralized runtime and provider: adds a build step bundling a browser theme initializer, switches layout to inject a pre-theme script, replaces next-themes with a custom ThemeProvider and hook, introduces seasonal themes (e.g., Halloween), updates UI components and assets, adjusts CSS variables, and removes cookie-sync. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant Document
participant PreThemeScript as Pre-theme script (buildThemeScript)
participant ThemeRuntime as ThemeRuntime (IIFE)
participant DOM as document.documentElement
note over Browser,ThemeRuntime: Initial page load (SSR -> first paint)
Browser->>Document: Request HTML
Document-->>Browser: HTML with <script id="pre-theme">...</script>
Browser->>PreThemeScript: Execute embedded bundle + init call
PreThemeScript->>ThemeRuntime: initTheme(config)
ThemeRuntime->>ThemeRuntime: Read cookies/localStorage + system pref
ThemeRuntime->>DOM: Apply classes (base/seasonal) and set data-* attrs
ThemeRuntime-->>PreThemeScript: Cleanup (re-enable transitions)
sequenceDiagram
autonumber
participant User
participant ThemeToggle as UI Theme Toggle
participant ThemeProvider as Custom ThemeProvider
participant Storage as localStorage/cookies
participant DOM as document.documentElement
User->>ThemeToggle: Select theme (e.g., "halloween" or "system")
ThemeToggle->>ThemeProvider: setTheme(nextPref)
ThemeProvider->>Storage: Persist preference (cookie/localStorage)
ThemeProvider->>DOM: Apply classes/datasets
ThemeProvider-->>ThemeToggle: State updated
rect rgba(200,230,255,0.2)
note right of ThemeProvider: If 'system', listens for OS changes<br/>and updates DOM accordingly
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/layout/about/aboutme/mode-toggle-link.tsx (1)
85-96: Accessibility: empty aria-label/title hide the button’s name.An empty aria-label overrides the visible “Dark mode” text, yielding an unnamed control. Remove both attributes or provide a meaningful label.
- aria-label="" - title=""> + /* rely on visible text for the accessible name */ + >src/components/ui/theme-toggle.tsx (1)
441-446: TypeScript: Document.startViewTransition isn’t in lib.dom.d.ts.Cast to access the experimental API to avoid TS errors.
- const startTransition = useCallback((updateFn: () => void) => { - if ('startViewTransition' in document) { - document.startViewTransition(updateFn) - } else { - updateFn() - } - }, []) + const startTransition = useCallback((updateFn: () => void) => { + const doc = document as unknown as { startViewTransition?: (cb: () => void) => void } + if (typeof doc.startViewTransition === 'function') { + doc.startViewTransition(updateFn) + } else { + updateFn() + } + }, [])
🧹 Nitpick comments (15)
src/components/layout/background/grid-lights.tsx (1)
184-186: OKLCH color-mix looks great; consider progressive fallback and moving styles to CSS for @supports.
- Some browsers disable color-mix or OKLCH behind flags. Add a CSS fallback (e.g., a class-based rule with @supports (color: oklch(100% 0 0)) and a non-OKLCH fallback) and shift these inline styles into CSS to avoid re-creating large strings on every render/resize.
Example (outside this range, in CSS):
/* default (fallback) */ .gl-dot { --glow-near: rgba(120, 160, 255, .7); --glow-far: rgba(120, 160, 255, .4); --gl-bg: radial-gradient(circle, rgba(120,160,255,.9) 0%, rgba(120,160,255,.5) 50%, transparent 100%); } @supports (color: oklch(100% 0 0)) and (color-mix(in oklch, black, white)) { .gl-dot { --glow-near: color-mix(in oklch, var(--grid-light) 70%, transparent); --glow-far: color-mix(in oklch, var(--grid-light) 40%, transparent); --gl-bg: radial-gradient(circle, color-mix(in oklch, var(--grid-light) 90%, transparent) 0%, color-mix(in oklch, var(--grid-light) 50%, transparent) 50%, transparent 100%); } }Then here:
- boxShadow: `0 0 ${near}px color-mix(in oklch, var(--grid-light) 70%, transparent), 0 0 ${far}px color-mix(in oklch, var(--grid-light) 40%, transparent)`, - background: - 'radial-gradient(circle, color-mix(in oklch, var(--grid-light) 90%, transparent) 0%, color-mix(in oklch, var(--grid-light) 50%, transparent) 50%, transparent 100%)', + boxShadow: `0 0 ${near}px var(--glow-near), 0 0 ${far}px var(--glow-far)`, + background: 'var(--gl-bg)',src/styles/globals.css (1)
181-222: Halloween theme block looks complete; add one guardrail: keep class on .
- Since color-scheme lives here (not html.halloween), ensure the runtime continues to apply .halloween on documentElement (it currently does). If that invariant may change, mirror the rule under html.halloween for safety.
Example (outside this range):
html.halloween { color-scheme: dark; }src/components/providers/theme-cookie-sync.tsx (1)
8-12: Set cookie with Secure when possible.On HTTPS, append ; secure to prevent downgrade leakage. Keep Lax for UX.
function setThemeCookie(value: string) { try { - document.cookie = `theme=${encodeURIComponent(value)}; path=/; max-age=31536000; samesite=lax` + const secure = (typeof window !== 'undefined' && window.isSecureContext) ? '; secure' : '' + document.cookie = `theme=${encodeURIComponent(value)}; path=/; max-age=31536000; samesite=lax${secure}` } catch {} }src/app/layout.tsx (1)
32-36: Make RootLayout sync; cookies() is synchronous.Removes unnecessary async/await; slight perf win and simpler typing.
-export default async function RootLayout({ children }: Readonly<{ children: React.ReactNode }>) { - const cookieStore = await cookies() +export default function RootLayout({ children }: Readonly<{ children: React.ReactNode }>) { + const cookieStore = cookies()src/components/providers/theme-provider.tsx (3)
41-43: Harden cookie attributes.Use standard casing and add Secure when served over HTTPS.
- document.cookie = `theme=${encodeURIComponent(value)}; path=/; max-age=31536000; samesite=lax` + document.cookie = `theme=${encodeURIComponent(value)}; Path=/; Max-Age=31536000; SameSite=Lax; Secure` @@ - document.cookie = `systemTheme=${encodeURIComponent(value)}; path=/; max-age=31536000; samesite=lax` + document.cookie = `systemTheme=${encodeURIComponent(value)}; Path=/; Max-Age=31536000; SameSite=Lax; Secure`Also applies to: 52-56
21-23: Make cookie regex tolerant to whitespace.Improves robustness across browsers.
- const re = /(?:^|; )theme=([^;]+)/ + const re = /(?:^|;\s*)theme=([^;]+)/
136-166: Avoid re-binding listeners on theme changes.Minor: register once and use a ref to read latest theme inside handlers.
I can provide a ref-based variant if you want it.
src/lib/theme-runtime.ts (3)
30-31: Avoid “always-on” ranges when start === end.With end <= start, equal endpoints yield a full-year active window. If equal should mean “disabled/empty,” guard explicitly and use strict < for cross-year.
- const crossesYear = compareMonthDay(end, start) <= 0 + // Equal endpoints => empty range (disabled) + if (compareMonthDay(end, start) === 0) return false + const crossesYear = compareMonthDay(end, start) < 0
25-39: Time zone drift around boundaries (SSR vs client).Server evaluates “now” in server TZ, client in user TZ. Near midnight on start/end dates this can diverge and flip seasonal defaults between server/client.
Consider:
- Compute on client only and hydrate a cookie with user TZ/offset for SSR, or
- Normalize comparisons to UTC by interpreting MonthDay as UTC midnights.
I can draft either approach if you confirm desired behavior.
45-49: Deduplicate available themes.If a seasonal name ever matches a base theme, this will duplicate. Cheap Set fixes it.
-export function getAvailableThemes(date: Date = new Date()): Theme[] { - const active = getActiveSeasonalThemes(date) - const seasonalNames = active.map(a => a.theme) - return ['system', ...BASE_CSS_THEMES, ...seasonalNames] -} +export function getAvailableThemes(date: Date = new Date()): Theme[] { + const active = getActiveSeasonalThemes(date) + const seasonalNames = active.map(a => a.theme as Theme) + return Array.from(new Set<Theme>(['system', ...(BASE_CSS_THEMES as unknown as Theme[]), ...seasonalNames])) +}src/components/layout/home/hero/profile-image.tsx (1)
84-93: Prefer dataset.appliedTheme over hard-coded class names.The init script sets data-applied-theme reliably; avoids drift and TypeScript coupling to specific ThemeName literals.
- const computeCssTheme = (): ThemeName => { - // Prefer explicit class among known css themes - const known: ThemeName[] = ['halloween', 'cyberpunk', 'dark', 'light'] - for (const k of known) { - if (root.classList.contains(k)) return k - } - // Fallback to resolved theme if it's a css theme - const r = theme.resolvedTheme - return r === 'dark' || r === 'light' ? r : 'light' - } + const computeCssTheme = (): ThemeName => { + const applied = root.dataset.appliedTheme as ThemeName | undefined + if (applied) return applied + const r = theme.resolvedTheme + return r === 'dark' || r === 'light' ? r : 'light' + }src/components/ui/theme-toggle.tsx (1)
101-107: Comment and behavior mismatch for “system” persistence.Code persists 'system' to localStorage; comment says “clear localStorage”. Update the comment to reflect intent.
- // Keep 'system' as the stored preference; clear localStorage when selecting system + // Persist 'system' as the stored preference so it overrides any theme cookie if (nextPref === 'system') { try { localStorage.setItem('theme', 'system') } catch {} }src/lib/themes.ts (3)
4-5: Prefer LucideIcon typing for accuracy and DXReturn icons typed as LucideIcon so consumers can pass size/color/stroke props without TS friction.
import { Ghost, Moon, Sun, Zap } from 'lucide-react' +import type { LucideIcon } from 'lucide-react' import type { StaticImageData } from 'next/image' import type { ComponentType } from 'react' -export type IconComponent = ComponentType<{ className?: string }> +export type IconComponent = LucideIconAlso applies to: 11-11
10-10: Document MonthDay semantics and seasonal range boundsClarify 1-based month/day and that end is exclusive to avoid off-by-one/overlap issues across modules.
+/** Month is 1–12; day is 1–31. */ export type MonthDay = { month: number; day: number } export type ThemeConfig = { name: string icon: IconComponent headshotImage: StaticImageData baseColor: BaseColor - timeRange?: { start: MonthDay; end: MonthDay } + /** Active in [start, end); end is exclusive. */ + timeRange?: { start: MonthDay; end: MonthDay }Also applies to: 18-18
62-64: Avoid repeated array scans by indexing themes onceMicro-opt: replace find(...) with a precomputed map.
- const entry = themes.find(t => t.name === theme) - return entry?.icon ?? Sun + const entry = THEMES_BY_NAME[theme] + return entry?.icon ?? Sun- const entry = themes.find(t => t.name === theme) - return entry?.headshotImage ?? Headshot + const entry = THEMES_BY_NAME[theme] + return entry?.headshotImage ?? HeadshotAdd after the themes array:
const THEMES_BY_NAME = Object.fromEntries( themes.map(t => [t.name, t] as const) ) as Record<ThemeName, ThemeEntry>;Also applies to: 66-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/app/layout.tsx(2 hunks)src/components/layout/about/aboutme/mode-toggle-link.tsx(1 hunks)src/components/layout/background/grid-lights.tsx(1 hunks)src/components/layout/home/hero/profile-image.tsx(3 hunks)src/components/providers/providers.tsx(1 hunks)src/components/providers/theme-cookie-sync.tsx(2 hunks)src/components/providers/theme-provider.tsx(1 hunks)src/components/ui/theme-toggle.tsx(7 hunks)src/lib/theme-runtime.ts(1 hunks)src/lib/themes.ts(1 hunks)src/styles/globals.css(5 hunks)
🧰 Additional context used
🪛 ast-grep (0.38.6)
src/app/layout.tsx
[warning] 58-58: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/app/layout.tsx
[error] 59-59: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (18)
src/styles/globals.css (4)
6-6: Variant hook LGTM.The @custom-variant halloween reads clean and aligns with the existing pattern.
75-75: Good: centralized token for grid lights.Introducing --grid-light at root enables themed lighting without component churn.
117-117: Dark token parity maintained.Dark’s --grid-light overrides are consistent with root; no conflicts spotted.
160-160: Cyberpunk parity maintained.Keeping a theme-specific --grid-light preserves visual identity per theme.
src/components/layout/about/aboutme/mode-toggle-link.tsx (2)
3-3: Swapping to the custom ThemeProvider is correct.Import path update aligns with the new theming architecture.
58-76: Seasonal overlay nuance: verify early-return logic with seasonal “darklike”.If a seasonal theme resolves to a dark base, resolvedTheme may already be 'dark', causing the toggle to no-op and leaving the user unable to force base 'dark'. Confirm your hook reports a base theme (light/dark) vs seasonal, or gate on the HTML’s 'dark' class instead.
Would you like me to adjust this to check document.documentElement.classList.contains('dark') or to use a baseResolvedTheme value if your hook exposes it?
src/components/providers/theme-cookie-sync.tsx (2)
3-6: Import surface matches the new runtime.Refs to useTheme, seasonal default, and themes list look correct.
36-39: Cookie stores the preference (not resolved) — good.Dependency on resolvedTheme ensures re-run on system switches; logic preserves 'system' as intended.
src/app/layout.tsx (1)
66-66: Providers wiring looks good.Prop name/type matches ThemeProvider API.
src/components/providers/providers.tsx (1)
8-14: LGTM on the new Providers API.Clean prop surface; correct typing; good composition order.
Also applies to: 17-21
src/components/providers/theme-provider.tsx (1)
74-91: Verify theme name inventory covers 'light' and 'dark'.applyClasses relies on themes.map(t.name) to include both system classes; confirm themes includes 'light' and 'dark' or handle them explicitly.
Also applies to: 95-98
src/lib/theme-runtime.ts (2)
19-23: MonthDay compare is correct and minimal.Good canonical ordering; keeps all downstream range math simple.
62-71: Inline init script and CSP.If a CSP with script-src nonce is enabled, this inline string must be injected via a <script nonce="...">. Ensure layout wiring provides the nonce.
I can adjust layout to use Next.js <Script nonce> or a helper that returns {__html} plus nonce.
src/components/layout/home/hero/profile-image.tsx (1)
76-78: Nice: initialize from SSR-applied theme to prevent image flash.This avoids a mismatched headshot on first paint.
src/components/ui/theme-toggle.tsx (1)
120-128: Good UX: skip animation when visual theme doesn’t change.Equating system+seasonal to explicit seasonal avoids gratuitous transitions.
src/lib/themes.ts (3)
21-47: Seasonal window sanity-checkConfirm Halloween should be active Oct 15–Oct 31 (end {11,1} exclusive). If Nov 1 should be included, set end to {11,2}.
49-52: Type exports look solidNice use of literal inference from the data source for ThemeName/Theme.
54-58: Label helper is fineSimple and sufficient for current single-word names.
…ses when an explicit theme is selected
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
public/scripts/apply-system-theme.js (1)
15-21: Harden cookie parsing and avoid adding a.lightclass.
- The regex misses cases where there’s no space after “;”. Use
;\s*.- Since light is the default
:roottheme (no.lightclass), don’t addlight; only adddark.Apply this diff:
- var match = document.cookie.match(/(?:^|; )systemTheme=([^;]+)/) + var match = document.cookie.match(/(?:^|;\s*)systemTheme=([^;]+)/) var captured = match && match[1] ? match[1] : null var systemTheme = captured ? decodeURIComponent(captured) : null - if (systemTheme === 'dark' || systemTheme === 'light') { - root.classList.add(systemTheme) - } + if (systemTheme === 'dark') { + root.classList.add('dark') + }src/app/layout.tsx (1)
49-53: Align fallback to light to reduce dark flash for light-system users.When seasonal default is active, you default
systodark. Uselightso users with unset/unknown systemTheme don’t see a dark flash on light-default sites.- const sys = baseSystem === 'dark' || baseSystem === 'light' ? baseSystem : 'dark' + const sys = baseSystem === 'dark' || baseSystem === 'light' ? baseSystem : 'light'src/components/providers/theme-cookie-sync.tsx (1)
41-61: Nice: explicit preference clears seasonal overlay state.This addresses the prior review about lingering seasonal overlays when users pick a non-system theme. Looks good.
🧹 Nitpick comments (10)
public/scripts/apply-system-theme.js (1)
1-23: This script duplicates logic in buildInitThemeScript(); consider removing.The init runtime already reads localStorage/cookies, validates, removes stale classes, and applies the theme. Keeping a second path risks drift. Prefer removing this file and relying solely on the init runtime.
src/lib/themes.ts (2)
13-19: Document timeRange end semantics (exclusive).
buildInitThemeScript()treatsendas exclusive (dt < ed). Add a brief JSDoc here to avoid off-by-one confusion for future seasonal themes.export type ThemeConfig = { name: string icon: IconComponent headshotImage: StaticImageData baseColor: BaseColor - timeRange?: { start: MonthDay; end: MonthDay } + /** + * Seasonal range; end is exclusive (active when date >= start and < end). + */ + timeRange?: { start: MonthDay; end: MonthDay } }
60-69: Minor: cache lookup to avoid repeated array scans (optional).If
getThemeIcon/getThemeHeadshotare hot, consider aMap<ThemeName, ThemeEntry>built once. Not critical at current scale.src/app/layout.tsx (2)
61-62: Consistently avoid emitting a.lightclass (optional).Since light is the default
:roottheme, consider treating the “light” case as empty (no theme class) for SSR parity with CSS. This reduces churn and class noise.- initialAppliedTheme = baseSystem === 'dark' ? 'dark' : 'light' + initialAppliedTheme = baseSystem === 'dark' ? 'dark' : 'light' // keep state + // Optionally: when using initialThemeClass below, emit '' instead of 'light'
32-36: No need for async/await with cookies(); simplify the component.
cookies()is synchronous; you can dropasyncand theawaitfor a tiny simplification.-export default async function RootLayout({ children }: Readonly<{ children: React.ReactNode }>) { - const cookieStore = await cookies() +export default function RootLayout({ children }: Readonly<{ children: React.ReactNode }>) { + const cookieStore = cookies()src/components/providers/theme-cookie-sync.tsx (3)
22-35: Avoid duplicating DOM class toggling; let ThemeProvider own class application.Both this component and ThemeProvider.applyClasses add/remove theme classes. This duplication risks flicker and inconsistent states. Make ThemeCookieSync solely persist the cookie (and optionally clear the overlay data-attr); delegate all classList changes to ThemeProvider.
Apply this diff to limit ThemeCookieSync to persistence-only:
@@ - const seasonal: ThemeName = seasonalDefault - try { - const root = document.documentElement - // Layer seasonal on top of system-effective (dark/light) without removing it - const allThemeClassNames = themes.map(t => t.name) - for (const cls of allThemeClassNames) { - if (cls === 'light' || cls === 'dark') continue - root.classList.remove(cls) - } - root.classList.add(seasonal) - root.dataset.seasonalOverlay = seasonal - } catch {} + const seasonal: ThemeName = seasonalDefault + // Defer DOM updates to ThemeProvider.applyClasses @@ - // If an explicit theme is chosen (non-system), proactively remove all non-base theme classes - if (pref !== 'system') { - const allThemeClassNames = themes.map(t => t.name) - for (const cls of allThemeClassNames) { - if (cls === 'light' || cls === 'dark') continue - root.classList.remove(cls) - } - // Apply explicit base theme or add explicit non-base theme class - if (pref === 'light' || pref === 'dark') { - const other = pref === 'light' ? 'dark' : 'light' - root.classList.remove(other) - root.classList.add(pref) - } else { - root.classList.add(pref) - } - } + // Defer DOM updates to ThemeProvider.applyClasses; this component only syncs persistence.Also applies to: 39-62
18-19: resolvedTheme is not used; trim the dependency array or use it intentionally.Either remove it from the deps or reference it (e.g., for cookie refresh on system flips) to avoid unnecessary effect runs.
Apply this diff if you choose to trim:
- }, [resolvedTheme, theme]) + }, [theme])Also applies to: 64-64
10-12: Unify cookie writing via a small helper (and standardize attributes).This cookie-writing logic is duplicated here and in ThemeProvider. Centralize it to reduce drift and use canonical casing (Max-Age, SameSite, Secure).
Apply these diffs:
+import { setCookie } from '@/lib/cookies' @@ -function setThemeCookie(value: string) { - try { - const isSecure = window.location.protocol === 'https:' ? '; secure' : '' - document.cookie = `theme=${encodeURIComponent(value)}; path=/; max-age=31536000; samesite=lax${isSecure}` - } catch {} -} +function setThemeCookie(value: string) { + setCookie('theme', value) +}Add this helper (new file):
// src/lib/cookies.ts export function setCookie(name: string, value: string, maxAge = 31536000) { try { const isSecure = window.location.protocol === 'https:' ? '; Secure' : '' document.cookie = `${name}=${encodeURIComponent(value)}; Path=/; Max-Age=${maxAge}; SameSite=Lax${isSecure}` } catch {} }src/components/providers/theme-provider.tsx (2)
43-48: DRY up cookie setting and standardize attributes.writeCookieTheme and writeCookieSystemTheme replicate cookie logic (also present in ThemeCookieSync). Centralize via a shared helper.
Apply these diffs:
+import { setCookie } from '@/lib/cookies' @@ -function writeCookieTheme(value: Theme) { - try { - const isSecure = window.location.protocol === 'https:' ? '; secure' : '' - document.cookie = `theme=${encodeURIComponent(value)}; path=/; max-age=31536000; samesite=lax${isSecure}` - } catch {} -} +function writeCookieTheme(value: Theme) { + setCookie('theme', value) +} @@ -function writeCookieSystemTheme(value: SystemTheme | undefined) { - if (!value) return - try { - const isSecure = window.location.protocol === 'https:' ? '; secure' : '' - document.cookie = `systemTheme=${encodeURIComponent(value)}; path=/; max-age=31536000; samesite=lax${isSecure}` - } catch {} -} +function writeCookieSystemTheme(value: SystemTheme | undefined) { + if (!value) return + setCookie('systemTheme', value) +}(Helper implementation shown in the other file’s comment.)
Also applies to: 56-62
83-86: Prefer :root for light theme and only toggle the ‘dark’ class
Remove applying the ‘light’ class (no.lightstyles exist; defaults live on:root). Only add/removedark.- // Ensure system dark/light present - if (!root.classList.contains(effective)) add(effective) - const other = effective === 'dark' ? 'light' : 'dark' - if (root.classList.contains(other)) remove(other) + // Ensure dark mode class only; light is default via :root + const other = effective === 'dark' ? 'light' : 'dark' + if (effective === 'dark') add('dark') + if (root.classList.contains(other)) remove(other)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
public/scripts/apply-system-theme.js(1 hunks)src/app/layout.tsx(2 hunks)src/components/providers/theme-cookie-sync.tsx(1 hunks)src/components/providers/theme-provider.tsx(1 hunks)src/lib/themes.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,cjs,mjs}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
**/*.{ts,tsx,js,jsx,cjs,mjs}: Never hardcode or hallucinate the PostHog API key; always read it from the value populated in the .env file
Create new feature flag names that are clear and descriptive
Gate flag-dependent code on a check that verifies the flag’s values are valid and expected
If a custom property for a person or event is referenced in two or more files or in two or more callsites within the same file, centralize the property name in an enum (TS) or const object (JS)
Files:
public/scripts/apply-system-theme.jssrc/components/providers/theme-provider.tsxsrc/app/layout.tsxsrc/components/providers/theme-cookie-sync.tsxsrc/lib/themes.ts
**/*.{js,jsx,cjs,mjs}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
In JavaScript, store feature flag names as string values in a const object (simulating an enum) with members written in UPPERCASE_WITH_UNDERSCORE and use a consistent naming convention
Files:
public/scripts/apply-system-theme.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
In TypeScript, store feature flag names in an enum with members written in UPPERCASE_WITH_UNDERSCORE and use a consistent naming convention
Files:
src/components/providers/theme-provider.tsxsrc/app/layout.tsxsrc/components/providers/theme-cookie-sync.tsxsrc/lib/themes.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: kiliantyler
PR: kiliantyler/kil.dev#2
File: src/lib/themes.ts:47-47
Timestamp: 2025-09-10T03:19:42.122Z
Learning: In CSS theming patterns, it's common and correct for the light theme to be implemented as the default `:root` styles without requiring a specific class, while dark and other themed variants override these defaults using their own classes like `.dark` and `.halloween`.
📚 Learning: 2025-09-10T03:19:42.122Z
Learnt from: kiliantyler
PR: kiliantyler/kil.dev#2
File: src/lib/themes.ts:47-47
Timestamp: 2025-09-10T03:19:42.122Z
Learning: In CSS theming patterns, it's common and correct for the light theme to be implemented as the default `:root` styles without requiring a specific class, while dark and other themed variants override these defaults using their own classes like `.dark` and `.halloween`.
Applied to files:
public/scripts/apply-system-theme.jssrc/app/layout.tsxsrc/lib/themes.ts
📚 Learning: 2025-09-10T03:19:42.122Z
Learnt from: kiliantyler
PR: kiliantyler/kil.dev#2
File: src/lib/themes.ts:47-47
Timestamp: 2025-09-10T03:19:42.122Z
Learning: In the kil.dev project, the light theme is implemented as the default theme using the base `:root` CSS custom properties in globals.css, without requiring a specific `.light` class. Other themes like `.dark` and `.halloween` override these default values with their own class-based styles.
Applied to files:
public/scripts/apply-system-theme.jssrc/lib/themes.ts
🧬 Code graph analysis (3)
src/components/providers/theme-provider.tsx (2)
src/lib/themes.ts (3)
Theme(51-51)ThemeName(50-50)themes(21-47)src/lib/theme-runtime.ts (3)
getDefaultThemeForNow(56-60)getAvailableThemes(45-49)SEASONAL_THEMES(13-15)
src/app/layout.tsx (3)
src/lib/theme-runtime.ts (2)
getDefaultThemeForNow(56-60)buildInitThemeScript(62-71)src/lib/themes.ts (2)
ThemeName(50-50)KNOWN_THEMES(72-72)src/components/providers/providers.tsx (1)
Providers(8-23)
src/components/providers/theme-cookie-sync.tsx (3)
src/components/providers/theme-provider.tsx (1)
useTheme(221-225)src/lib/theme-runtime.ts (1)
getDefaultThemeForNow(56-60)src/lib/themes.ts (2)
ThemeName(50-50)themes(21-47)
🔇 Additional comments (5)
src/lib/themes.ts (1)
21-47: Centralized, typed theme registry looks solid.Good use of
as const+ReadonlyArray<ThemeConfig>; icon/headshot mapping and seasonal window on Halloween are clear.src/app/layout.tsx (2)
37-43: Cookie/theme validation on the server is correct.Using
KNOWN_THEMESforthemeCookieand narrowingsystemThemeCookietodark|lightproperly prevents class injection. Nice.
69-73: Great move to Next <Script> and runtime-generated init code.Nonce/CSP-friendly and removes the previous dangerouslySetInnerHTML issue.
src/components/providers/theme-provider.tsx (2)
207-211: resolvedTheme computation is clean. LGTM.Concise and correct memoization.
9-15: Context valueinitialAppliedThemeNameis in use and cannot be dropped. It’s destructured and used in src/components/ui/theme-toggle.tsx (line 31; icon logic at 349–351) and in src/components/layout/home/hero/profile-image.tsx (line 77) for SSR theme initialization.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/app/layout.tsx (1)
33-33: Replace dangerouslySetInnerHTML with Next.js Script (nonce-aware).
This triggers security lint and weakens CSP. Use <Script beforeInteractive> with string children.- <script id="pre-theme" dangerouslySetInnerHTML={{ __html: buildThemeScript() }} /> + {/* Next hoists and attaches nonce when configured */} + <Script id="pre-theme" strategy="beforeInteractive"> + {buildThemeScript()} + </Script>Add import:
- import { type Metadata } from 'next' + import { type Metadata } from 'next' + import Script from 'next/script'If you enforce CSP nonces, pass the nonce prop from headers.
🧹 Nitpick comments (14)
package.json (1)
8-11: Avoid Bun coupling and duplicate runs.
- Using bun in lifecycle hooks breaks environments without Bun (e.g., npm/pnpm CI).
- build:theme runs twice during preview (prepreview and prebuild inside preview).
Suggested adjustments:
- Prefer a Node-compatible runner (e.g., tsx) or ship a .mjs wrapper.
- Remove prepreview to prevent double execution.
- "build:theme": "bun scripts/build-theme-runtime.ts", - "prebuild": "bun run build:theme", - "predev": "bun run build:theme", - "prepreview": "bun run build:theme", + "build:theme": "tsx scripts/build-theme-runtime.ts", + "prebuild": "npm run build:theme", + "predev": "npm run build:theme"(If you prefer Bun, document it and drop prepreview to avoid duplicate runs.)
eslint.config.js (1)
36-45: Scope the scripts override narrowly.
Limit disabled “no-unsafe-*” rules to the specific build script to avoid masking issues in other scripts.- { - files: ['scripts/**/*.ts'], + { + files: ['scripts/build-theme-runtime.ts'], rules: { '@typescript-eslint/no-unsafe-assignment': 'off', '@typescript-eslint/no-unsafe-member-access': 'off', '@typescript-eslint/no-unsafe-call': 'off', '@typescript-eslint/no-unsafe-argument': 'off', 'no-console': 'off', }, },src/lib/theme-script.ts (3)
68-76: Harden cookie regex against optional whitespace.
More robust cookie parsing.- const re = /(?:^|; )theme=([^;]+)/ + const re = /(?:^|;\s*)theme=([^;]+)/
86-93: Use Set for uniqueness.
Simpler and preserves insertion order.-function uniqueStrings(values: string[]): string[] { - const seen: Record<string, 1> = {} - for (const v of values) { - seen[v] = 1 - } - return Object.keys(seen) -} +function uniqueStrings(values: string[]): string[] { + return [...new Set(values)] +}
136-163: Avoid adding a.lightclass (light is default root).
Per your theming pattern, keep light unclassed to reduce specificity issues and flashes.- const baseClass = sysDark ? 'dark' : 'light' + const baseClass = sysDark ? 'dark' : '' // '' denotes light baseline @@ - if (explicit) { - targetClasses.push(explicit) - } else { - targetClasses.push(baseClass) - if (overlay) targetClasses.push(overlay) - } + if (explicit) { + targetClasses.push(explicit) + } else { + if (baseClass) targetClasses.push(baseClass) // only add 'dark' + if (overlay) targetClasses.push(overlay) + } @@ - const known = uniqueStrings([...config.base, ...config.seasonal.map(s => s.theme), 'light', 'dark']) + const known = uniqueStrings([...config.base, ...config.seasonal.map(s => s.theme), 'dark'])If other code reads dataset.appliedTheme ‘light’, keep datasets as-is even if no class is added.
scripts/build-theme-runtime.ts (3)
30-35: Select the JS artifact explicitly from esbuild’s outputFiles.When sourcemaps are enabled, outputFiles can contain multiple entries; relying on index 0 is brittle.
- const outputFile = result.outputFiles?.[0] + const outputFile = result.outputFiles?.find(f => f.path.endsWith('.js'))
1-1: Clarify runtime requirement or add Node fallback.Shebang pins Bun. If CI/devs may use Node, either document this or add a small Node wrapper/script to invoke via tsx/node.
36-39: Mark generated file for tooling.Add a generated annotation and optionally disable lint rules to avoid noise on the emitted file.
- const header = `// This file is generated by scripts/build-theme-runtime.ts\n// Do not edit manually.\n` + const header = + `// @generated by scripts/build-theme-runtime.ts\n` + + `// Do not edit manually.\n` + + `/* eslint-disable */\n`src/components/ui/theme-toggle.tsx (2)
126-148: Capture both preference and applied (visual) theme for analytics; centralize event keys.You compute currentVisual/nextVisual—log that alongside preference and avoid string literals spread around.
- if (currentVisual === nextVisual) { - setTheme(nextPref) - captureThemeChanged(nextPref) + if (currentVisual === nextVisual) { + setTheme(nextPref) + captureThemeChanged({ preference: nextPref, applied: nextVisual }) setOpen(false) return }- startTransition(() => { - setTheme(nextPref) - captureThemeChanged(nextPref) - }) + startTransition(() => { + setTheme(nextPref) + captureThemeChanged({ preference: nextPref, applied: nextVisual }) + })Add/update this helper (outside this file) to centralize event/prop names:
// src/hooks/posthog.ts export enum AnalyticsEvent { THEME_CHANGED = 'theme_changed' } export enum AnalyticsProps { PREFERENCE = 'preference', APPLIED = 'applied_theme' } export function captureThemeChanged(payload: { preference: string; applied: string }) { posthog.capture(AnalyticsEvent.THEME_CHANGED, { [AnalyticsProps.PREFERENCE]: payload.preference, [AnalyticsProps.APPLIED]: payload.applied, }) }Also applies to: 159-163
49-71: Memo deps: document intent.themes is a module-constant; [] is fine. Consider adding a comment to prevent “missing dep” churn during refactors.
src/components/layout/home/hero/profile-image.tsx (1)
94-108: Memo deps: clarify const-ness of themes.If themes ever becomes dynamic (HMR or runtime), include it in deps; otherwise, add a short comment noting it’s a compile-time constant.
src/components/providers/theme-provider.tsx (1)
122-139: Don’t add/remove 'light' class; it’s the :root default.Keep only the 'dark' base toggled. This matches the project’s theming model and avoids unnecessary class churn.
- if (pref === 'system') { - const effective = system ?? 'light' - // Ensure system dark/light present - if (!root.classList.contains(effective)) add(effective) - const other = effective === 'dark' ? 'light' : 'dark' - if (root.classList.contains(other)) remove(other) + if (pref === 'system') { + const effective = system ?? 'light' + // Only toggle the dark class; light is provided by :root + if (effective === 'dark') { + if (!root.classList.contains('dark')) add('dark') + } else { + if (root.classList.contains('dark')) remove('dark') + } // Seasonal overlay when active; otherwise ensure non-system theme classes are removedsrc/lib/theme-runtime.ts (1)
10-12: Use hasOwnProperty for a tighter type guard.Avoid prototype chain surprises; prefer
hasOwnProperty/Object.hasOwn.-function hasTimeRange(entry: ThemeEntry): entry is ThemeEntry & { timeRange: { start: MonthDay; end: MonthDay } } { - return 'timeRange' in entry -} +function hasTimeRange(entry: ThemeEntry): entry is ThemeEntry & { timeRange: { start: MonthDay; end: MonthDay } } { + return Object.prototype.hasOwnProperty.call(entry, 'timeRange') +}src/lib/themes.ts (1)
80-82: Rename for clarity:isSystemVal→isBaseColor.The current name suggests it checks
'system', but it validates base colors ('light'|'dark').-export function isSystemVal(val: unknown): val is BaseColor { - return val === 'dark' || val === 'light' -} +// Deprecated: prefer isBaseColor +export function isSystemVal(val: unknown): val is BaseColor { + return val === 'dark' || val === 'light' +} +export function isBaseColor(val: unknown): val is BaseColor { + return val === 'dark' || val === 'light' +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.gitignore(2 hunks)eslint.config.js(2 hunks)package.json(3 hunks)scripts/build-theme-runtime.ts(1 hunks)src/app/layout.tsx(2 hunks)src/components/layout/about/aboutme/mode-toggle-link.tsx(2 hunks)src/components/layout/home/hero/profile-image.tsx(4 hunks)src/components/providers/providers.tsx(1 hunks)src/components/providers/theme-cookie-sync.tsx(0 hunks)src/components/providers/theme-provider.tsx(1 hunks)src/components/ui/theme-toggle.tsx(7 hunks)src/lib/theme-runtime.ts(1 hunks)src/lib/theme-script.ts(1 hunks)src/lib/themes.ts(1 hunks)src/types/theme-bundle.d.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/providers/theme-cookie-sync.tsx
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/layout/about/aboutme/mode-toggle-link.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,cjs,mjs}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
**/*.{ts,tsx,js,jsx,cjs,mjs}: Never hardcode or hallucinate the PostHog API key; always read it from the value populated in the .env file
Create new feature flag names that are clear and descriptive
Gate flag-dependent code on a check that verifies the flag’s values are valid and expected
If a custom property for a person or event is referenced in two or more files or in two or more callsites within the same file, centralize the property name in an enum (TS) or const object (JS)
Files:
src/types/theme-bundle.d.tsscripts/build-theme-runtime.tseslint.config.jssrc/lib/theme-script.tssrc/lib/theme-runtime.tssrc/components/ui/theme-toggle.tsxsrc/components/layout/home/hero/profile-image.tsxsrc/lib/themes.tssrc/components/providers/providers.tsxsrc/components/providers/theme-provider.tsxsrc/app/layout.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
In TypeScript, store feature flag names in an enum with members written in UPPERCASE_WITH_UNDERSCORE and use a consistent naming convention
Files:
src/types/theme-bundle.d.tsscripts/build-theme-runtime.tssrc/lib/theme-script.tssrc/lib/theme-runtime.tssrc/components/ui/theme-toggle.tsxsrc/components/layout/home/hero/profile-image.tsxsrc/lib/themes.tssrc/components/providers/providers.tsxsrc/components/providers/theme-provider.tsxsrc/app/layout.tsx
**/*.{js,jsx,cjs,mjs}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
In JavaScript, store feature flag names as string values in a const object (simulating an enum) with members written in UPPERCASE_WITH_UNDERSCORE and use a consistent naming convention
Files:
eslint.config.js
🧠 Learnings (3)
📚 Learning: 2025-09-10T03:19:42.122Z
Learnt from: kiliantyler
PR: kiliantyler/kil.dev#2
File: src/lib/themes.ts:47-47
Timestamp: 2025-09-10T03:19:42.122Z
Learning: In CSS theming patterns, it's common and correct for the light theme to be implemented as the default `:root` styles without requiring a specific class, while dark and other themed variants override these defaults using their own classes like `.dark` and `.halloween`.
Applied to files:
src/lib/theme-runtime.tssrc/lib/themes.tssrc/components/providers/theme-provider.tsxsrc/app/layout.tsx
📚 Learning: 2025-09-10T03:19:42.122Z
Learnt from: kiliantyler
PR: kiliantyler/kil.dev#2
File: src/lib/themes.ts:47-47
Timestamp: 2025-09-10T03:19:42.122Z
Learning: In the kil.dev project, the light theme is implemented as the default theme using the base `:root` CSS custom properties in globals.css, without requiring a specific `.light` class. Other themes like `.dark` and `.halloween` override these default values with their own class-based styles.
Applied to files:
src/lib/themes.tssrc/components/providers/theme-provider.tsx
📚 Learning: 2025-09-10T03:43:31.166Z
Learnt from: kiliantyler
PR: kiliantyler/kil.dev#2
File: src/components/providers/theme-provider.tsx:100-103
Timestamp: 2025-09-10T03:43:31.166Z
Learning: In the kil.dev project's theme system, non-base themes like 'cyberpunk' and 'halloween' are self-contained with complete CSS variable definitions. They don't inherit from base theme classes like '.dark'. The --theme-darklike variable is a flag for card readability styling, not an indicator of CSS inheritance. Theme application should remove all existing theme classes and apply only the selected theme class.
Applied to files:
src/lib/themes.tssrc/components/providers/theme-provider.tsx
🧬 Code graph analysis (6)
src/lib/theme-runtime.ts (2)
src/lib/themes.ts (5)
ThemeName(50-50)MonthDay(10-10)ThemeEntry(49-49)themes(21-47)Theme(51-51)src/types/theme-bundle.d.ts (1)
THEME_RUNTIME_BUNDLE(2-2)
src/components/ui/theme-toggle.tsx (4)
src/lib/themes.ts (2)
Theme(51-51)themes(21-47)src/lib/theme-runtime.ts (2)
getDefaultThemeForNow(67-71)getAvailableThemes(56-60)src/hooks/posthog.ts (1)
captureThemeChanged(11-15)src/lib/utils.ts (1)
cn(4-6)
src/components/layout/home/hero/profile-image.tsx (2)
src/hooks/use-hash.ts (1)
useHash(3-56)src/lib/themes.ts (2)
getThemeHeadshot(67-70)themes(21-47)
src/components/providers/providers.tsx (2)
src/lib/themes.ts (1)
ThemeName(50-50)src/components/providers/theme-provider.tsx (1)
ThemeProvider(146-304)
src/components/providers/theme-provider.tsx (2)
src/lib/themes.ts (3)
Theme(51-51)ThemeName(50-50)themes(21-47)src/lib/theme-runtime.ts (3)
getAvailableThemes(56-60)getDefaultThemeForNow(67-71)SEASONAL_THEMES(14-16)
src/app/layout.tsx (1)
src/lib/theme-runtime.ts (1)
buildThemeScript(73-90)
🪛 ast-grep (0.38.6)
src/components/ui/theme-toggle.tsx
[warning] 349-349: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
[warning] 350-350: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
src/components/layout/home/hero/profile-image.tsx
[warning] 124-124: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/components/ui/theme-toggle.tsx
[error] 350-350: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 351-351: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/components/layout/home/hero/profile-image.tsx
[error] 125-125: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/app/layout.tsx
[error] 33-33: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (15)
src/types/theme-bundle.d.ts (1)
1-3: LGTM: ambient module matches output path alias.
No issues; aligns with '@/lib/theme-bundle'.eslint.config.js (1)
11-11: LGTM: ignore generated bundle.
Ignoring src/lib/theme-bundle.ts prevents noisy lint.src/lib/theme-script.ts (1)
18-52: LGTM: config validation and name constraints.
Good defensive checks and clear error messages.src/app/layout.tsx (2)
7-7: LGTM: using buildThemeScript() to centralize runtime.
Keeps layout lean and consistent.
33-33: Audit all dangerouslySetInnerHTML usages for XSS — verify outputs are static/trusted or properly sanitized.
- src/app/layout.tsx:33 — <script id="pre-theme" dangerouslySetInnerHTML={{ __html: buildThemeScript() }} /> (verify buildThemeScript() cannot contain user-controlled data).
- src/components/ui/theme-toggle.tsx:350-351 — <style dangerouslySetInnerHTML={{ __html: themeIconCss }} /> and <style dangerouslySetInnerHTML={{ __html: spinCss }} /> (ensure CSS strings are not derived from user input).
- src/components/layout/home/hero/profile-image.tsx:125 — <style dangerouslySetInnerHTML={{ __html: profileImgCss }} /> (verify safety or switch to class-based styling).
Action: if any content is user-controllable, sanitize or replace with safer patterns (CSS classes, CSS-in-JS, server-side sanitization, or CSP + escaping).
package.json (1)
49-49: Confirm Bun consumer and pin esbuild for reproducible builds.
scripts/build-theme-runtime.ts uses Bun (#!/usr/bin/env bun) and directly imports { build } from 'esbuild'; package.json currently lists "esbuild": "^0.24.0" — pin to an exact version or commit a lockfile (bun.lockb / package-lock.json) and verify the esbuild release is compatible with the Bun runtime.src/components/providers/providers.tsx (1)
7-13: Plumbing looks right; Provider API is clean.Passing initialAppliedTheme down keeps SSR-applied theme stable on mount. LGTM.
Also applies to: 16-16
src/components/providers/theme-provider.tsx (2)
26-48: VALID_THEMES vs seasonal availability.VALID_THEMES is static; coerceToValidTheme uses getAvailableThemes() (seasonal). This is good. Keep it this way to allow seasonals in cookies while normalizing expired picks to 'system'.
254-259: Idempotent class application path is solid.setTheme persists to both storage and cookies and reapplies classes; matches cross-tab sync logic below.
scripts/build-theme-runtime.ts (1)
12-16: Verified — entry is correct and browser-safe for IIFE bundling.
src/lib/theme-script.ts exists and is the browser runtime (uses document/localStorage/window.matchMedia) and contains no Node/core imports or require() calls.src/lib/theme-runtime.ts (4)
26-50: Recurring date window logic looks solid (incl. leap-year fallback).
67-71: Default selection is sensible.Prefers first active seasonal theme, else 'system'; ordering provides deterministic tie-breaks.
73-90: Harden inline script against </script> breakouts; add optional sourceURL.Repo search didn't locate the runtime bundle file or an initTheme symbol; verify THEME_RUNTIME_BUNDLE contains the minified runtime and that window.ThemeRuntime.initTheme is exposed before applying this change.
File: src/lib/theme-runtime.ts (lines 73-90)
- const serializedCfg = JSON.stringify(cfg) - const invoke = ';try{window.ThemeRuntime&&window.ThemeRuntime.initTheme(' + serializedCfg + ')}catch(e){}' - return THEME_RUNTIME_BUNDLE + invoke + const serializedCfg = JSON.stringify(cfg) + // Prevent closing-script token from breaking out of the tag. + const safeBundle = THEME_RUNTIME_BUNDLE.replace(/<\/script/gi, '<\\/script') + const invoke = ';try{window.ThemeRuntime&&window.ThemeRuntime.initTheme(' + serializedCfg + ')}catch(e){}' + return safeBundle + invoke + '\n//# sourceURL=theme-runtime-inline.js'
18-18: Exclude 'light' from BASE_CSS_THEMES — light is implemented on :rootCSS contains no
.lightclass; remove'light'to avoid no-op class toggles and SSR/CSR drift. src/lib/theme-bundle.ts was not present in the repo snapshot—confirm no runtime references to the'light'token before merging.-const BASE_CSS_THEMES: ThemeName[] = themes.filter(t => !('timeRange' in t)).map(t => t.name) +const BASE_CSS_THEMES: ThemeName[] = themes + .filter(t => t.name !== 'light' && !('timeRange' in t)) + .map(t => t.name)src/lib/themes.ts (1)
21-47: Centralized, typed theme registry is clean and future-proof.
as const satisfies ReadonlyArray<ThemeConfig>gives nice literal inference while keeping shape constraints. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/components/layout/home/hero/profile-image.tsx (1)
125-125: Security issue addressed correctly.The
dangerouslySetInnerHTMLusage has been replaced with safe JSX children syntax as recommended in previous reviews. This addresses the security linting warnings while maintaining functionality.src/components/ui/theme-toggle.tsx (1)
350-351: Security issue addressed correctly.The
dangerouslySetInnerHTMLusage has been replaced with safe JSX children syntax<style>{css}</style>. This removes XSS vulnerabilities while maintaining functionality.
🧹 Nitpick comments (4)
src/lib/theme-runtime.ts (1)
10-12: Consider using 'in' operator for better type safety.The type guard uses
Object.prototype.hasOwnProperty.call(), which is safe but verbose. Consider using theinoperator for better readability.-function hasTimeRange(entry: ThemeEntry): entry is ThemeEntry & { timeRange: { start: MonthDay; end: MonthDay } } { - return Object.prototype.hasOwnProperty.call(entry, 'timeRange') -} +function hasTimeRange(entry: ThemeEntry): entry is ThemeEntry & { timeRange: { start: MonthDay; end: MonthDay } } { + return 'timeRange' in entry +}src/components/ui/theme-toggle.tsx (1)
50-71: CSS selector complexity may impact maintainability.The theme icon CSS generation uses complex nested selectors with multiple negations. While functionally correct, this complexity could make debugging difficult. Consider if there's a simpler approach or add comprehensive comments explaining the logic.
The CSS selector logic could benefit from inline comments explaining each rule:
const themeIconCss = useMemo(() => { const names = themes.map(t => t.name) const nonBase = names.filter(n => n !== 'light' && n !== 'dark') const rules: string[] = [] - // Hide all by default + // Step 1: Hide all theme icons by default rules.push('.theme-icon{display:none}') - // Non-base themes win when their class is on <html> + // Step 2: Show non-base theme icons when their specific class is active for (const n of nonBase) { rules.push(`html.${n} .theme-icon[data-theme="${n}"]{display:inline-block}`) } - // Dark shows when .dark present and no non-base theme class active + // Step 3: Show dark icon when dark class is present and no non-base themes are active if (names.includes('dark')) { const notNonBase = nonBase.map(n => `:not(.${n})`).join('') rules.push(`html.dark${notNonBase} .theme-icon[data-theme="dark"]{display:inline-block}`) } - // Light shows when not dark and no non-base theme class active + // Step 4: Show light icon as fallback when no other theme classes are active if (names.includes('light')) { const notOthers = ['dark', ...nonBase].map(n => `:not(.${n})`).join('') rules.push(`html${notOthers} .theme-icon[data-theme="light"]{display:inline-block}`) } return rules.join('') }, [])package.json (2)
7-10: Consider inlining the theme step intobuildto reduce lifecycle coupling.Inlining removes reliance on
prebuildsemantics in custom runners.- "build": "next build", + "build": "bun run build:theme && next build", - "prebuild": "bun run build:theme", + "prebuild": "": ""Note: remove
prebuildentirely or omit it (empty entry shown here just to illustrate removal).
48-48: Confirm esbuild availability in build environments.
esbuildis a devDependency. Ensure the build pipeline installs devDeps (e.g., Vercel/CI default) sincescripts/build-theme-runtime.tslikely imports it. If not, moveesbuildtodependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
eslint.config.js(2 hunks)package.json(3 hunks)scripts/build-theme-runtime.ts(1 hunks)src/components/layout/home/hero/profile-image.tsx(4 hunks)src/components/ui/theme-toggle.tsx(7 hunks)src/lib/theme-runtime.ts(1 hunks)src/lib/theme-script.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/build-theme-runtime.ts
- eslint.config.js
- src/lib/theme-script.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,cjs,mjs}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
**/*.{ts,tsx,js,jsx,cjs,mjs}: Never hardcode or hallucinate the PostHog API key; always read it from the value populated in the .env file
Create new feature flag names that are clear and descriptive
Gate flag-dependent code on a check that verifies the flag’s values are valid and expected
If a custom property for a person or event is referenced in two or more files or in two or more callsites within the same file, centralize the property name in an enum (TS) or const object (JS)
Files:
src/lib/theme-runtime.tssrc/components/layout/home/hero/profile-image.tsxsrc/components/ui/theme-toggle.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
In TypeScript, store feature flag names in an enum with members written in UPPERCASE_WITH_UNDERSCORE and use a consistent naming convention
Files:
src/lib/theme-runtime.tssrc/components/layout/home/hero/profile-image.tsxsrc/components/ui/theme-toggle.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-10T03:19:42.122Z
Learnt from: kiliantyler
PR: kiliantyler/kil.dev#2
File: src/lib/themes.ts:47-47
Timestamp: 2025-09-10T03:19:42.122Z
Learning: In CSS theming patterns, it's common and correct for the light theme to be implemented as the default `:root` styles without requiring a specific class, while dark and other themed variants override these defaults using their own classes like `.dark` and `.halloween`.
Applied to files:
src/lib/theme-runtime.ts
🧬 Code graph analysis (3)
src/lib/theme-runtime.ts (2)
src/lib/themes.ts (5)
ThemeName(50-50)MonthDay(10-10)ThemeEntry(49-49)themes(21-47)Theme(51-51)src/types/theme-bundle.d.ts (1)
THEME_RUNTIME_BUNDLE(2-2)
src/components/layout/home/hero/profile-image.tsx (2)
src/hooks/use-hash.ts (1)
useHash(3-56)src/lib/themes.ts (2)
getThemeHeadshot(67-70)themes(21-47)
src/components/ui/theme-toggle.tsx (2)
src/lib/themes.ts (2)
Theme(51-51)themes(21-47)src/lib/theme-runtime.ts (2)
getDefaultThemeForNow(67-71)getAvailableThemes(56-60)
🔇 Additional comments (10)
src/components/layout/home/hero/profile-image.tsx (3)
9-9: Well-structured theme integration.The component now properly integrates with the centralized theme system, using
getThemeHeadshotandthemesfrom the centralized theme definitions. The theme-specific image logic is clean and maintainable.Also applies to: 75-78
126-138: Theme headshots verified — mapping complete.src/lib/themes.ts provides headshotImage for 'light' (cartoon-headshot.webp), 'dark' (cartoon-headshot.webp), 'cyberpunk' (cartoon-cyberpunk.webp) and 'halloween' (cartoon-halloween.webp); getThemeHeadshot returns the mapped image and falls back to Headshot — no missing images.
94-107: Verified — CSS selectors correctly handle theme combinations; no change required.src/lib/themes.ts defines 'light' and 'dark'; profile-image.tsx mirrors the selector logic in src/components/ui/theme-toggle.tsx and correctly handles nonBase, empty-nonBase, and conflicting-class cases.
src/lib/theme-runtime.ts (3)
1-3: Clean modular design with proper imports.The module structure is well-organized with clear separation between seasonal and base themes. The imports are properly typed and the module depends on centralized theme definitions.
Also applies to: 14-18
26-50: Robust date range handling with leap year protection.The
isDateInRecurringRangefunction properly handles year-crossing ranges and includes protection for leap year edge cases. The logic for handling Feb 29th by falling back to Feb 28th is correct.
73-90: Well-architected script generation with security considerations.The
buildThemeScriptfunction properly separates the runtime bundle from configuration, uses safe JSON serialization, and includes error handling. The approach of embedding the minified runtime and appending configuration is secure and efficient.src/components/ui/theme-toggle.tsx (2)
3-3: Proper integration with centralized theme system.The component now correctly imports from the centralized theme provider and theme runtime, replacing the previous next-themes dependency. This aligns with the PR's goal of migrating to a custom theme system.
Also applies to: 11-12
126-174: Verify seasonal-default resolution and overlap/tie‑breaking.getDefaultThemeForNow returns the first active seasonal theme (active[0]?.theme) or 'system' — seasonal configs use timeRange in src/lib/themes.ts. I could not locate/inspect getActiveSeasonalThemes or confirm where systemTheme/resolvedTheme are sourced in this change; confirm getActiveSeasonalThemes enforces a deterministic priority for overlapping ranges and that handleThemeChange's currentVisual === nextVisual comparison remains correct when seasonal, system, or resolved-theme boundaries change. Files: src/lib/theme-runtime.ts (getDefaultThemeForNow), src/lib/themes.ts (seasonal timeRanges), src/components/ui/theme-toggle.tsx (handleThemeChange).
package.json (2)
71-71: Trailing newline addition looks good.Keeps tooling and POSIX-friendly diffs happy.
8-10: Make theme build reliable across all entry points; declare Bun as required.Verified: root package.json defines build:theme (bun scripts/build-theme-runtime.ts) and prebuild/predev call it; preview is "next build && next start" (bypasses prebuild). bun.lock is present; packageManager/engines are not declared.
- Option A (preferred): route preview through the package scripts so lifecycle hooks run.
- "preview": "next build && next start", + "preview": "bun run build && next start",
- Option B: keep preview but ensure the theme step runs first.
+ "prepreview": "bun run build:theme",
- Also add packageManager and engines to root package.json to declare Bun (set the version to the Bun used by CI/local dev).
1 participant
Improves theme management by introducing seasonal themes and refining theme persistence and transitions.
Addresses:
Summary by CodeRabbit
New Features
Improvements
Performance
Chores