refactor(desktop): Restyled and less intrusive/interupting update install#464
refactor(desktop): Restyled and less intrusive/interupting update install#464
Conversation
WalkthroughThis PR introduces an event-driven auto-update architecture for the desktop application. A new TRPC router exposes auto-update operations (subscribe, getStatus, install, dismiss, check), while the refactored auto-updater emits status events via an EventEmitter instead of triggering direct UI dialogs. A new UpdateToast React component subscribes to these events to render update notifications. The MainWindow no longer registers itself with the auto-updater. Changes
Sequence DiagramsequenceDiagram
participant App as App Startup
participant AU as Auto-updater<br/>(main)
participant Emitter as EventEmitter
participant TRPC as TRPC Router
participant UI as UpdateToast<br/>(renderer)
App->>AU: setupAutoUpdater()
activate AU
AU->>AU: Register auto-updater<br/>lifecycle listeners
AU->>Emitter: Emit STATUS_CHANGED<br/>(status: IDLE)
deactivate AU
rect rgb(200, 220, 240)
Note over AU,UI: Periodic/User-Triggered Check
AU->>AU: checkForUpdates()
AU->>Emitter: Emit STATUS_CHANGED<br/>(status: CHECKING)
Emitter-->>TRPC: subscribe() stream
TRPC-->>UI: Status update event
UI->>UI: Re-render
end
rect rgb(220, 240, 200)
Note over AU,UI: Update Available & Download
AU->>Emitter: Emit STATUS_CHANGED<br/>(status: DOWNLOADING)
Emitter-->>TRPC: subscribe() stream
TRPC-->>UI: Status update event
UI->>UI: Show download message
AU->>Emitter: Emit STATUS_CHANGED<br/>(status: READY)
Emitter-->>TRPC: subscribe() stream
TRPC-->>UI: Status update event
UI->>UI: Show "Install" button
end
rect rgb(240, 220, 200)
Note over UI,AU: User Action
UI->>TRPC: installAndRestart mutation
TRPC->>AU: installUpdate()
AU->>Emitter: Emit STATUS_CHANGED<br/>(status: IDLE)
Emitter-->>TRPC: subscribe() stream
TRPC-->>UI: Confirm & restart
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
AviPeltz
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (1)
49-55: Consider handling the ERROR status for user feedback.The component only shows UI for
DOWNLOADINGandREADYstates. When an error occurs during update checking or downloading, the user receives no visual feedback. Consider whether displaying an error state (perhaps with a retry option) would improve user experience.🔎 Optional: Add error state handling
const isDownloading = status?.status === AUTO_UPDATE_STATUS.DOWNLOADING; const isReady = status?.status === AUTO_UPDATE_STATUS.READY; + const isError = status?.status === AUTO_UPDATE_STATUS.ERROR; // Only show when downloading or ready - if (!status || (!isDownloading && !isReady)) { + if (!status || (!isDownloading && !isReady && !isError)) { return null; }Then add error UI in the render section if desired.
apps/desktop/src/lib/trpc/routers/auto-update/index.ts (1)
87-93: DEV-only guard is in the implementation, not the router.The docstring says "DEV ONLY" but this endpoint is publicly accessible. The runtime guard exists in
simulateUpdateReady()inauto-updater.ts, which logs a warning and returns early in production. This is acceptable since the effect is harmless in production, but consider whether the endpoint should be conditionally registered.apps/desktop/src/main/lib/auto-updater.ts (1)
205-209: Consider exposing download progress to the UI.Currently,
download-progressonly logs to console. Since the UI shows "Downloading v{version}...", exposing the progress percentage could enhance user experience. This could be done by extending the status payload or adding a separate progress field.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/renderer/index.tsxapps/desktop/src/shared/constants.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/main/windows/main.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/main/lib/auto-updater.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/main/lib/auto-updater.ts
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/renderer/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/renderer/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/main/lib/auto-updater.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/main/lib/auto-updater.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/auto-updater.ts
🧬 Code graph analysis (4)
apps/desktop/src/renderer/index.tsx (1)
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (1)
UpdateToast(7-97)
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (3)
apps/desktop/src/renderer/components/UpdateToast/index.ts (1)
UpdateToast(1-1)apps/desktop/src/shared/constants.ts (1)
AUTO_UPDATE_STATUS(55-61)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)
apps/desktop/src/lib/trpc/routers/index.ts (1)
apps/desktop/src/lib/trpc/routers/auto-update/index.ts (1)
createAutoUpdateRouter(20-95)
apps/desktop/src/main/lib/auto-updater.ts (1)
apps/desktop/src/shared/constants.ts (4)
AutoUpdateStatus(63-64)AUTO_UPDATE_STATUS(55-61)AUTO_UPDATE_EVENTS(50-52)PLATFORM(4-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (10)
apps/desktop/src/renderer/components/UpdateToast/index.ts (1)
1-1: LGTM!Clean barrel export following the project's component organization pattern.
apps/desktop/src/renderer/index.tsx (1)
5-5: LGTM!UpdateToast is correctly imported via the barrel file and placed within AppProviders, ensuring it has access to the tRPC context for subscription and mutations.
Also applies to: 16-16
apps/desktop/src/lib/trpc/routers/index.ts (1)
4-4: LGTM!The auto-update router is correctly integrated into the application router. Unlike window-dependent routers, it appropriately uses an event-driven approach and doesn't require a window reference.
Also applies to: 29-29
apps/desktop/src/shared/constants.ts (1)
48-64: LGTM!Well-structured constants following the existing patterns in this file. The status values comprehensively cover the auto-update lifecycle, and the type derivation is idiomatic TypeScript.
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (1)
7-31: LGTM on the tRPC integration pattern.The hook setup correctly uses:
- Query for initial status fetch
- Mutations with
onSuccesscallbacks to invalidate the cache- Subscription for real-time updates with cache invalidation
- Ref pattern to avoid effect dependency issues
This aligns well with the coding guidelines requiring tRPC for Electron IPC and the observable pattern for subscriptions.
apps/desktop/src/lib/trpc/routers/auto-update/index.ts (2)
25-54: LGTM on the subscription implementation.The observable pattern is correctly used as required by the coding guidelines for trpc-electron subscriptions. The implementation properly:
- Emits current status immediately on subscribe
- Sets up the event listener for subsequent updates
- Returns a cleanup function to remove the listener
1-11: No issues found. The router correctly imports from main process modules and executes in the main process context, as intended.apps/desktop/src/main/lib/auto-updater.ts (3)
16-23: LGTM on the state management approach.The module-level state variables with an EventEmitter provide a clean, centralized state management for auto-update status. This effectively decouples the UI from direct dialog interactions.
169-231: LGTM on setupAutoUpdater implementation.The setup correctly:
- Guards against dev/non-macOS environments
- Configures autoDownload and autoInstallOnAppQuit
- Sets up all lifecycle event handlers with appropriate status emissions
- Uses
interval.unref()to prevent keeping the process alive- Handles both ready and not-ready app states for initial check
71-73: TheisForceRunAfter=trueparameter is ignored whenisSilent=false.The
isForceRunAfterparameter is ignored ifisSilentis set tofalse. WithisSilent=false, the installer runs in non-silent mode, showing the UI, and theautoRunAppAfterInstallsetting controls whether the app runs after installation. If auto-restart after installation is required, verify that eitherisSilentshould betruewithisForceRunAfter=true, or thatautoRunAppAfterInstallis properly configured for non-silent mode.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.