feat(desktop): persist Electric collections to SQLite for offline launch#3841
feat(desktop): persist Electric collections to SQLite for offline launch#3841saddlepaddle merged 3 commits intomainfrom
Conversation
Wraps all 21 Electric synced collections with @tanstack/electron-db-sqlite-persistence so the local cache survives cold start and the app's core shell renders offline. Drops loading-status guards on the Tasks view that were tying spinners to "Electric has confirmed sync," which kept the page broken until the network responded — now gated on data presence so cached rows render immediately.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe pull request introduces TanStack SQLite persistence infrastructure to the Electron main process, initializes it during app startup and shutdown, removes loading state UI from task view components, and migrates Electric-synced collections to use SQLite-backed persistence instead of IndexedDB. Changes
Sequence Diagram(s)sequenceDiagram
participant MainProcess as Main Process
participant AppInit as App Initialization
participant FileSys as File System
participant SQLiteDB as SQLite Database
participant TanStackAdapter as TanStack Adapter
participant IPC as IPC Channel
participant Renderer as Renderer Process
AppInit->>MainProcess: Application starts
MainProcess->>MainProcess: initAppState()
MainProcess->>MainProcess: initTanstackDbPersistence()
MainProcess->>FileSys: Ensure SUPERSET_HOME_DIR exists
MainProcess->>SQLiteDB: Open/create tanstack-db.sqlite
SQLiteDB-->>MainProcess: Database connection
MainProcess->>TanStackAdapter: Create SQLite persistence adapter
MainProcess->>IPC: exposeElectronSQLitePersistence(ipcMain)
IPC->>Renderer: IPC persistence available
Renderer-->>IPC: Request persisted collections
rect rgba(200, 150, 255, 0.5)
note over MainProcess,IPC: Renderer uses persistence for collections
IPC-->>Renderer: Persistence instance
end
AppInit->>MainProcess: before-quit event
MainProcess->>MainProcess: shutdownTanstackDbPersistence()
MainProcess->>TanStackAdapter: Invoke disposer
MainProcess->>SQLiteDB: Close connection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Greptile SummaryThis PR persists all 21 Electric-synced collections to SQLite ( Confidence Score: 4/5Safe to merge with one minor resource-management fix recommended before production: the SQLite database handle should be explicitly closed on shutdown. All findings are P2. The database-not-closed issue is unlikely to cause data corruption (SQLite is crash-safe, OS releases locks on process exit), but it conflicts with the PR's own stated test goal of no WAL stragglers and could interfere with a future re-init path. The hardcoded schemaVersion is a known limitation documented in the follow-ups. The isLoading → data-presence change is intentional and correct. apps/desktop/src/main/lib/persistence/persistence.ts — database handle should be stored at module scope and closed in shutdownTanstackDbPersistence
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/lib/persistence/persistence.ts | New file: opens better-sqlite3 DB and registers IPC handlers; database handle leaks on shutdown because only the IPC-handler dispose function is stored in module scope |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | Wraps all 21 Electric collections with SQLite persistence via createPersistedElectricCollection; schemaVersion is hardcoded at 1 with no per-collection override path |
| apps/desktop/src/main/index.ts | Integrates initTanstackDbPersistence into app lifecycle (whenReady → before-quit), placement is correct |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx | Replaces isLoading spinner with data-presence check (integrations !== undefined); intentional trade-off to unblock offline rendering |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksData/useTasksData.tsx | Drops isLoading from return type; callers updated consistently |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx | Drops isLoading from return type; consistent with useTasksData change |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/BoardContent/BoardContent.tsx | Removes isLoading spinner; falls through to empty-state check when no data |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TableContent/TableContent.tsx | Removes isLoading spinner; falls through to empty-state check when no rows |
Sequence Diagram
sequenceDiagram
participant R as Renderer (collections.ts)
participant IPC as IPC Bridge (window.ipcRenderer)
participant M as Main Process (persistence.ts)
participant DB as better-sqlite3 (tanstack-db.sqlite)
participant E as Electric Server
Note over M: app.whenReady → initTanstackDbPersistence()
M->>DB: new Database("tanstack-db.sqlite")
M->>M: createNodeSQLitePersistence({ database })
M->>M: exposeElectronSQLitePersistence({ ipcMain, persistence })
Note over R: Module load → createElectronSQLitePersistence
R->>R: createPersistedElectricCollection(electricCollectionOptions)
Note over R: App renders — cold start
R->>IPC: invoke(tanstack-db:read, { collectionId })
IPC->>M: IPC channel
M->>DB: SELECT * FROM collection_table
DB-->>M: cached rows
M-->>IPC: rows
IPC-->>R: cached data (no network needed)
R->>R: render shell immediately
Note over R: Electric sync (online)
R->>E: shape subscription
E-->>R: up-to-date rows
R->>IPC: invoke(tanstack-db:write, { collectionId, rows })
IPC->>M: IPC channel
M->>DB: UPSERT rows
Note over M: before-quit → shutdownTanstackDbPersistence()
M->>M: dispose() — removes IPC handlers
Note over M,DB: database.close() not called
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/persistence/persistence.ts
Line: 11-23
Comment:
**Database handle not closed on shutdown**
`database` is a local variable — only `dispose` (the IPC-handler teardown returned by `exposeElectronSQLitePersistence`) is kept in module scope. Calling `shutdownTanstackDbPersistence` removes the IPC handlers but never calls `database.close()`. On `before-quit` the process then force-exits via `app.exit(0)`. While the OS will release file locks, SQLite may not flush its page cache or checkpoint a WAL file cleanly, which is exactly what the test plan item "No SQLite lockfile / WAL stragglers in `~/.superset/`" is testing for.
Store the database reference at module scope alongside `dispose` so shutdown can close it explicitly:
```ts
let dispose: (() => void) | null = null;
let db: Database.Database | null = null;
export function initTanstackDbPersistence(): void {
ensureSupersetHomeDirExists();
db = new Database(join(SUPERSET_HOME_DIR, "tanstack-db.sqlite"));
const persistence = createNodeSQLitePersistence({ database: db });
dispose = exposeElectronSQLitePersistence({ ipcMain, persistence });
}
export function shutdownTanstackDbPersistence(): void {
dispose?.();
dispose = null;
db?.close();
db = null;
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts
Line: 79-91
Comment:
**`schemaVersion` is not overridable per collection**
`createPersistedElectricCollection` hardcodes `schemaVersion: 1` for every collection. If any single collection needs a breaking change (field rename, type change, new `getKey`), the only way to bump it is to change the shared constant — which forces all 21 collections to drop and re-pull their caches simultaneously. This is fine for now, but consider accepting an optional `schemaVersion` parameter so individual collections can be bumped independently when the need arises:
```ts
const createPersistedElectricCollection = ((
config: ElectricSyncConfig,
schemaVersion = 1,
) => {
const persisted = persistedCollectionOptions({
...config,
persistence,
schemaVersion,
} as any);
return createCollection({ ...persisted, ...indexDefaults } as any);
}) as unknown as typeof createCollection;
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "chore(desktop): drop AGENTS.md persisten..." | Re-trigger Greptile
| let dispose: (() => void) | null = null; | ||
|
|
||
| export function initTanstackDbPersistence(): void { | ||
| ensureSupersetHomeDirExists(); | ||
| const database = new Database(join(SUPERSET_HOME_DIR, "tanstack-db.sqlite")); | ||
| const persistence = createNodeSQLitePersistence({ database }); | ||
| dispose = exposeElectronSQLitePersistence({ ipcMain, persistence }); | ||
| } | ||
|
|
||
| export function shutdownTanstackDbPersistence(): void { | ||
| dispose?.(); | ||
| dispose = null; | ||
| } |
There was a problem hiding this comment.
Database handle not closed on shutdown
database is a local variable — only dispose (the IPC-handler teardown returned by exposeElectronSQLitePersistence) is kept in module scope. Calling shutdownTanstackDbPersistence removes the IPC handlers but never calls database.close(). On before-quit the process then force-exits via app.exit(0). While the OS will release file locks, SQLite may not flush its page cache or checkpoint a WAL file cleanly, which is exactly what the test plan item "No SQLite lockfile / WAL stragglers in ~/.superset/" is testing for.
Store the database reference at module scope alongside dispose so shutdown can close it explicitly:
let dispose: (() => void) | null = null;
let db: Database.Database | null = null;
export function initTanstackDbPersistence(): void {
ensureSupersetHomeDirExists();
db = new Database(join(SUPERSET_HOME_DIR, "tanstack-db.sqlite"));
const persistence = createNodeSQLitePersistence({ database: db });
dispose = exposeElectronSQLitePersistence({ ipcMain, persistence });
}
export function shutdownTanstackDbPersistence(): void {
dispose?.();
dispose = null;
db?.close();
db = null;
}Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/persistence/persistence.ts
Line: 11-23
Comment:
**Database handle not closed on shutdown**
`database` is a local variable — only `dispose` (the IPC-handler teardown returned by `exposeElectronSQLitePersistence`) is kept in module scope. Calling `shutdownTanstackDbPersistence` removes the IPC handlers but never calls `database.close()`. On `before-quit` the process then force-exits via `app.exit(0)`. While the OS will release file locks, SQLite may not flush its page cache or checkpoint a WAL file cleanly, which is exactly what the test plan item "No SQLite lockfile / WAL stragglers in `~/.superset/`" is testing for.
Store the database reference at module scope alongside `dispose` so shutdown can close it explicitly:
```ts
let dispose: (() => void) | null = null;
let db: Database.Database | null = null;
export function initTanstackDbPersistence(): void {
ensureSupersetHomeDirExists();
db = new Database(join(SUPERSET_HOME_DIR, "tanstack-db.sqlite"));
const persistence = createNodeSQLitePersistence({ database: db });
dispose = exposeElectronSQLitePersistence({ ipcMain, persistence });
}
export function shutdownTanstackDbPersistence(): void {
dispose?.();
dispose = null;
db?.close();
db = null;
}
```
How can I resolve this? If you propose a fix, please make it concise.| const createPersistedElectricCollection = ((config: ElectricSyncConfig) => { | ||
| const persisted = persistedCollectionOptions({ | ||
| ...config, | ||
| persistence, | ||
| schemaVersion: 1, | ||
| // biome-ignore lint/suspicious/noExplicitAny: forces sync-wrapped overload | ||
| } as any); | ||
| return createCollection({ | ||
| ...persisted, | ||
| ...indexDefaults, | ||
| // biome-ignore lint/suspicious/noExplicitAny: persisted utils widen generics | ||
| } as any); | ||
| }) as unknown as typeof createCollection; |
There was a problem hiding this comment.
schemaVersion is not overridable per collection
createPersistedElectricCollection hardcodes schemaVersion: 1 for every collection. If any single collection needs a breaking change (field rename, type change, new getKey), the only way to bump it is to change the shared constant — which forces all 21 collections to drop and re-pull their caches simultaneously. This is fine for now, but consider accepting an optional schemaVersion parameter so individual collections can be bumped independently when the need arises:
const createPersistedElectricCollection = ((
config: ElectricSyncConfig,
schemaVersion = 1,
) => {
const persisted = persistedCollectionOptions({
...config,
persistence,
schemaVersion,
} as any);
return createCollection({ ...persisted, ...indexDefaults } as any);
}) as unknown as typeof createCollection;Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts
Line: 79-91
Comment:
**`schemaVersion` is not overridable per collection**
`createPersistedElectricCollection` hardcodes `schemaVersion: 1` for every collection. If any single collection needs a breaking change (field rename, type change, new `getKey`), the only way to bump it is to change the shared constant — which forces all 21 collections to drop and re-pull their caches simultaneously. This is fine for now, but consider accepting an optional `schemaVersion` parameter so individual collections can be bumped independently when the need arises:
```ts
const createPersistedElectricCollection = ((
config: ElectricSyncConfig,
schemaVersion = 1,
) => {
const persisted = persistedCollectionOptions({
...config,
persistence,
schemaVersion,
} as any);
return createCollection({ ...persisted, ...indexDefaults } as any);
}) as unknown as typeof createCollection;
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
apps/desktopwith@tanstack/electron-db-sqlite-persistenceso the local cache survives cold start. SQLite db lives at~/.superset/tanstack-db.sqlite, mirroring the existing~/.superset/local.dbconvention. The 6 local-only collections stay onlocalStorageCollectionOptions(already offline-capable; migrating them carries data-migration risk that's out of scope here).useLiveQuery'sisLoadingguards on the Tasks view.isLoadingiscollection.status === "loading", which alpha 0.1.9 only flips toreadyafter Electric's first up-to-date handshake — so the page sat on a spinner forever offline even though cached rows were available. Now gated on data presence.How it works
Renderer constructs persistence via
createElectronSQLitePersistence({ invoke })using the existingwindow.ipcRenderer.invokepreload bridge — no preload changes. Main process opensbetter-sqlite3(already a dep, already rebuilt for Electron viaelectron-builder) and registers IPC handlers inwhenReadyafterinitAppState, disposes them inbefore-quit. New helpercreatePersistedElectricCollectionwrapselectricCollectionOptionswithpersistedCollectionOptions({ persistence, schemaVersion: 1 })+ index defaults; mechanically applied to all 21 sites.Per-org isolation: collection ids are already org-scoped (
tasks-${organizationId}), so SQLite tables don't collide across orgs. Per-user: matches existinglocal.dbconvention (machine-wide, not cleared on sign-out).Test plan
~/.superset/tanstack-db.sqlitewithsqlite3CLI; confirm 21 tables per active org~/.superset/Follow-ups
OrganizationSettings.tsxhas the sameisLoadingpattern overmembers— settings-only, not on the offline-critical path, deferredv2TerminalPresets,v2WorkspaceLocalState, etc.) is a separate, larger PR@tanstack/*-db-sqlite-persistence@0.1.9is "first alpha release of persistence") — pin exact versions, monitor releasesSummary by cubic
Persist Electric-synced collections to SQLite so cached data survives cold start and the desktop app launches offline. Also removes Tasks view loading guards so cached data renders immediately instead of spinning.
New Features
persistedCollectionOptions(schemaVersion: 1) using@tanstack/electron-db-sqlite-persistence; data is stored at~/.superset/tanstack-db.sqlitewith org-scoped tables.@tanstack/node-db-sqlite-persistence+better-sqlite3, exposed over existing IPC; lifecycle managed withinitTanstackDbPersistence/shutdownTanstackDbPersistence.localStorageCollectionOptions(no migration in this PR).Bug Fixes
isLoadingguards in Tasks Board/Table and Linear check; views now render whendataexists.Written for commit 5f2646d. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
UI Improvements