Uh oh!
There was an error while loading. Please reload this page.
Sync diff renderer and worker theme with resolved app theme - #108
Merged
Conversation
- Pass explicit Pierre light/dark theme into diff render options - Re-key file diffs on resolved theme so views rerender on theme switch - Stabilize `useTheme` snapshots to propagate system theme updates reliably
WalkthroughThese changes implement theme synchronization across the diff rendering system. DiffPanel removes the "system" theme option and uses a concrete theme mapping to "pierre-light" or "pierre-dark". DiffWorkerPoolProvider now synchronizes theme changes to the worker pool, and useTheme introduces snapshot-based caching for optimized theme resolution with system preference tracking. Changes
Sequence DiagramsequenceDiagram
participant UI as React UI
participant useTheme as useTheme Hook
participant ExtStore as External Store
participant DiffProvider as DiffWorkerPoolProvider
participant Worker as Worker Pool
UI->>useTheme: Read current theme
useTheme->>ExtStore: getSnapshot() → ThemeSnapshot
ExtStore-->>useTheme: {theme, systemDark}
useTheme-->>UI: resolvedTheme (light/dark)
UI->>DiffProvider: Theme changed signal
DiffProvider->>useTheme: useTheme() → current theme
useTheme-->>DiffProvider: resolvedTheme
DiffProvider->>DiffProvider: Compute diffThemeName<br/>(light→pierre-light, etc.)
DiffProvider->>Worker: setRenderOptions({<br/>highlighterOptions: {theme}})
Worker-->>DiffProvider: Updated
DiffProvider->>UI: DiffPanel with themed<br/>FileDiff components
UI-->>UI: Render with active theme
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/components/DiffWorkerPoolProvider.tsx (1)
10-32: Consider logging errors instead of silently swallowing them.The
.catch(() => undefined)on line 28 silently discards any errors fromsetRenderOptions. While you may not want to crash the UI, logging the error would help with debugging theme sync issues.♻️ Proposed improvement
void workerPool .setRenderOptions({ ...current, theme: themeName, }) - .catch(() => undefined);+ .catch((err) => {+ console.error("[DiffWorkerThemeSync] Failed to sync theme:", err);+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/DiffWorkerPoolProvider.tsx` around lines 10 - 32, In DiffWorkerThemeSync, stop silently swallowing errors from workerPool.setRenderOptions; replace the .catch(() => undefined) with a handler that logs the error (e.g., console.error or your app logger) and includes context (mention themeName and that it occurred in DiffWorkerThemeSync when calling setRenderOptions on the workerPool returned by useWorkerPool); ensure this change is applied where setRenderOptions is called and keep the non-blocking behavior (do not rethrow).apps/web/src/components/DiffPanel.tsx (1)
19-23: ExtractresolveDiffThemeNameto a shared utility to eliminate duplication.This function is duplicated in
DiffWorkerPoolProvider.tsx(lines 6-8). Consider extracting it to a shared location, e.g.,~/lib/diffTheme.ts.♻️ Proposed extraction
Create a new file
apps/web/src/lib/diffTheme.ts:exporttypeDiffThemeType="light"|"dark";exportfunctionresolveDiffThemeName(theme: DiffThemeType){returntheme==="dark" ? "pierre-dark" : "pierre-light";}Then import it in both files:
-type DiffThemeType = "light" | "dark";--function resolveDiffThemeName(theme: "light" | "dark") {- return theme === "dark" ? "pierre-dark" : "pierre-light";-}+import { resolveDiffThemeName, type DiffThemeType } from "~/lib/diffTheme";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/DiffPanel.tsx` around lines 19 - 23, Extract the duplicated resolveDiffThemeName and DiffThemeType into a shared utility: create a new module that exports `type DiffThemeType = "light" | "dark"` and `function resolveDiffThemeName(theme: DiffThemeType)` (returning "pierre-dark" for "dark" else "pierre-light"), then replace the local definitions in both `DiffPanel` and `DiffWorkerPoolProvider` with imports of `DiffThemeType` and `resolveDiffThemeName` from the new utility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 19-23: Extract the duplicated resolveDiffThemeName and
DiffThemeType into a shared utility: create a new module that exports `type
DiffThemeType = "light" | "dark"` and `function resolveDiffThemeName(theme:
DiffThemeType)` (returning "pierre-dark" for "dark" else "pierre-light"), then
replace the local definitions in both `DiffPanel` and `DiffWorkerPoolProvider`
with imports of `DiffThemeType` and `resolveDiffThemeName` from the new utility.
In `@apps/web/src/components/DiffWorkerPoolProvider.tsx`:
- Around line 10-32: In DiffWorkerThemeSync, stop silently swallowing errors
from workerPool.setRenderOptions; replace the .catch(() => undefined) with a
handler that logs the error (e.g., console.error or your app logger) and
includes context (mention themeName and that it occurred in DiffWorkerThemeSync
when calling setRenderOptions on the workerPool returned by useWorkerPool);
ensure this change is applied where setRenderOptions is called and keep the
non-blocking behavior (do not rethrow).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/components/DiffPanel.tsxapps/web/src/components/DiffWorkerPoolProvider.tsxapps/web/src/hooks/useTheme.ts
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicate helper function across two files
- Extracted resolveDiffThemeName into the shared lib/diffRendering.ts module and updated both DiffPanel.tsx and DiffWorkerPoolProvider.tsx to import from it.
Or push these changes by commenting:
@cursor push 48f196d911
Preview (48f196d911)
diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx--- a/apps/web/src/components/DiffPanel.tsx+++ b/apps/web/src/components/DiffPanel.tsx@@ -10,7 +10,7 @@
import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch";
import { isElectron } from "../env";
import { useTheme } from "../hooks/useTheme";
-import { buildPatchCacheKey } from "../lib/diffRendering";+import { buildPatchCacheKey, resolveDiffThemeName } from "../lib/diffRendering";
import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries";
import { useStore } from "../store";
import { ToggleGroup, Toggle } from "./ui/toggle-group";
@@ -18,10 +18,6 @@
type DiffRenderMode = "stacked" | "split";
type DiffThemeType = "light" | "dark";
-function resolveDiffThemeName(theme: "light" | "dark") {- return theme === "dark" ? "pierre-dark" : "pierre-light";-}-
type RenderablePatch =
| {
kind: "files";
diff --git a/apps/web/src/components/DiffWorkerPoolProvider.tsx b/apps/web/src/components/DiffWorkerPoolProvider.tsx--- a/apps/web/src/components/DiffWorkerPoolProvider.tsx+++ b/apps/web/src/components/DiffWorkerPoolProvider.tsx@@ -2,11 +2,8 @@
import DiffsWorker from "@pierre/diffs/worker/worker.js?worker";
import { useEffect, useMemo, type ReactNode } from "react";
import { useTheme } from "../hooks/useTheme";
+import { resolveDiffThemeName } from "../lib/diffRendering";-function resolveDiffThemeName(theme: "light" | "dark") {- return theme === "dark" ? "pierre-dark" : "pierre-light";-}-
function DiffWorkerThemeSync({ themeName }: { themeName: "pierre-light" | "pierre-dark" }) {
const workerPool = useWorkerPool();
diff --git a/apps/web/src/lib/diffRendering.ts b/apps/web/src/lib/diffRendering.ts--- a/apps/web/src/lib/diffRendering.ts+++ b/apps/web/src/lib/diffRendering.ts@@ -12,6 +12,10 @@
return hash >>> 0;
}
+export function resolveDiffThemeName(theme: "light" | "dark") {+ return theme === "dark" ? "pierre-dark" : "pierre-light";+}+
export function buildPatchCacheKey(patch: string, scope = "diff-panel"): string {
const normalizedPatch = patch.trim();
const primary = fnv1a32(normalizedPatch, FNV_OFFSET_BASIS_32, FNV_PRIME_32).toString(36);| function resolveDiffThemeName(theme: "light" | "dark") { | ||
| return theme === "dark" ? "pierre-dark" : "pierre-light"; | ||
| } |
Contributor
There was a problem hiding this comment.
Duplicate helper function across two files
Low Severity
resolveDiffThemeName is identically defined in both DiffPanel.tsx and DiffWorkerPoolProvider.tsx. If the theme name mapping ever changes (e.g., new theme variants), both copies need to be updated in sync, which is easy to miss. Extracting it to a shared module avoids the maintenance risk.
Additional Locations (1)
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pierre-light/pierre-dark) derived from the resolved app themeDiffWorkerThemeSyncuseThemesnapshot stability by tracking both stored theme and system dark-mode state for consistentuseSyncExternalStoreupdatesTesting
light,dark, andsystemand verify diff colors update immediately without stale highlightingsystemtheme selected, change OS color scheme and verify diff panel/worker rendering follows the new resolved themeNote
Medium Risk
Touches diff rendering and worker pool configuration; regressions could cause stale or incorrect syntax highlighting when toggling themes, but changes are UI-only and not security/data-critical.
Overview
Ensures diff rendering updates cleanly on theme changes by mapping the app’s resolved theme to explicit Pierre themes (
pierre-light/pierre-dark) and passing them intoFileDiffoptions.Updates the diff worker pool to initialize and continuously sync highlighter/render options to the current theme via a new
DiffWorkerThemeSync, and adjustsuseTheme’suseSyncExternalStoresnapshot to include cached system dark-mode state for more consistent updates (especially whentheme=system).Written by Cursor Bugbot for commit df323f8. This will update automatically on new commits. Configure here.
Note
Sync diff renderer and worker theme with resolved app theme in
apps/web/src/components/DiffPanel.tsxandapps/web/src/components/DiffWorkerPoolProvider.tsxRestrict
DiffThemeTypeto"light" | "dark", addresolveDiffThemeNameto map resolved themes to"pierre-light"or"pierre-dark", remountFileDiffitems on theme change via keyedresolvedTheme, and propagate theme names to the worker pool render and highlighter options. Core changes are in DiffPanel.tsx, DiffWorkerPoolProvider.tsx, and useTheme.ts.📍Where to Start
Start with
resolveDiffThemeNameand its usage inDiffPanelin DiffPanel.tsx, then reviewDiffWorkerPoolProviderandDiffWorkerThemeSyncin DiffWorkerPoolProvider.tsx.Macroscope summarized df323f8.
Summary by CodeRabbit
Changes
Improvements