Rebuild Electron build system from scratch#84
Conversation
|
Warning Rate limit exceeded@saddlepaddle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (71)
WalkthroughA large refactoring PR that modernizes the desktop app's build system, applies consistent path alias imports across ~80 files, removes legacy configuration files, and replaces dynamic electron-builder configuration with a TypeScript-based approach. The workspace manager is decomposed into modular components, and the development environment is simplified with a fixed dev port and reduced dynamic environment variable injection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DevServer as Dev Server<br/>(Port 4927)
participant Renderer
participant IPC
participant Main
Note over DevServer: Old: Dynamic port via getPortSync()
Note over DevServer: New: Fixed port 4927, non-strict mode
User->>DevServer: Request app on fixed port 4927
DevServer->>Renderer: Serve renderer assets from dist/renderer
Renderer->>IPC: IPC setup (uses `@/` path aliases)
IPC->>Main: Main process handlers (uses `@/` path aliases)
Main->>Renderer: Event responses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Completely rewrote the Electron build configuration using best practices: - Build outputs now go to dist/ instead of node_modules/.dev/ - Simplified package.json scripts (removed complex pre/post hooks) - Clean electron.vite.config.ts with proper workspace package handling - TypeScript electron-builder.config.ts for type safety - Fixed TSConfig path resolution (baseUrl: "./src") - Removed fragile environment variable loading - Updated turbo.jsonc to cache dist/ properly This fixes the issue where adding new packages would break the build due to build artifacts living in node_modules. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
@superset/ui doesn't have a root "." export (only subpath exports), so Vite's optimizeDeps.include was failing. Workspace packages that export raw source files don't need to be pre-bundled - Vite handles them automatically. Dev mode now works correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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/screens/main/components/SidebarOverlay/SidebarOverlay.tsx (1)
12-12: Update inline import to use the alias path.The inline type import on line 12 still uses the old path format
import("shared/types").Worktree, which is inconsistent with the alias-based imports used elsewhere in this file and across the PR.Apply this diff to align with the standardized import paths:
- onUpdateWorktree: (worktreeId: string, updatedWorktree: import("shared/types").Worktree) => void; + onUpdateWorktree: (worktreeId: string, updatedWorktree: import("@/shared/types").Worktree) => void;Alternatively, import the type at the top of the file to avoid inline imports:
-import type { Workspace } from "@/shared/types"; +import type { Workspace, Worktree } from "@/shared/types";Then update line 12:
- onUpdateWorktree: (worktreeId: string, updatedWorktree: import("shared/types").Worktree) => void; + onUpdateWorktree: (worktreeId: string, updatedWorktree: Worktree) => void;
🧹 Nitpick comments (1)
apps/desktop/electron-builder.config.ts (1)
10-13: Consider reusing appId generation logic.The appId generation logic here duplicates the logic in
apps/desktop/src/shared/utils.ts. Consider importingmakeAppIdfrom the shared utilities to maintain DRY principles.import { join } from "node:path"; import type { Configuration } from "electron-builder"; import pkg from "./package.json"; +import { makeAppId } from "./src/shared/utils"; const currentYear = new Date().getFullYear(); -const author = pkg.author?.name ?? pkg.author; -const authorInKebabCase = author.replace(/\s+/g, "-"); -const appId = `com.${authorInKebabCase}.${pkg.name}`.toLowerCase(); +const author = pkg.author?.name ?? pkg.author; +const appId = makeAppId(); const config: Configuration = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (69)
apps/desktop/.editorconfig(0 hunks)apps/desktop/.github/FUNDING.yml(0 hunks)apps/desktop/.gitignore(0 hunks)apps/desktop/.npmrc(0 hunks)apps/desktop/.vscode/extensions.json(0 hunks)apps/desktop/.vscode/settings.json(0 hunks)apps/desktop/ELECTRON_INSTALL_FIX.md(0 hunks)apps/desktop/components.json(0 hunks)apps/desktop/docs/DEEP_LINKING.md(0 hunks)apps/desktop/docs/TYPE_SAFE_IPC.md(0 hunks)apps/desktop/electron-builder.config.ts(1 hunks)apps/desktop/electron-builder.ts(0 hunks)apps/desktop/electron.vite.config.ts(2 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/lib/electron-app/factories/app/setup.ts(1 hunks)apps/desktop/src/lib/electron-app/factories/windows/create.ts(1 hunks)apps/desktop/src/main/index.ts(2 hunks)apps/desktop/src/main/lib/config-manager.ts(1 hunks)apps/desktop/src/main/lib/workspace-ipcs.ts(1 hunks)apps/desktop/src/main/lib/workspace-manager.ts(1 hunks)apps/desktop/src/main/lib/workspace-manager.ts.old(0 hunks)apps/desktop/src/main/lib/workspace/group-cleanup.ts(1 hunks)apps/desktop/src/main/lib/workspace/tab-helpers.ts(1 hunks)apps/desktop/src/main/lib/workspace/tab-operations.ts(1 hunks)apps/desktop/src/main/lib/workspace/workspace-operations.ts(1 hunks)apps/desktop/src/main/lib/workspace/worktree-operations.ts(1 hunks)apps/desktop/src/main/windows/main.ts(1 hunks)apps/desktop/src/preload/index.ts(1 hunks)apps/desktop/src/renderer/components/ui/dialog.tsx(1 hunks)apps/desktop/src/renderer/components/ui/input.tsx(1 hunks)apps/desktop/src/renderer/components/ui/label.tsx(1 hunks)apps/desktop/src/renderer/components/ui/separator.tsx(1 hunks)apps/desktop/src/renderer/contexts/WorktreeContext.tsx(1 hunks)apps/desktop/src/renderer/routes.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/AddTaskModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/TaskTabs.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/types.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/TabGroup.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContentArea/MainContentArea.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/KanbanColumn.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/TaskCard.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeModal/CreateWorktreeModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceCarousel/WorkspaceCarousel.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspacePortIndicator/WorkspacePortIndicator.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceSwitcher/WorkspaceSwitcher.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/WorktreeList.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/PortIndicator/PortIndicator.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/GitStatusDialog.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreePortsList/WorktreePortsList.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SidebarOverlay/SidebarOverlay.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TabContent/components/PortTab/PortTab.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TabContent/components/PreviewTab/PreviewTab.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useTabs.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useTasks.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts(1 hunks)apps/desktop/src/renderer/screens/main/utils.ts(1 hunks)apps/desktop/src/shared/types.ts(1 hunks)apps/desktop/src/shared/utils.ts(1 hunks)apps/desktop/tsconfig.json(1 hunks)turbo.jsonc(1 hunks)
💤 Files with no reviewable changes (12)
- apps/desktop/ELECTRON_INSTALL_FIX.md
- apps/desktop/.vscode/extensions.json
- apps/desktop/.gitignore
- apps/desktop/electron-builder.ts
- apps/desktop/.npmrc
- apps/desktop/docs/TYPE_SAFE_IPC.md
- apps/desktop/.github/FUNDING.yml
- apps/desktop/docs/DEEP_LINKING.md
- apps/desktop/src/main/lib/workspace-manager.ts.old
- apps/desktop/components.json
- apps/desktop/.vscode/settings.json
- apps/desktop/.editorconfig
🔇 Additional comments (64)
apps/desktop/src/renderer/contexts/WorktreeContext.tsx (1)
3-3: LGTM! Import path standardization applied correctly.The import path has been successfully updated to use the
@/alias, aligning with the codebase-wide refactor. Since both dev and build have been verified as working, this change is confirmed functional.apps/desktop/src/main/lib/workspace/worktree-operations.ts (1)
6-14: Alias import migration looks correctThe updated imports to
@/shared/typesand@/shared/utils/slugare consistent with the new alias scheme and do not alter runtime behavior. No further changes needed here.apps/desktop/src/main/lib/workspace/tab-helpers.ts (1)
2-2: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias, aligning with the project-wide module resolution refactoring.apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
4-4: LGTM! Import path updated to use project alias.The type import has been correctly updated to use the
@/alias for consistent module resolution across the codebase.apps/desktop/src/renderer/screens/main/components/TabContent/components/PreviewTab/PreviewTab.tsx (1)
12-12: LGTM! Import path updated to use project alias.The type import path has been correctly migrated to use the
@/alias, maintaining consistency with the build system refactoring.apps/desktop/src/renderer/screens/main/components/PlanView/KanbanColumn.tsx (1)
3-3: LGTM! Import path updated to use project alias.The type import has been correctly updated to use the
@/alias, consistent with the codebase-wide standardization.apps/desktop/src/renderer/screens/main/utils.ts (1)
1-1: LGTM! Import path updated to use project alias.The type import has been correctly migrated to the
@/alias pattern, aligning with the build system modernization.turbo.jsonc (1)
10-10: LGTM! Build output consolidated to dist/ directory.The build outputs have been correctly simplified to use the single
dist/**path, consistent with the cleaned-up build system. This removes the previous fragmented output directories and aligns with the electron-vite best practices implementation.apps/desktop/src/main/lib/workspace/tab-operations.ts (1)
3-10: LGTM! Import path updated to use project alias.The type imports have been correctly updated to use the
@/alias on line 10, maintaining consistency with the project-wide module resolution standardization.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx (1)
29-30: LGTM! Import paths updated to use project aliases.Both import statements have been correctly migrated to use the
@/alias pattern—@/renderer/components/ui/dialogfor UI components and@/shared/typesfor shared type definitions. This aligns with the standardized module resolution strategy across the codebase.apps/desktop/tsconfig.json (2)
19-23: LGTM! Path alias configuration is well-structured.The new path mappings correctly establish the foundation for the alias-based import strategy:
@/resolves to./src/for application code~/resolves to the monorepo root for cross-package importsThis aligns perfectly with the systematic import path updates across the codebase.
28-28: LGTM! Include path updated correctly.The update from
electron-builder.tstoelectron-builder.config.tsaligns with the config file migration mentioned in the PR objectives.apps/desktop/src/renderer/screens/main/components/TabContent/components/PortTab/PortTab.tsx (1)
4-4: LGTM! Import path updated correctly.The migration from
"shared/types"to"@/shared/types"is consistent with the project-wide alias strategy.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/GitStatusDialog.tsx (1)
4-11: LGTM! Dialog import correctly migrated to alias path.The update to
@/renderer/components/ui/dialogfollows the consistent alias pattern used throughout this refactor.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/PortIndicator/PortIndicator.tsx (1)
9-9: LGTM! Type import updated correctly.Consistent alias-based import migration.
apps/desktop/src/main/lib/workspace/workspace-operations.ts (1)
3-8: LGTM! Main process imports correctly migrated.The alias-based import works correctly in the main process, confirming the path resolution is properly configured for both renderer and main contexts.
apps/desktop/src/renderer/screens/main/hooks/useTasks.ts (1)
2-2: LGTM! Hook import path updated correctly.Consistent with the project-wide alias migration.
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
18-18: LGTM! Type imports correctly updated.Follows the consistent alias pattern used throughout this refactor.
apps/desktop/src/preload/index.ts (1)
2-6: LGTM! Preload context imports correctly migrated.The alias resolution working in the preload script confirms that the path configuration is properly set up across all Electron contexts (main, renderer, and preload).
apps/desktop/src/lib/electron-app/factories/windows/create.ts (1)
3-4: LGTM! Import paths successfully migrated to aliases.The import paths have been correctly updated to use the
@/alias prefix, aligning with the project-wide standardization effort.apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeModal/CreateWorktreeModal.tsx (1)
11-15: LGTM! UI component and type imports properly aliased.All import paths have been correctly migrated to use the
@/alias, maintaining consistency with the broader refactoring effort.apps/desktop/src/shared/types.ts (1)
3-3: LGTM! Type import correctly updated to alias path.The import path for
registerRoutehas been properly migrated to the@/alias format.apps/desktop/src/renderer/screens/main/components/SidebarOverlay/SidebarOverlay.tsx (1)
1-1: LGTM! Top-level import updated correctly.The Workspace type import has been properly migrated to use the
@/shared/typesalias.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceCarousel/WorkspaceCarousel.tsx (1)
9-9: LGTM! Type import migrated to alias path.The Workspace type import has been correctly updated to use the
@/shared/typesalias.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceSwitcher/WorkspaceSwitcher.tsx (1)
12-12: LGTM! Type import correctly aliased.The Workspace type import has been properly migrated to the
@/shared/typesalias path.apps/desktop/src/main/lib/config-manager.ts (1)
5-5: LGTM! Type import updated to alias path.The WorkspaceConfig type import has been correctly migrated to use the
@/shared/typesalias.apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts (1)
2-2: LGTM! Type imports migrated to alias path.The Workspace and Worktree type imports have been correctly updated to use the
@/shared/typesalias.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/WorktreeList.tsx (1)
3-3: LGTM! Alias-based import standardization.The import path update to
@/shared/typesaligns with the project-wide migration to standardized path aliases described in the PR objectives.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspacePortIndicator/WorkspacePortIndicator.tsx (1)
9-9: LGTM! Alias-based import standardization.The import path update to
@/shared/typesaligns with the project-wide migration to standardized path aliases.apps/desktop/src/renderer/screens/main/components/Layout/AddTaskModal.tsx (1)
30-30: LGTM! Alias-based import standardization.The import path update to
@/shared/typesaligns with the project-wide migration to standardized path aliases.apps/desktop/src/renderer/screens/main/components/MainContentArea/MainContentArea.tsx (1)
8-8: LGTM! Alias-based import standardization.The import path update to
@/shared/typesaligns with the project-wide migration to standardized path aliases.apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts (1)
2-2: LGTM! Alias-based import standardization.The import path update to
@/shared/typesaligns with the project-wide migration to standardized path aliases.apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx (1)
9-9: LGTM! Alias-based import standardization.The import path update to
@/shared/typesaligns with the project-wide migration to standardized path aliases.apps/desktop/src/renderer/routes.tsx (1)
1-1: LGTM! Alias-based import standardization.The import path update to
@/lib/electron-router-domaligns with the project-wide migration to standardized path aliases.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreePortsList/WorktreePortsList.tsx (1)
4-4: LGTM! Alias-based import standardization.The import path update to
@/shared/typesaligns with the project-wide migration to standardized path aliases.apps/desktop/src/main/windows/main.ts (1)
4-7: LGTM! Clean path alias migration.The import updates use
@/for internal modules and~/for root-level files (package.json), aligning with the build system refactor. DestructuringdisplayNamefrompkgis the correct approach.apps/desktop/src/main/lib/workspace/group-cleanup.ts (1)
1-1: LGTM!Type import correctly updated to use the
@/path alias.apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx (1)
2-2: LGTM!Type import correctly updated to use the
@/path alias, consistent with the build system refactor.apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/types.ts (1)
1-1: LGTM!Type import correctly updated to use the
@/path alias.apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx (1)
4-5: LGTM!Import paths correctly updated to use
@/aliases for both UI components and shared types, consistent with the build system refactor.apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/TaskTabs.tsx (1)
11-11: LGTM!Dialog component import correctly updated to use the
@/path alias.apps/desktop/src/renderer/screens/main/components/MainContent/TabGroup.tsx (1)
9-9: LGTM!Type import correctly updated to use the
@/path alias, consistent with the build system refactor.apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx (1)
5-5: LGTM!Type import correctly updated to use the
@/path alias, aligning with the project-wide build system refactor.apps/desktop/src/renderer/screens/main/hooks/useTabs.ts (1)
1-1: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias, aligning with the project-wide standardization of import paths.apps/desktop/src/main/lib/workspace-ipcs.ts (1)
10-10: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias for shared types, maintaining consistency with the broader path alias refactor.apps/desktop/src/renderer/components/ui/label.tsx (1)
4-4: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias, consistent with the standardization across UI components.apps/desktop/src/renderer/components/ui/separator.tsx (1)
3-3: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias for the utility function.apps/desktop/src/renderer/components/ui/input.tsx (1)
3-3: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias, maintaining consistency with other UI components.apps/desktop/src/main/lib/workspace-manager.ts (1)
11-11: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias for shared types, consistent with the project-wide path alias standardization.apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx (1)
5-5: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias for shared types, aligning with the standardized import structure.apps/desktop/src/renderer/screens/main/components/PlanView/TaskCard.tsx (1)
5-5: LGTM! Import path updated to use project alias.The import path has been correctly updated to use the
@/alias, consistent with the broader refactor across the codebase.apps/desktop/src/main/index.ts (2)
3-9: LGTM! Import path standardization.The migration to @/ alias-based imports is clean and consistent with the PR's objective to standardize import paths across the codebase.
46-47: LGTM! Dynamic import updated correctly.The dynamic import correctly uses the @/ alias and maintains the same functionality.
apps/desktop/src/renderer/components/ui/dialog.tsx (1)
5-5: LGTM! Consistent alias usage.The import path update aligns with the project-wide migration to @/ aliases.
apps/desktop/src/lib/electron-app/factories/app/setup.ts (1)
7-9: LGTM! Import standardization.The import paths have been successfully migrated to use @/ aliases, maintaining consistency with the broader refactoring effort.
apps/desktop/electron.vite.config.ts (4)
72-75: Good addition: React deduplication for workspace packages.The dedupe configuration for react and react-dom is essential when using workspace packages to prevent multiple React instances, which can cause hooks errors and other runtime issues.
16-16: LGTM! Standardized output to dist/ directory.The hardcoded output directories (dist/main, dist/preload, dist/renderer) align with the PR objective to clean up build output and the package.json main field update to
./dist/main/index.js.Also applies to: 28-28, 64-64
61-61: Directory exists and contains public assets — verification passed.The public assets directory at
src/resources/publichas been confirmed to exist and contains the expected public asset (illustration.svg). The configuration is correct.
35-36: I need to gather more information about the actual module type and electron-vite's handling of__dirname. Let me verify further:I need to verify the module type configuration and the full context of the config file:
Now let me check the actual package.json and config file structure:
The review comment is based on a misunderstanding of how electron-vite handles
__dirnamein config files.electron-vite is explicitly documented as compatible with CommonJS syntax including
__dirname, with no need to fix these syntax problems. More specifically, electron-vite supports config files suffixed with .ts, and electron-vite is compatible with__dirnameand there is no need to fix these syntax problems during migration.The key distinction is that configuration files are build-time tooling, not runtime code. electron-vite's own documentation shows
__dirnamebeing used directly in config files, demonstrating this is a supported pattern.However, the review comment raises a valid general concern: if your actual runtime code (not the config file) uses
__dirnamein an ESM context with"type": "module"in package.json, then that code would need to useimport.meta.urlwithfileURLToPath. You should verify whether__dirnameis used in runtime Electron code (main process, preload scripts, etc.), as those would require the suggested fix.Likely an incorrect or invalid review comment.
apps/desktop/electron-builder.config.ts (2)
15-88: LGTM! Comprehensive Electron Builder configuration.The configuration is well-structured with appropriate settings for all platforms (macOS, Linux, Windows), deep linking support, and sensible build optimizations. The TypeScript migration from the old electron-builder.js improves type safety.
23-23: Resource paths verified and correctly configured.Verification confirms all resource paths referenced in the electron-builder configuration exist and are accessible:
- Build resources directory:
apps/desktop/src/resources/build✓- Icon files present:
icon.icns,icon.ico✓No action required.
apps/desktop/package.json (3)
6-6: LGTM! Main entry point updated correctly.The main field now correctly points to the new dist/main/index.js output, aligning with the electron.vite.config.ts build output directory.
14-25: LGTM! Streamlined scripts with electron-vite and bun.The script reorganization simplifies the build workflow by:
- Using electron-vite for dev, build, and preview
- Leveraging bun for fast execution
- Removing complex multi-step processes
This aligns well with the PR objective to simplify the build system.
27-77: Removed dependencies are confirmed unused.Verification complete: cross-env, dotenv, open, and rollup-plugin-inject-process-env are not referenced anywhere in the codebase. The removal is safe.
Converted all bare imports to use path aliases for consistency: - @/* for src/ imports (e.g., @/main/lib/...) - ~/* for root imports (e.g., ~/package.json) This improves IDE autocomplete and makes import paths more explicit. All 52 files updated to use the new import convention. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Removed: - cross-env (no longer used in scripts) - dotenv (using Vite's envDir instead) - rollup-plugin-inject-process-env (removed from config) - open (not imported anywhere) - ELECTRON_INSTALL_FIX.md (outdated documentation) - src/main/lib/workspace-manager.ts.old (old backup file) Build and dev mode still work correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Removed files that should be at monorepo root or aren't needed: - .editorconfig, .npmrc (not used with Bun) - .vscode/, .github/ (root handles these) - components.json (belongs in packages/ui) - docs/ (TYPE_SAFE_IPC.md, DEEP_LINKING.md) - .gitignore (root has this) Kept only essential app-specific files: - electron.vite.config.ts - electron-builder.config.ts - tsconfig.json - index.d.ts (webview types) - package.json - trusted-dependencies-scripts.json Build verified working after cleanup.
saddlepaddle
force-pushed
the
desktop-build-audit
branch
from
7865e71 to
0533ffa
Compare
- Prefix unused variables/parameters with underscore - Fix module augmentation false positive for ImportMeta - Auto-fixed 70+ files with biome check --write Note: Remaining lint/typecheck errors are pre-existing from main branch
There was a problem hiding this comment.
Actionable comments posted: 1
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/screens/main/components/PlanView/TaskPage.tsx (1)
179-179: Fix missing arguments toonTabCreated()call.The pipeline failure indicates
onTabCreated()expects 2 arguments(worktreeId: string, tab: Tab)per the prop type at line 26, but is called with 0 arguments.Apply this diff:
- await onTabCreated(); + await onTabCreated(targetWorktreeId, result.tab);
♻️ Duplicate comments (1)
apps/desktop/src/shared/utils.ts (1)
1-6: Guard againstpkg.authorbeing null/non‑string (startup crash risk)
pkg.authorcan benull, a string, or an object. As written:
pkg.author.namewill throw ifpkg.authorisnull/undefined.author.replace(...)will throw ifauthoris not a string.Per previous review context, the root
package.jsoncurrently hasauthor: null, so this will crash at startup whenapps/desktop/src/shared/utils.tsis loaded.Normalize
pkg.authorto a safe string (object →.name, string → itself, anything else → fallback) before calling.replace, and also givepkg.namea fallback.Here’s a concrete fix:
-import pkg from "~/package.json"; - -const author = pkg.author.name ?? pkg.author; -const name = pkg.name; -const authorInKebabCase = author.replace(/\s+/g, "-"); -const appId = `com.${authorInKebabCase}.${name}`.toLowerCase(); +import pkg from "~/package.json"; + +const rawAuthor = pkg.author; +const author = + typeof rawAuthor === "string" + ? rawAuthor + : rawAuthor && + typeof rawAuthor === "object" && + "name" in rawAuthor && + typeof (rawAuthor as { name?: unknown }).name === "string" + ? (rawAuthor as { name: string }).name + : "unknown-author"; +const name = typeof pkg.name === "string" ? pkg.name : "app"; +const authorInKebabCase = author.replace(/\s+/g, "-"); +const appId = `com.${authorInKebabCase}.${name}`.toLowerCase();
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/TabContent/components/PreviewTab/PreviewTab.tsx (1)
212-213: Use thegetStorageKeyhelper consistently.The
getStorageKeyhelper function is defined at line 80 but these locations use hardcoded storage key strings instead. For better maintainability and DRY compliance, use the helper consistently throughout the component.Apply this diff to use the helper:
- try { - const storageKey = `preview-tab-${tab.id}-url`; - sessionStorage.setItem(storageKey, normalized); + try { + sessionStorage.setItem(getStorageKey("url"), normalized);And at line 358:
- try { - const storageKey = `preview-tab-${tab.id}-url`; - sessionStorage.setItem(storageKey, url); + try { + sessionStorage.setItem(getStorageKey("url"), url);Also applies to: 358-359
📜 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 (68)
apps/desktop/.editorconfig(0 hunks)apps/desktop/.github/FUNDING.yml(0 hunks)apps/desktop/.gitignore(0 hunks)apps/desktop/.npmrc(0 hunks)apps/desktop/.vscode/extensions.json(0 hunks)apps/desktop/.vscode/settings.json(0 hunks)apps/desktop/ELECTRON_INSTALL_FIX.md(0 hunks)apps/desktop/components.json(0 hunks)apps/desktop/docs/DEEP_LINKING.md(0 hunks)apps/desktop/docs/TYPE_SAFE_IPC.md(0 hunks)apps/desktop/electron-builder.config.ts(1 hunks)apps/desktop/electron-builder.ts(0 hunks)apps/desktop/electron.vite.config.ts(2 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/lib/electron-app/factories/app/setup.ts(1 hunks)apps/desktop/src/lib/electron-app/factories/windows/create.ts(1 hunks)apps/desktop/src/main/index.ts(2 hunks)apps/desktop/src/main/lib/config-manager.ts(1 hunks)apps/desktop/src/main/lib/workspace-ipcs.ts(1 hunks)apps/desktop/src/main/lib/workspace-manager.ts(1 hunks)apps/desktop/src/main/lib/workspace-manager.ts.old(0 hunks)apps/desktop/src/main/lib/workspace/group-cleanup.ts(1 hunks)apps/desktop/src/main/lib/workspace/tab-helpers.ts(1 hunks)apps/desktop/src/main/lib/workspace/tab-operations.ts(1 hunks)apps/desktop/src/main/lib/workspace/workspace-operations.ts(1 hunks)apps/desktop/src/main/lib/workspace/worktree-operations.ts(1 hunks)apps/desktop/src/main/windows/main.ts(1 hunks)apps/desktop/src/preload/index.ts(1 hunks)apps/desktop/src/renderer/components/ui/dialog.tsx(1 hunks)apps/desktop/src/renderer/components/ui/input.tsx(1 hunks)apps/desktop/src/renderer/components/ui/label.tsx(1 hunks)apps/desktop/src/renderer/components/ui/separator.tsx(1 hunks)apps/desktop/src/renderer/contexts/WorktreeContext.tsx(1 hunks)apps/desktop/src/renderer/routes.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/TaskTabs.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/types.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/TabGroup.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContentArea/MainContentArea.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/KanbanColumn.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/TaskCard.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeModal/CreateWorktreeModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceCarousel/WorkspaceCarousel.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspacePortIndicator/WorkspacePortIndicator.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceSwitcher/WorkspaceSwitcher.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/WorktreeList.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/PortIndicator/PortIndicator.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/GitStatusDialog.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreePortsList/WorktreePortsList.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SidebarOverlay/SidebarOverlay.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TabContent/components/PortTab/PortTab.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TabContent/components/PreviewTab/PreviewTab.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useTabs.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useTasks.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts(1 hunks)apps/desktop/src/renderer/screens/main/utils.ts(1 hunks)apps/desktop/src/shared/types.ts(1 hunks)apps/desktop/src/shared/utils.ts(1 hunks)apps/desktop/tsconfig.json(1 hunks)turbo.jsonc(1 hunks)
💤 Files with no reviewable changes (12)
- apps/desktop/components.json
- apps/desktop/.vscode/settings.json
- apps/desktop/.editorconfig
- apps/desktop/docs/TYPE_SAFE_IPC.md
- apps/desktop/src/main/lib/workspace-manager.ts.old
- apps/desktop/electron-builder.ts
- apps/desktop/.gitignore
- apps/desktop/ELECTRON_INSTALL_FIX.md
- apps/desktop/.vscode/extensions.json
- apps/desktop/.npmrc
- apps/desktop/.github/FUNDING.yml
- apps/desktop/docs/DEEP_LINKING.md
✅ Files skipped from review due to trivial changes (3)
- apps/desktop/src/main/lib/config-manager.ts
- apps/desktop/src/main/lib/workspace-ipcs.ts
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx
🚧 Files skipped from review as they are similar to previous changes (36)
- apps/desktop/src/renderer/components/ui/label.tsx
- apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx
- apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts
- apps/desktop/src/renderer/screens/main/components/TabContent/components/PortTab/PortTab.tsx
- apps/desktop/src/renderer/screens/main/hooks/useTabs.ts
- apps/desktop/src/main/lib/workspace/group-cleanup.ts
- apps/desktop/src/main/index.ts
- apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts
- apps/desktop/src/renderer/contexts/WorktreeContext.tsx
- apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx
- apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx
- apps/desktop/src/renderer/components/ui/separator.tsx
- apps/desktop/src/renderer/screens/main/components/PlanView/KanbanColumn.tsx
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspacePortIndicator/WorkspacePortIndicator.tsx
- apps/desktop/src/lib/electron-app/factories/windows/create.ts
- apps/desktop/src/renderer/components/ui/dialog.tsx
- apps/desktop/src/shared/types.ts
- apps/desktop/src/renderer/screens/main/utils.ts
- apps/desktop/src/main/lib/workspace/tab-helpers.ts
- apps/desktop/src/main/lib/workspace/workspace-operations.ts
- apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx
- apps/desktop/src/renderer/screens/main/components/MainContent/TabGroup.tsx
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/PortIndicator/PortIndicator.tsx
- apps/desktop/src/renderer/screens/main/components/MainContentArea/MainContentArea.tsx
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/WorktreeList.tsx
- apps/desktop/src/main/windows/main.ts
- apps/desktop/src/renderer/screens/main/components/PlanView/TaskCard.tsx
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeModal/CreateWorktreeModal.tsx
- turbo.jsonc
- apps/desktop/src/renderer/components/ui/input.tsx
- apps/desktop/src/main/lib/workspace/worktree-operations.ts
- apps/desktop/src/main/lib/workspace-manager.ts
- apps/desktop/src/renderer/screens/main/components/SidebarOverlay/SidebarOverlay.tsx
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceCarousel/WorkspaceCarousel.tsx
- apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/types.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx
🧰 Additional context used
🪛 GitHub Actions: CI
apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx
[error] 179-179: TS2554: Expected 2 arguments, but got 0.
🔇 Additional comments (19)
apps/desktop/src/main/lib/workspace/tab-operations.ts (1)
10-10: LGTM! Import path standardization applied correctly.The change from
"shared/types"to"@/shared/types"aligns with the PR's objective to standardize import paths using the@/alias across the codebase.apps/desktop/src/renderer/screens/main/components/TabContent/components/PreviewTab/PreviewTab.tsx (1)
12-12: LGTM! Path alias migration looks correct.The import path update from
"shared/types"to"@/shared/types"aligns with the PR's objective to standardize module resolution using path aliases. Since the PR reports successful verification with both dev and build commands, the alias configuration is properly set up.apps/desktop/src/renderer/screens/main/hooks/useTasks.ts (1)
2-2: LGTM! Clean import alias refactor.The import path update from
"shared/types"to"@/shared/types"aligns with the PR's standardization objectives and has been verified through successful dev and build testing.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
18-18: LGTM! Path alias update aligns with build system modernization.The import path update to use the
@/alias is part of the project-wide standardization effort and correctly resolves the types.apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx (1)
5-5: LGTM! Path alias update aligns with build system modernization.The import path update to use the
@/alias is part of the project-wide standardization effort and correctly resolves the types.apps/desktop/src/renderer/screens/main/components/Layout/TaskTabs/TaskTabs.tsx (1)
11-11: LGTM! Path alias update aligns with build system modernization.The import path update to use the
@/alias is part of the project-wide standardization effort and correctly resolves the Dialog components.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorkspaceSwitcher/WorkspaceSwitcher.tsx (1)
12-12: LGTM! Path alias update aligns with build system modernization.The import path update to use the
@/alias is part of the project-wide standardization effort and correctly resolves the Workspace type.apps/desktop/tsconfig.json (2)
19-23: LGTM! TypeScript configuration correctly supports path aliases.The
baseUrlandpathsconfiguration correctly enables@/(src-relative) and~/(monorepo root-relative) aliases used throughout the codebase. This aligns with the build system modernization objectives.
28-28: LGTM! Config file reference updated correctly.The include path correctly references the new
electron-builder.config.tsfile mentioned in the PR objectives, replacing the oldelectron-builder.ts.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/GitStatusDialog.tsx (1)
11-11: LGTM! Path alias update aligns with build system modernization.The import path update to use the
@/alias is part of the project-wide standardization effort and correctly resolves the Dialog components.apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreePortsList/WorktreePortsList.tsx (1)
4-4: LGTM! Path alias update aligns with build system modernization.The import path update to use the
@/alias is part of the project-wide standardization effort and correctly resolves the Worktree type.apps/desktop/src/renderer/routes.tsx (1)
1-1: LGTM! Path alias update aligns with build system modernization.The import path update to use the
@/alias is part of the project-wide standardization effort and correctly resolves the Router module.apps/desktop/src/preload/index.ts (1)
2-6: Alias import switch looks correctUsing
@/shared/ipc-channelshere aligns with the repo-wide aliasing strategy and doesn’t change runtime behavior. No issues.apps/desktop/src/lib/electron-app/factories/app/setup.ts (1)
7-9: Imports updated to alias paths without behavior changeThe switch to
@/…aliases forterminalManager,ENVIRONMENT/PLATFORM, andmakeAppIdis consistent with the new path strategy and keeps runtime behavior the same.apps/desktop/electron.vite.config.ts (1)
1-76: Build/output restructuring in electron-vite config is correctly wiredVerification confirms the outDir paths are consistent across configs: package.json's main field points to
./dist/main/index.js, electron-vite.config.ts outputs todist/main|preload|renderer, and electron-builder.config.ts captures them withdist/**/*. No functional issues detected.apps/desktop/package.json (4)
6-6: Main entry path aligns with new build output structure.Updated to
./dist/main/index.js, consistent with the PR's consolidation of build output todist/rather thannode_modules/.dev/. This assumes electron-vite is configured to output main todist/main/index.jsand that the Electron Builder config references this correctly.
14-19: Scripts successfully reorganized for electron-vite workflow.The new scripts cleanly separate concerns:
- Lines 14–16: electron-vite commands (
dev,build,preview)- Lines 17–19: Packaging orchestrated as
bun run build && electron-builder[...]This replaces the fragile environment-based setup and removes the need for cross-platform and environment-injection tooling. Script chaining is sound:
packageandreleaseboth callbun run buildfirst (invoking line 15's electron-vite build), ensuring a fresh build before packaging.
21-21: Clean script appropriately added.The
cleanscript removes thedist/output directory and is needed to support the new build structure.rimrafis present in devDependencies (line 70).
27-77: Dependency footprint appropriately streamlined.Four unused dependencies removed (cross-env, dotenv, open, rollup-plugin-inject-process-env), aligning with the PR's elimination of fragile environment loading and dynamic environment variable injection. Key build dependencies are retained:
- electron-vite (line 69), vite (line 75) for the new build system
- tsx (line 73) for TypeScript config execution
- electron-builder (line 67) for packaging
1 participant
Summary
Completely rebuilt the desktop app's build system using electron-vite best practices. Removed fragile environment loading, simplified configurations, and standardized import paths with
@/and~/aliases throughout the codebase.Changes
dist/directory (was innode_modules/.dev/)Verification
Both
bun devandbun run buildverified working correctly. Zero TODOs related to build system.Summary by CodeRabbit
Build System Updates
Documentation