Add Electron app setup and release workflow #403
Conversation
WalkthroughThis pull request introduces significant changes to support the development and release of a desktop application using Electron. It adds a new GitHub Actions workflow for automating the release process, a Changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (3)
native/src/main.ts (2)
18-24: Improve application lifecycle managementThe application lifecycle handling could be more robust. Consider adding proper cleanup and error handling.
Here's a suggested improvement:
+let mainWindow: BrowserWindow | null = null; + function createWindow() { const win = new BrowserWindow({...}); + mainWindow = win; + + win.on('closed', () => { + mainWindow = null; + }); // ... rest of createWindow function } -app.whenReady().then(createWindow); +app.whenReady().then(() => { + createWindow(); + + process.on('uncaughtException', (error) => { + console.error('Uncaught exception:', error); + }); +}); app.on("window-all-closed", () => { + // Perform any cleanup tasks here if (process.platform !== "darwin") { app.quit(); } });
26-30: Add window state persistenceConsider adding window state management to preserve the window's position and size between sessions.
I recommend using
electron-window-statepackage. Here's how to implement it:+import * as windowStateKeeper from 'electron-window-state'; function createWindow() { + const windowState = windowStateKeeper({ + defaultWidth: 800, + defaultHeight: 600 + }); + const win = new BrowserWindow({ - width: 800, - height: 600, + x: windowState.x, + y: windowState.y, + width: windowState.width, + height: windowState.height, // ... rest of options }); + + windowState.manage(win); }native/package.json (1)
27-62: Enhance build configuration for security and optimizationSeveral improvements are recommended for the build configuration:
- Consider optimizing the included files to reduce package size
- Add code signing configuration for macOS distribution
- Consider adding auto-update configuration
"build": { "publish": { "provider": "github" }, "appId": "com.example.codepair", "productName": "codepair", "files": [ "dist/**/*", - "node_modules/**/*", + "!node_modules/**/{CHANGELOG.md,README.md,README,readme.md,readme}", + "!node_modules/**/{test,__tests__,tests,powered-test,example,examples}", "package.json", "!node_modules/**/*.{ts,map}" ], "mac": { - "target": "dmg" + "target": "dmg", + "hardenedRuntime": true, + "gatekeeperAssess": false, + "entitlements": "build/entitlements.mac.plist", + "entitlementsInherit": "build/entitlements.mac.plist" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
.github/workflows/app-release.yml(1 hunks)native/package.json(1 hunks)native/src/main.ts(1 hunks)native/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- native/tsconfig.json
🔇 Additional comments (5)
native/src/main.ts (1)
11-15: Verify development and production URLs
Please ensure that:
- The development server is configured to run on port 5173
- The production URL is correct and accessible
.github/workflows/app-release.yml (3)
1-6: LGTM! Workflow trigger configuration is appropriate.
The workflow name and trigger configuration are well-defined for release automation.
43-53: Verify electron-builder configuration and release artifacts.
The workflow should verify the electron-builder configuration and ensure all platform-specific artifacts are included.
#!/bin/bash
# Description: Verify electron-builder configuration and build scripts
# Expected: Configuration exists with correct build targets
# Check electron-builder configuration
echo "Checking electron-builder configuration:"
rg -A 10 "build|electron-builder" native/package.json
# Check if all platform targets are configured
echo "Checking platform targets:"
rg -A 5 "mac|win|linux" native/electron-builder.*7-18: Consider using Node.js 20.x for longer-term support.
Node.js 18.x will reach EOL in April 2025. Consider upgrading to Node.js 20.x (LTS) for longer-term support.
- node-version: [18.x]
+ node-version: [20.x]Verify the working directory exists.
The workflow assumes the existence of a ./native directory.
✅ Verification successful
The ./native directory exists with proper electron-builder configuration
The directory exists and contains all necessary files including package.json with complete electron-builder configuration for all platforms (Windows, macOS, and Linux). The build configuration is properly set up in the package.json file.
Regarding Node.js version upgrade suggestion:
The package.json doesn't specify Node.js engine requirements, and the project is using modern dependencies including Electron v33 and TypeScript 5.3.3. Since these are compatible with Node.js 20.x, the suggestion to upgrade from Node.js 18.x to 20.x is valid and beneficial for longer-term support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the native directory and required files
# Expected: Directory exists with package.json and electron-builder configuration
# Check if native directory exists
if ! [ -d "native" ]; then
echo "Error: native directory not found"
exit 1
fi
# Check for essential files
echo "Checking for essential files in native directory:"
ls -la native/package.json native/electron-builder.* 2>/dev/null || echo "Warning: Missing essential configuration files"
Length of output: 1337
Script:
#!/bin/bash
# Let's verify if the native directory has the electron-builder configuration in package.json
rg -A 20 '"build":\s*{' native/package.json
# Also check the contents of the native directory to see if electron-builder config exists with different extensions
ls -la native/ | grep -iE 'electron-builder|builder'
# Check package.json for Node.js engine requirements
rg '"engines"' -A 3 native/package.json
Length of output: 230
Script:
#!/bin/bash
# Let's try with simpler patterns to check the configuration
echo "Checking native directory contents:"
ls -la native/
echo -e "\nChecking package.json content:"
cat native/package.json
echo -e "\nChecking for Node.js version requirements:"
rg '"node":' native/package.json
Length of output: 2191
native/package.json (1)
20-26: Review dependency versions for compatibility and security
The dependency versions need attention:
- [email protected] is outdated compared to the latest Electron version
- Consider using caret ranges for minor version updates
#!/bin/bash
# Check latest versions of key dependencies
pnpm view electron-builder version
pnpm view electron version "devDependencies": {
- "@types/node": "^22.7.6",
+ "@types/node": "^20.0.0",
"cross-env": "^7.0.3",
"electron": "^33.0.1",
- "electron-builder": "^25.1.8",
+ "electron-builder": "^24.0.0",
"typescript": "^5.3.3"
},| if (process.env.NODE_ENV === "development") { | ||
| win.loadURL("http://localhost:5173"); | ||
| } else { | ||
| win.loadURL("https://codepair.yorkie.dev"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for URL loading
The URL loading lacks error handling and validation. Also, consider adding security headers for the loaded content.
Consider this improved implementation:
if (process.env.NODE_ENV === "development") {
- win.loadURL("http://localhost:5173");
+ win.loadURL("http://localhost:5173").catch(err => {
+ console.error('Failed to load dev server:', err);
+ });
} else {
- win.loadURL("https://codepair.yorkie.dev");
+ win.loadURL("https://codepair.yorkie.dev", {
+ httpHeaders: {
+ 'Content-Security-Policy': "default-src 'self' https://codepair.yorkie.dev"
+ }
+ }).catch(err => {
+ console.error('Failed to load production URL:', err);
+ });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (process.env.NODE_ENV === "development") { | |
| win.loadURL("http://localhost:5173"); | |
| } else { | |
| win.loadURL("https://codepair.yorkie.dev"); | |
| } | |
| if (process.env.NODE_ENV === "development") { | |
| win.loadURL("http://localhost:5173").catch(err => { | |
| console.error('Failed to load dev server:', err); | |
| }); | |
| } else { | |
| win.loadURL("https://codepair.yorkie.dev", { | |
| httpHeaders: { | |
| 'Content-Security-Policy': "default-src 'self' https://codepair.yorkie.dev" | |
| } | |
| }).catch(err => { | |
| console.error('Failed to load production URL:', err); | |
| }); | |
| } |
| const win = new BrowserWindow({ | ||
| width: 800, | ||
| height: 600, | ||
| webPreferences: { | ||
| nodeIntegration: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Critical: Secure the BrowserWindow configuration
The current window configuration has security vulnerabilities. Node integration should be disabled by default, and a preload script should be used instead.
Apply these security-focused changes:
const win = new BrowserWindow({
width: 800,
height: 600,
webPreferences: {
- nodeIntegration: true,
+ nodeIntegration: false,
+ contextIsolation: true,
+ sandbox: true,
+ preload: path.join(__dirname, 'preload.js')
},
});Committable suggestion skipped: line range outside the PR's diff.
| - name: Install Dependencies | ||
| run: pnpm install --no-frozen-lockfile | ||
|
|
There was a problem hiding this comment.
Remove --no-frozen-lockfile flag for consistent builds.
Using --no-frozen-lockfile in CI/CD can lead to inconsistent builds across different environments as it allows package versions to be updated during installation.
- run: pnpm install --no-frozen-lockfile
+ run: pnpm install📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install Dependencies | |
| run: pnpm install --no-frozen-lockfile | |
| - name: Install Dependencies | |
| run: pnpm install | |
| - name: Build Electron App | ||
| run: pnpm run build | ||
|
|
||
| - name: Build and publish | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| pnpm run electron-builder --publish always | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and build verification.
The build steps should include error handling and verification of outputs.
- name: Build Electron App
- run: pnpm run build
+ run: |
+ pnpm run build
+ if [ $? -ne 0 ]; then
+ echo "Build failed"
+ exit 1
+ fi
+ # Verify build outputs
+ if [ ! -d "dist" ]; then
+ echo "Build directory not found"
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build Electron App | |
| run: pnpm run build | |
| - name: Build and publish | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| run: | | |
| pnpm run electron-builder --publish always | |
| - name: Build Electron App | |
| run: | | |
| pnpm run build | |
| if [ $? -ne 0 ]; then | |
| echo "Build failed" | |
| exit 1 | |
| fi | |
| # Verify build outputs | |
| if [ ! -d "dist" ]; then | |
| echo "Build directory not found" | |
| exit 1 | |
| fi | |
| - name: Build and publish | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| run: | | |
| pnpm run electron-builder --publish always |
| "scripts": { | ||
| "build": "tsc src/main.ts --outDir dist", | ||
| "start": "pnpm run build && cross-env NODE_ENV=production pnpx electron dist/main.js", | ||
| "start:dev": "pnpm run start:codepair:dev && pnpm run build && cross-env NODE_ENV=development pnpx electron dist/main.js", | ||
| "start:codepair:dev": "cd ../frontend && Pnpm run dev & ", | ||
| "electron-builder": "pnpm run build && electron-builder", | ||
| "release": "pnpm run build && electron-builder --publish always" | ||
| }, |
There was a problem hiding this comment.
Fix script configuration issues
There are several issues in the scripts configuration:
- The build script should use the project's tsconfig.json
- There's a typo in start:codepair:dev (Pnpm vs pnpm)
- The background process in start:codepair:dev might not clean up properly
"scripts": {
- "build": "tsc src/main.ts --outDir dist",
+ "build": "tsc -p tsconfig.json",
"start": "pnpm run build && cross-env NODE_ENV=production pnpx electron dist/main.js",
"start:dev": "pnpm run start:codepair:dev && pnpm run build && cross-env NODE_ENV=development pnpx electron dist/main.js",
- "start:codepair:dev": "cd ../frontend && Pnpm run dev & ",
+ "start:codepair:dev": "cd ../frontend && pnpm run dev",
"electron-builder": "pnpm run build && electron-builder",
"release": "pnpm run build && electron-builder --publish always"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "scripts": { | |
| "build": "tsc src/main.ts --outDir dist", | |
| "start": "pnpm run build && cross-env NODE_ENV=production pnpx electron dist/main.js", | |
| "start:dev": "pnpm run start:codepair:dev && pnpm run build && cross-env NODE_ENV=development pnpx electron dist/main.js", | |
| "start:codepair:dev": "cd ../frontend && Pnpm run dev & ", | |
| "electron-builder": "pnpm run build && electron-builder", | |
| "release": "pnpm run build && electron-builder --publish always" | |
| }, | |
| "scripts": { | |
| "build": "tsc -p tsconfig.json", | |
| "start": "pnpm run build && cross-env NODE_ENV=production pnpx electron dist/main.js", | |
| "start:dev": "pnpm run start:codepair:dev && pnpm run build && cross-env NODE_ENV=development pnpx electron dist/main.js", | |
| "start:codepair:dev": "cd ../frontend && pnpm run dev", | |
| "electron-builder": "pnpm run build && electron-builder", | |
| "release": "pnpm run build && electron-builder --publish always" | |
| }, |
| { | ||
| "name": "@codepair/native", | ||
| "version": "1.0.1", | ||
| "main": "dist/main.js", | ||
| "description": "Codepair Native", | ||
| "author": "yorkie-team", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/minai621/codepair" | ||
| }, | ||
| "license": "Apache-2.0", |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Repository URL needs to be updated to match the official repository
The repository URL in package.json should be updated to point to the official yorkie-team repository instead of the personal fork. Based on the git remote information, here's what needs to be changed:
- Update repository URL from
https://github.com/minai621/codepairtohttps://github.com/yorkie-team/codepair.git - Keep version as 1.0.1 since this might be intentional for the native package versioning strategy
🔗 Analysis chain
Review and update repository metadata
Several metadata fields need attention:
- The repository URL appears to point to a personal fork rather than the main repository
- Consider starting with version 1.0.0 for the initial release
{
"name": "@codepair/native",
- "version": "1.0.1",
+ "version": "1.0.0",
"main": "dist/main.js",
"description": "Codepair Native",
"author": "yorkie-team",
"repository": {
"type": "git",
- "url": "https://github.com/minai621/codepair"
+ "url": "<main-repository-url>"
},Let me try another script to get the repository information since the previous command didn't work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the correct repository URL
gh repo view --json url --jq .url
Length of output: 287
Script:
#!/bin/bash
# Try different approaches to get repository info
cat package.json | grep -A 5 "repository"
# Also check if there's a root package.json
cat native/package.json | grep -A 5 "repository"
# Check git remote
git remote -v
Length of output: 366
|
Thanks for the PR! It seems to be working well on my device. I’ll review it soon, though it may take some time since I’m not familiar with Electron. 
|
What this PR does / why we need it:
This PR sets up the Electron application build and release workflow. It introduces a GitHub Actions workflow that automates the building and publishing of the Electron app across macOS, Windows, and Linux platforms whenever a new tag matching v* is pushed. This streamlines the release process and ensures that users can access the latest versions of the desktop application directly from GitHub Releases.
Which issue(s) this PR fixes:
Fixes #285
Special notes for your reviewer:
The workflow uses electron-builder for packaging the app.
The package.json in the native directory has been updated with build scripts and configurations for electron-builder.
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
package.jsonfile to manage project metadata and scripts for building and starting the application.Bug Fixes