-
Notifications
You must be signed in to change notification settings - Fork 966
Persist zustand stores using electron-store #109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { ipcMain } from "electron"; | ||
| import { store } from "./storage-manager"; | ||
|
|
||
| /** | ||
| * Register storage IPC handlers | ||
| * These handlers provide access to electron-store from the renderer process | ||
| */ | ||
| 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); | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import Store from "electron-store"; | ||
| import { homedir } from "node:os"; | ||
| import { join } from "node:path"; | ||
|
|
||
| /** | ||
| * Electron store instance for persisting application state | ||
| * Stores data in ~/.superset/app-state.json | ||
| */ | ||
| export const store = new Store({ | ||
| cwd: join(homedir(), ".superset"), | ||
| name: "app-state", | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,20 @@ | ||||||||||||||||||||||||||
| import { createJSONStorage } from "zustand/middleware"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Custom Zustand storage adapter that uses electron-store for persistence via IPC | ||||||||||||||||||||||||||
| * Stores state in ~/.superset/app-state.json | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| const electronStorageAdapter = { | ||||||||||||||||||||||||||
| getItem: async (name: string): Promise<string | null> => { | ||||||||||||||||||||||||||
| const value = await window.electronStore.get(name); | ||||||||||||||||||||||||||
| return value as string | null; | ||||||||||||||||||||||||||
|
Comment on lines
+8
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsafe type assertion could cause runtime errors. The type cast 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| setItem: async (name: string, value: string): Promise<void> => { | ||||||||||||||||||||||||||
| await window.electronStore.set(name, value); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| removeItem: async (name: string): Promise<void> => { | ||||||||||||||||||||||||||
| await window.electronStore.delete(name); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export const electronStorage = createJSONStorage(() => electronStorageAdapter); | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,16 @@ | ||||||||||
| import { useWorkspacesStore } from "renderer/stores/workspaces"; | ||||||||||
| import { useAddTab } from "renderer/stores"; | ||||||||||
|
|
||||||||||
| export function NewWorkspaceView() { | ||||||||||
| const activeWorkspaceId = useWorkspacesStore( | ||||||||||
| (state) => state.activeWorkspaceId, | ||||||||||
| ); | ||||||||||
| const addWorkspace = useWorkspacesStore((state) => state.addWorkspace); | ||||||||||
| const markWorkspaceAsUsed = useWorkspacesStore( | ||||||||||
| (state) => state.markWorkspaceAsUsed, | ||||||||||
| ); | ||||||||||
| const addTab = useAddTab(); | ||||||||||
|
|
||||||||||
| return ( | ||||||||||
| <div className="flex-1 h-full flex items-center justify-center"> | ||||||||||
| <div className="text-center max-w-2xl px-6"> | ||||||||||
|
|
@@ -29,11 +41,27 @@ export function NewWorkspaceView() { | |||||||||
| </button> | ||||||||||
| <button | ||||||||||
| type="button" | ||||||||||
| onClick={() => { | ||||||||||
| if (!activeWorkspaceId) { | ||||||||||
| addWorkspace(); | ||||||||||
| setTimeout(() => { | ||||||||||
| const newWorkspaceId = | ||||||||||
| useWorkspacesStore.getState().activeWorkspaceId; | ||||||||||
| if (newWorkspaceId) { | ||||||||||
| addTab(newWorkspaceId); | ||||||||||
| markWorkspaceAsUsed(newWorkspaceId); | ||||||||||
| } | ||||||||||
| }, 0); | ||||||||||
| } else { | ||||||||||
| addTab(activeWorkspaceId); | ||||||||||
| markWorkspaceAsUsed(activeWorkspaceId); | ||||||||||
| } | ||||||||||
| }} | ||||||||||
|
Comment on lines
+44
to
+59
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race condition: Remove setTimeout anti-pattern. The 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 🤖 Prompt for AI Agents |
||||||||||
| className="p-6 border border-border rounded-lg hover:bg-accent hover:border-accent-foreground transition-colors text-left" | ||||||||||
| > | ||||||||||
| <div className="text-2xl mb-2">🔍</div> | ||||||||||
| <div className="font-medium text-foreground">Search</div> | ||||||||||
| <div className="text-sm text-muted-foreground">Find anything</div> | ||||||||||
| <div className="font-medium text-foreground">Create Tab & Start</div> | ||||||||||
| <div className="text-sm text-muted-foreground">Test persistence</div> | ||||||||||
|
Comment on lines
+63
to
+64
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| </button> | ||||||||||
| <button | ||||||||||
| type="button" | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,14 @@ | ||
| import type { MosaicNode } from "react-mosaic-component"; | ||
| import { create } from "zustand"; | ||
| import { devtools } from "zustand/middleware"; | ||
| import { devtools, persist } from "zustand/middleware"; | ||
| import { | ||
| cleanLayout, | ||
| handleDragTabToTab, | ||
| removeTabFromLayout, | ||
| } from "./drag-logic"; | ||
| import { type Tab, TabType } from "./types"; | ||
| import { createNewTab, getChildTabIds } from "./utils"; | ||
| import { electronStorage } from "../../lib/electron-storage"; | ||
|
|
||
| interface TabsState { | ||
| tabs: Tab[]; | ||
|
|
@@ -174,10 +175,11 @@ const handleEmptyGroupRemoval = ( | |
|
|
||
| export const useTabsStore = create<TabsState>()( | ||
| devtools( | ||
| (set, get) => ({ | ||
| tabs: createInitialTabs(), | ||
| activeTabIds: { "workspace-1": "tab-single-1" }, | ||
| tabHistoryStacks: { "workspace-1": [] }, | ||
| persist( | ||
| (set, get) => ({ | ||
| tabs: [], | ||
| activeTabIds: {}, | ||
| tabHistoryStacks: {}, | ||
|
Comment on lines
+180
to
+182
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Breaking change: Empty initial state will affect new and existing users. The initial state has changed from populated tabs (via
Consider initializing with Example approach: - tabs: [],
- activeTabIds: {},
- tabHistoryStacks: {},
+ tabs: createInitialTabs(),
+ activeTabIds: { "workspace-1": "tab-single-1" },
+ tabHistoryStacks: {},Then let persistence override these defaults on subsequent loads.
🤖 Prompt for AI Agents |
||
|
|
||
| addTab: (workspaceId, type = TabType.Single) => { | ||
| const newTab = createNewTab(workspaceId, type); | ||
|
|
@@ -605,7 +607,12 @@ export const useTabsStore = create<TabsState>()( | |
| const historyStack = get().tabHistoryStacks[workspaceId] || []; | ||
| return historyStack[0] || null; | ||
| }, | ||
| }), | ||
| }), | ||
| { | ||
| name: "tabs-storage", | ||
| storage: electronStorage, | ||
| }, | ||
| ), | ||
| { name: "TabsStore" }, | ||
| ), | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { create } from "zustand"; | ||
| import { devtools } from "zustand/middleware"; | ||
| import { devtools, persist } from "zustand/middleware"; | ||
| import { electronStorage } from "../lib/electron-storage"; | ||
|
|
||
| export interface Workspace { | ||
| id: string; | ||
|
|
@@ -26,65 +27,71 @@ const createNewWorkspace = (): Workspace => ({ | |
|
|
||
| export const useWorkspacesStore = create<WorkspacesState>()( | ||
| devtools( | ||
| (set) => ({ | ||
| workspaces: [{ id: "workspace-1", title: "Home" }], | ||
| activeWorkspaceId: "workspace-1", | ||
| 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, | ||
| }, | ||
| }), | ||
| ), | ||
|
Comment on lines
+30
to
+94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify initial empty state doesn't break existing components. The initial state changed to Run this script to verify all consumers handle null/empty state: Ensure all consuming components handle:
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 🏁 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 🏁 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 Fix: Change line 12 to: if (!activeWorkspace || activeWorkspace?.isNew) {This ensures 🤖 Prompt for AI Agents |
||
| { name: "WorkspacesStore" }, | ||
| ), | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| /** | ||
| * Storage-related IPC channels for electron-store persistence | ||
| */ | ||
|
|
||
| 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; | ||
| }; | ||
| } | ||
|
Comment on lines
+5
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Replace Using 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 |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling and input validation.
The IPC handlers lack:
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; + } }); }