Persist zustand stores using electron-store#109
Conversation
WalkthroughThe changes introduce persistent storage to the Electron desktop app using electron-store. A new storage layer is implemented with IPC handlers in the main process, a Zustand adapter in the renderer, and persistence wiring added to existing tabs and workspaces stores. The UI is updated to leverage the new persistence functionality. Changes
Sequence DiagramsequenceDiagram
participant App as Main Process
participant IPC as IPC Handler
participant Store as electron-store
participant Preload as Preload Script
participant Renderer as Renderer Process
participant Zustand as Zustand Store
App->>IPC: registerStorageHandlers()
Note over IPC: Set up 3 IPC channels<br/>storage:get/set/delete
Renderer->>Zustand: Initialize with<br/>electronStorage adapter
Zustand->>Preload: Call window.electronStore.get/set
Preload->>IPC: Send IPC message<br/>(storage:get/set/delete)
IPC->>Store: Delegate to store.get/set/delete
Store->>Store: Persist to<br/>~/.superset/app-state.json
Store-->>IPC: Return result
IPC-->>Preload: IPC response
Preload-->>Zustand: Return value
Zustand-->>Renderer: Update state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
43-95: Remove unused createInitialTabs function.This function is no longer called after switching to empty initial state. Either use it for first-time initialization or remove it to avoid confusion.
🧹 Nitpick comments (6)
apps/desktop/src/main/index.ts (1)
27-28: Consider moving storage handler registration inside app.whenReady().While
ipcMain.handle()can be called before the app is ready, electron-store may require the app to be fully initialized to determine the correct user data path. Moving the registration inside theapp.whenReady()callback ensures all Electron APIs are available.Apply this diff:
-// Register storage IPC handlers -registerStorageHandlers(); - // Allow multiple instances - removed single instance lock (async () => { await app.whenReady(); + // Register storage IPC handlers + registerStorageHandlers(); + await makeAppSetup(() => MainWindow()); // Stop all periodic rescans when app is quittingapps/desktop/src/renderer/stores/tabs/store.ts (1)
611-614: Consider adding state validation and migration logic.The persist configuration lacks:
- Schema validation on load (corrupt or incompatible state could crash the app)
- Migration logic for schema changes
- Version tracking
Zustand's persist middleware supports a
migratefunction andversionfield to handle schema evolution.Example:
{ name: "tabs-storage", storage: electronStorage, + version: 1, + migrate: (persistedState: any, version: number) => { + // Validate and migrate state if needed + if (!persistedState?.tabs || !Array.isArray(persistedState.tabs)) { + return { tabs: [], activeTabIds: {}, tabHistoryStacks: {} }; + } + return persistedState; + }, }apps/desktop/src/renderer/lib/electron-storage.ts (1)
7-18: Add error handling for IPC failures.IPC calls can fail due to various reasons (main process crashed, IPC channel not registered, etc.). The adapter should handle these gracefully to prevent store corruption.
const electronStorageAdapter = { getItem: async (name: string): Promise<string | null> => { + try { const value = await window.electronStore.get(name); return value as string | null; + } catch (error) { + console.error(`Failed to get item "${name}":`, error); + return null; + } }, setItem: async (name: string, value: string): Promise<void> => { + try { await window.electronStore.set(name, value); + } catch (error) { + console.error(`Failed to set item "${name}":`, error); + } }, removeItem: async (name: string): Promise<void> => { + try { await window.electronStore.delete(name); + } catch (error) { + console.error(`Failed to remove item "${name}":`, error); + } }, };apps/desktop/src/main/lib/storage-manager.ts (1)
9-12: Consider adding TypeScript schema for type safety.electron-store supports generic type parameters to provide type-safe access to stored data. This prevents runtime errors from type mismatches.
Example:
+interface StoreSchema { + 'tabs-storage': string; + 'workspaces-storage': string; + // Add other storage keys as needed +} + -export const store = new Store({ +export const store = new Store<StoreSchema>({ cwd: join(homedir(), ".superset"), name: "app-state", });This ensures compile-time type checking for all storage operations.
apps/desktop/src/main/lib/storage-ipcs.ts (1)
15-15: Replaceanywith a more specific type.Using
anyfor the value parameter disables type checking and could allow storing incompatible data that breaks on deserialization.Consider:
-async (_event, input: { key: string; value: any }) => { +async (_event, input: { key: string; value: unknown }) => {Using
unknownforces explicit type checking before use, preventing runtime errors.apps/desktop/src/preload/index.ts (1)
13-17: Improve type safety for electronStore API.The
getmethod returnsany, which disables type checking. Consider usingunknownto enforce type validation at the call site.electronStore: { - get: (key: string) => any; + get: (key: string) => Promise<unknown>; set: (key: string, value: any) => void; delete: (key: string) => void; };Note: The methods should also return
Promise<void>for consistency since they're async IPC calls:electronStore: { get: (key: string) => Promise<unknown>; - set: (key: string, value: any) => void; - delete: (key: string) => void; + set: (key: string, value: any) => Promise<void>; + delete: (key: string) => Promise<void>; };
📜 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 (12)
apps/desktop/electron.vite.config.ts(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/main/index.ts(2 hunks)apps/desktop/src/main/lib/storage-ipcs.ts(1 hunks)apps/desktop/src/main/lib/storage-manager.ts(1 hunks)apps/desktop/src/preload/index.ts(2 hunks)apps/desktop/src/renderer/lib/electron-storage.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx(2 hunks)apps/desktop/src/renderer/stores/tabs/store.ts(3 hunks)apps/desktop/src/renderer/stores/workspaces.ts(2 hunks)apps/desktop/src/shared/ipc-channels/index.ts(2 hunks)apps/desktop/src/shared/ipc-channels/storage.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/desktop/src/main/lib/storage-ipcs.ts (1)
apps/desktop/src/main/lib/storage-manager.ts (1)
store(9-12)
apps/desktop/src/shared/ipc-channels/index.ts (1)
apps/desktop/src/shared/ipc-channels/storage.ts (1)
StorageChannels(5-20)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx (2)
apps/desktop/src/renderer/stores/workspaces.ts (1)
useWorkspacesStore(28-97)apps/desktop/src/renderer/stores/tabs/store.ts (1)
useAddTab(624-624)
apps/desktop/src/renderer/stores/tabs/store.ts (2)
apps/old-desktop/src/main/lib/storage/lowdb-adapter.ts (2)
set(174-184)get(163-172)apps/desktop/src/renderer/lib/electron-storage.ts (1)
electronStorage(20-20)
apps/desktop/src/main/index.ts (1)
apps/desktop/src/main/lib/storage-ipcs.ts (1)
registerStorageHandlers(8-23)
apps/desktop/src/renderer/stores/workspaces.ts (1)
apps/desktop/src/renderer/lib/electron-storage.ts (1)
electronStorage(20-20)
🔇 Additional comments (4)
apps/desktop/electron.vite.config.ts (1)
28-28: LGTM!Correctly externalizes electron-store to prevent bundling issues with native Electron modules.
apps/desktop/src/preload/index.ts (1)
74-83: LGTM!The electronStore API correctly bridges IPC calls to the main process storage handlers via contextBridge, following Electron security best practices.
apps/desktop/package.json (1)
47-47: electron-store version 11.0.2 is valid with no known vulnerabilities.Verification confirms that electron-store version 11.0.2 exists on npm and has no known security advisories in the GitHub security database.
apps/desktop/src/shared/ipc-channels/index.ts (1)
11-11: LGTM: Storage channels properly integrated.The
StorageChannelsinterface is correctly imported and added to theIpcChannelstype aggregation, maintaining type safety across IPC boundaries.Also applies to: 39-40
| export function registerStorageHandlers() { | ||
| ipcMain.handle("storage:get", async (_event, input: { key: string }) => { | ||
| return store.get(input.key); | ||
| }); | ||
|
|
||
| ipcMain.handle( | ||
| "storage:set", | ||
| async (_event, input: { key: string; value: any }) => { | ||
| store.set(input.key, input.value); | ||
| }, | ||
| ); | ||
|
|
||
| ipcMain.handle("storage:delete", async (_event, input: { key: string }) => { | ||
| store.delete(input.key); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add error handling and input validation.
The IPC handlers lack:
- Error handling for store operations (could throw)
- Input validation (empty/invalid keys)
- Return value handling for storage:set
Without error handling, exceptions will crash or hang the renderer process awaiting the IPC response.
Apply this diff:
export function registerStorageHandlers() {
ipcMain.handle("storage:get", async (_event, input: { key: string }) => {
+ if (!input?.key || typeof input.key !== 'string') {
+ throw new Error('Invalid key provided');
+ }
+ try {
return store.get(input.key);
+ } catch (error) {
+ console.error(`Failed to get key "${input.key}":`, error);
+ throw error;
+ }
});
ipcMain.handle(
"storage:set",
async (_event, input: { key: string; value: any }) => {
+ if (!input?.key || typeof input.key !== 'string') {
+ throw new Error('Invalid key provided');
+ }
+ try {
store.set(input.key, input.value);
+ } catch (error) {
+ console.error(`Failed to set key "${input.key}":`, error);
+ throw error;
+ }
},
);
ipcMain.handle("storage:delete", async (_event, input: { key: string }) => {
+ if (!input?.key || typeof input.key !== 'string') {
+ throw new Error('Invalid key provided');
+ }
+ try {
store.delete(input.key);
+ } catch (error) {
+ console.error(`Failed to delete key "${input.key}":`, error);
+ throw error;
+ }
});
}| getItem: async (name: string): Promise<string | null> => { | ||
| const value = await window.electronStore.get(name); | ||
| return value as string | null; |
There was a problem hiding this comment.
Unsafe type assertion could cause runtime errors.
The type cast as string | null on line 10 assumes the stored value is always a string, but electron-store can return any type. If non-string data was previously stored under the same key, this will cause issues when Zustand tries to parse it as JSON.
Apply this diff to add validation:
getItem: async (name: string): Promise<string | null> => {
const value = await window.electronStore.get(name);
- return value as string | null;
+ if (value === undefined || value === null) return null;
+ if (typeof value !== 'string') {
+ console.warn(`Expected string for key "${name}", got ${typeof value}`);
+ return null;
+ }
+ return value;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getItem: async (name: string): Promise<string | null> => { | |
| const value = await window.electronStore.get(name); | |
| return value as string | null; | |
| getItem: async (name: string): Promise<string | null> => { | |
| const value = await window.electronStore.get(name); | |
| if (value === undefined || value === null) return null; | |
| if (typeof value !== 'string') { | |
| console.warn(`Expected string for key "${name}", got ${typeof value}`); | |
| return null; | |
| } | |
| return value; | |
| }, |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/lib/electron-storage.ts around lines 8 to 10,
remove the unsafe type assertion and add runtime validation: check if the
retrieved value is undefined or null and return null; if it's already a string
return it; otherwise convert non-string values to a JSON string (JSON.stringify)
before returning so Zustand receives a string and you avoid runtime type errors.
| onClick={() => { | ||
| if (!activeWorkspaceId) { | ||
| addWorkspace(); | ||
| setTimeout(() => { | ||
| const newWorkspaceId = | ||
| useWorkspacesStore.getState().activeWorkspaceId; | ||
| if (newWorkspaceId) { | ||
| addTab(newWorkspaceId); | ||
| markWorkspaceAsUsed(newWorkspaceId); | ||
| } | ||
| }, 0); | ||
| } else { | ||
| addTab(activeWorkspaceId); | ||
| markWorkspaceAsUsed(activeWorkspaceId); | ||
| } | ||
| }} |
There was a problem hiding this comment.
Race condition: Remove setTimeout anti-pattern.
The setTimeout(..., 0) pattern is unnecessary and introduces a race condition. Zustand state updates are synchronous by default, so activeWorkspaceId is updated immediately when addWorkspace() completes. The setTimeout adds complexity without benefit and the direct getState() call (line 49) bypasses the hook-based reactivity model.
Apply this diff to simplify the logic:
onClick={() => {
if (!activeWorkspaceId) {
addWorkspace();
- setTimeout(() => {
- const newWorkspaceId =
- useWorkspacesStore.getState().activeWorkspaceId;
- if (newWorkspaceId) {
- addTab(newWorkspaceId);
- markWorkspaceAsUsed(newWorkspaceId);
- }
- }, 0);
- } else {
- addTab(activeWorkspaceId);
- markWorkspaceAsUsed(activeWorkspaceId);
}
+ // activeWorkspaceId is now guaranteed to be set (either existing or newly created)
+ const currentWorkspaceId = useWorkspacesStore.getState().activeWorkspaceId;
+ if (currentWorkspaceId) {
+ addTab(currentWorkspaceId);
+ markWorkspaceAsUsed(currentWorkspaceId);
+ }
}}Alternatively, refactor addWorkspace to return the new workspace ID for even cleaner code.
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx
around lines 44 to 59, remove the setTimeout anti-pattern and the bypass of hook
reactivity: call addWorkspace(), then immediately read the new ID from
useWorkspacesStore.getState() (or even better, change addWorkspace to return the
new workspace ID) and proceed to addTab(newWorkspaceId) and
markWorkspaceAsUsed(newWorkspaceId); if activeWorkspaceId already exists, keep
the existing branch that calls addTab(activeWorkspaceId) and
markWorkspaceAsUsed(activeWorkspaceId). Ensure no asynchronous wrapper
(setTimeout) is used and rely on Zustand’s synchronous update or return value
from addWorkspace.
| <div className="font-medium text-foreground">Create Tab & Start</div> | ||
| <div className="text-sm text-muted-foreground">Test persistence</div> |
There was a problem hiding this comment.
Replace debug text with production-ready copy.
The subtitle "Test persistence" appears to be temporary debug text. Please update with appropriate user-facing copy before merging.
Suggested alternatives:
-<div className="text-sm text-muted-foreground">Test persistence</div>
+<div className="text-sm text-muted-foreground">Open a new tab</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="font-medium text-foreground">Create Tab & Start</div> | |
| <div className="text-sm text-muted-foreground">Test persistence</div> | |
| <div className="font-medium text-foreground">Create Tab & Start</div> | |
| <div className="text-sm text-muted-foreground">Open a new tab</div> |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx
around lines 63-64, the subtitle currently reads "Test persistence" which is
debug text; replace it with production-ready user-facing copy such as "Save your
workspace settings", "Automatically persist tabs and state", or simply
"Workspace will be saved automatically" (choose the wording that best fits the
UI tone), and ensure the string is i18n-ready if the app uses localization.
| tabs: [], | ||
| activeTabIds: {}, | ||
| tabHistoryStacks: {}, |
There was a problem hiding this comment.
Breaking change: Empty initial state will affect new and existing users.
The initial state has changed from populated tabs (via createInitialTabs()) to empty structures. This means:
- New users won't see welcome tabs on first launch
- The
createInitialTabs()function (lines 43-95) is now unused
Consider initializing with createInitialTabs() on first load, then relying on persistence for subsequent sessions.
Example approach:
- tabs: [],
- activeTabIds: {},
- tabHistoryStacks: {},
+ tabs: createInitialTabs(),
+ activeTabIds: { "workspace-1": "tab-single-1" },
+ tabHistoryStacks: {},Then let persistence override these defaults on subsequent loads.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/stores/tabs/store.ts around lines 180 to 182, the
reducer/store initial state was changed to empty structures which removes the
welcome/default tabs and leaves createInitialTabs() unused; restore initial tabs
by calling createInitialTabs() when initializing state if no persisted state
exists (i.e., use createInitialTabs() as the default initial value or run it on
first load), then allow the persistence layer to override that default on
subsequent loads so new users still see the welcome tabs while existing users
keep their persisted tabs.
| persist( | ||
| (set) => ({ | ||
| workspaces: [], | ||
| activeWorkspaceId: null, | ||
|
|
||
| addWorkspace: () => { | ||
| const newWorkspace = createNewWorkspace(); | ||
| set((state) => ({ | ||
| workspaces: [...state.workspaces, newWorkspace], | ||
| activeWorkspaceId: newWorkspace.id, | ||
| })); | ||
| }, | ||
|
|
||
| removeWorkspace: (id) => { | ||
| set((state) => { | ||
| const workspaces = state.workspaces.filter( | ||
| (workspace) => workspace.id !== id, | ||
| ); | ||
| if (workspaces.length === 0) { | ||
| const newWorkspace = createNewWorkspace(); | ||
| return { | ||
| workspaces: [newWorkspace], | ||
| activeWorkspaceId: newWorkspace.id, | ||
| }; | ||
| } | ||
| addWorkspace: () => { | ||
| const newWorkspace = createNewWorkspace(); | ||
| set((state) => ({ | ||
| workspaces: [...state.workspaces, newWorkspace], | ||
| activeWorkspaceId: newWorkspace.id, | ||
| })); | ||
| }, | ||
|
|
||
| if (id === state.activeWorkspaceId) { | ||
| const closedIndex = state.workspaces.findIndex( | ||
| (workspace) => workspace.id === id, | ||
| removeWorkspace: (id) => { | ||
| set((state) => { | ||
| const workspaces = state.workspaces.filter( | ||
| (workspace) => workspace.id !== id, | ||
| ); | ||
| const nextWorkspace = | ||
| workspaces[closedIndex] || workspaces[closedIndex - 1]; | ||
| return { workspaces, activeWorkspaceId: nextWorkspace.id }; | ||
| } | ||
| if (workspaces.length === 0) { | ||
| const newWorkspace = createNewWorkspace(); | ||
| return { | ||
| workspaces: [newWorkspace], | ||
| activeWorkspaceId: newWorkspace.id, | ||
| }; | ||
| } | ||
|
|
||
| return { workspaces }; | ||
| }); | ||
| }, | ||
| if (id === state.activeWorkspaceId) { | ||
| const closedIndex = state.workspaces.findIndex( | ||
| (workspace) => workspace.id === id, | ||
| ); | ||
| const nextWorkspace = | ||
| workspaces[closedIndex] || workspaces[closedIndex - 1]; | ||
| return { workspaces, activeWorkspaceId: nextWorkspace.id }; | ||
| } | ||
|
|
||
| setActiveWorkspace: (id) => { | ||
| set({ activeWorkspaceId: id }); | ||
| }, | ||
| return { workspaces }; | ||
| }); | ||
| }, | ||
|
|
||
| reorderWorkspaces: (startIndex, endIndex) => { | ||
| set((state) => { | ||
| const workspaces = [...state.workspaces]; | ||
| const [removed] = workspaces.splice(startIndex, 1); | ||
| workspaces.splice(endIndex, 0, removed); | ||
| return { workspaces }; | ||
| }); | ||
| }, | ||
| setActiveWorkspace: (id) => { | ||
| set({ activeWorkspaceId: id }); | ||
| }, | ||
|
|
||
| reorderWorkspaces: (startIndex, endIndex) => { | ||
| set((state) => { | ||
| const workspaces = [...state.workspaces]; | ||
| const [removed] = workspaces.splice(startIndex, 1); | ||
| workspaces.splice(endIndex, 0, removed); | ||
| return { workspaces }; | ||
| }); | ||
| }, | ||
|
|
||
| markWorkspaceAsUsed: (id) => { | ||
| set((state) => ({ | ||
| workspaces: state.workspaces.map((workspace) => | ||
| workspace.id === id ? { ...workspace, isNew: false } : workspace, | ||
| ), | ||
| })); | ||
| markWorkspaceAsUsed: (id) => { | ||
| set((state) => ({ | ||
| workspaces: state.workspaces.map((workspace) => | ||
| workspace.id === id ? { ...workspace, isNew: false } : workspace, | ||
| ), | ||
| })); | ||
| }, | ||
| }), | ||
| { | ||
| name: "workspaces-storage", | ||
| storage: electronStorage, | ||
| }, | ||
| }), | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
Verify initial empty state doesn't break existing components.
The initial state changed to workspaces: [] and activeWorkspaceId: null (lines 32-33). While appropriate for persistence (loading saved state), this differs from previous behavior and may break components that assume at least one workspace always exists. The empty-workspace guard in removeWorkspace (lines 48-54) doesn't cover the initial load case.
Run this script to verify all consumers handle null/empty state:
Ensure all consuming components handle:
activeWorkspaceId === nullworkspaces.length === 0
Or initialize with a default workspace if this breaks expected behavior.
🏁 Script executed:
#!/bin/bash
# Description: Find components using activeWorkspaceId and workspaces to verify null/empty handling
# Search for activeWorkspaceId usage
echo "=== Components accessing activeWorkspaceId ==="
rg -n --type=tsx --type=ts -C3 'activeWorkspaceId' -g '!*.test.*' -g '!workspaces.ts'
echo -e "\n=== Components accessing workspaces array ==="
rg -n --type=tsx --type=ts -C3 'useWorkspacesStore.*workspaces[^a-zA-Z]' -g '!*.test.*' -g '!workspaces.ts'
echo -e "\n=== Check for .workspaces[0] or similar direct access ==="
rg -n --type=tsx --type=ts 'workspaces\[0\]' -g '!*.test.*'Length of output: 236
I need to fix the script—the --type=tsx flag isn't recognized by ripgrep. Let me run a corrected version that properly searches TypeScript files:
🏁 Script executed:
#!/bin/bash
# Description: Find components using activeWorkspaceId and workspaces to verify null/empty handling
# Search for activeWorkspaceId usage (corrected)
echo "=== Components accessing activeWorkspaceId ==="
rg -n 'activeWorkspaceId' --type=ts -C3 -g '!*.test.*' -g '!workspaces.ts'
echo -e "\n=== Components accessing workspaces array ==="
rg -n 'workspaces\s*\??\.' --type=ts -C3 -g '!*.test.*' -g '!workspaces.ts'
echo -e "\n=== Check for .workspaces[0] or similar direct array access ==="
rg -n 'workspaces\s*\[0\]' --type=ts -g '!*.test.*'
echo -e "\n=== Check for activeWorkspaceId?.id or null checks ==="
rg -n 'activeWorkspaceId\?' --type=ts -C2 -g '!workspaces.ts'Length of output: 66636
I need to examine more closely what happens when the store initializes with empty state. Let me search for app initialization and the WorkspaceView render logic:
Let me search for the electronStorage implementation to see if it's synchronous or asynchronous:
🏁 Script executed:
#!/bin/bash
# Find electronStorage definition and usage
echo "=== Find electronStorage definition ==="
rg -n 'electronStorage' --type=ts -B2 -A5 -g '!*.test.*' | head -60
echo -e "\n=== Check if electronStorage is sync or async ==="
rg -n 'export.*electronStorage|const.*electronStorage.*=' --type=ts -A3 -g '!*.test.*'Length of output: 3928
Now let me examine the WorkspaceView component more closely to see how it handles the case where activeWorkspace might be undefined:
🏁 Script executed:
#!/bin/bash
# Get full WorkspaceView implementation
echo "=== WorkspaceView full implementation ==="
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx
echo -e "\n=== Check app initialization / root component ==="
rg -n 'WorkspaceView|main entry' --type=ts -B3 -A3 | head -40Length of output: 4474
Let me verify if there's app initialization that creates a default workspace, and check what Sidebar renders:
🏁 Script executed:
#!/bin/bash
# Check for app initialization / root setup
echo "=== Check App.tsx or main entry ==="
find apps/desktop/src/renderer -name "App.tsx" -o -name "main.tsx" -o -name "index.tsx" | head -5 | xargs -I {} sh -c 'echo "File: {}"; head -40 "{}"'
echo -e "\n=== Check Sidebar implementation ==="
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx 2>/dev/null | head -60
echo -e "\n=== Check if there's any useEffect that initializes workspace ==="
rg -n 'useEffect|init.*workspace' --type=ts apps/desktop/src/renderer/screens/main/ -A3 | head -40Length of output: 11062
WorkspaceView doesn't handle the undefined activeWorkspace case during initial load.
While most consumers properly guard against null activeWorkspaceId, the WorkspaceView component (line 12) only checks if (activeWorkspace?.isNew). When activeWorkspaceId is null during async storage hydration, activeWorkspace becomes undefined, causing the condition to be false. This renders the default Sidebar + ContentView layout instead of NewWorkspaceView, creating a broken UI momentarily.
Fix: Change line 12 to:
if (!activeWorkspace || activeWorkspace?.isNew) {This ensures NewWorkspaceView renders when no active workspace exists, matching the intended UX flow where users create their first workspace.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/stores/workspaces.ts around lines 30 to 94, the
store allows activeWorkspaceId to be null during async hydration which can make
consumers receive undefined activeWorkspace; update consumers to treat missing
activeWorkspace as the "new" case by checking for both undefined/null and isNew.
Specifically, change the guard in WorkspaceView from only checking
activeWorkspace?.isNew to a combined check that treats !activeWorkspace
(null/undefined) or activeWorkspace.isNew as the NewWorkspaceView case so the UI
shows the creation flow during initial load.
| export interface StorageChannels { | ||
| "storage:get": { | ||
| request: { key: string }; | ||
| response: any; | ||
| }; | ||
|
|
||
| "storage:set": { | ||
| request: { key: string; value: any }; | ||
| response: void; | ||
| }; | ||
|
|
||
| "storage:delete": { | ||
| request: { key: string }; | ||
| response: void; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any with unknown for type safety.
Using any for response (line 8) and value (line 12) bypasses TypeScript's type checking. Use unknown instead to maintain type safety while allowing any value—consumers must then explicitly narrow the type before use.
Apply this diff:
export interface StorageChannels {
"storage:get": {
request: { key: string };
- response: any;
+ response: unknown;
};
"storage:set": {
- request: { key: string; value: any };
+ request: { key: string; value: unknown };
response: void;
};
"storage:delete": {
request: { key: string };
response: void;
};
}🤖 Prompt for AI Agents
In apps/desktop/src/shared/ipc-channels/storage.ts around lines 5 to 20, replace
the uses of the any type with unknown: change the "storage:get" response type
from any to unknown and change the "storage:set" request.value type from any to
unknown so callers must explicitly narrow types; leave other shapes unchanged
and run TypeScript checks to ensure no downstream type errors.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Updates
✏️ Tip: You can customize this high-level summary in your review settings.