refactor(code): migrate from electron forge to electron-builder#2795
refactor(code): migrate from electron forge to electron-builder#2795charlesvien wants to merge 17 commits into
Conversation
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Prompt To Fix All With AIFix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
apps/code/scripts/before-pack.cjs:66-86
**Silent failure for required arch-specific watcher package**
`copyDep` only emits `console.warn` when a package is missing instead of throwing. For the arch-specific `@parcel/watcher-*` packages this is dangerous: the old Forge `packageAfterCopy` hook threw a hard error on the same condition. The CI "Verify package" step only checks for `@parcel/watcher` (the base package), not the arch-specific variant, so a missing `@parcel/watcher-darwin-arm64` (or the Linux/Windows equivalents) silently passes verification. Later in this same hook the fallback `@parcel/watcher/build` directory is removed, leaving no native binary at all — the packaged app would fail at runtime with no prior signal.
### Issue 2 of 4
apps/code/scripts/dev.mjs:64-118
**`waitForLine` is defined but never called**
This function is not invoked anywhere in `dev.mjs` or the rest of the codebase. Its line-buffering logic is redundant with the `forwardAndCheck` helper that is actually used. Keeping it around violates the "no superfluous parts" simplicity rule and could confuse readers into thinking it is wired up.
### Issue 3 of 4
apps/code/scripts/dev.mjs:120-145
**Renderer server crash is not handled**
`rendererServer` has no `close` event listener. If the Vite dev server exits unexpectedly after Electron has already started, the Electron window will point to a dead URL with no feedback. The pattern used for the Electron process (`electron.on('close', () => killAll(...))`) should be mirrored here to propagate the crash back to the parent and clean up all child processes.
### Issue 4 of 4
apps/code/scripts/merge-mac-manifests.mjs:27
**Merged manifest inherits arm64 top-level `path`/`sha512`/`size`**
`{ ...arm64, files: mergedFiles }` copies the arm64 `path`, `sha512`, and `size` fields into the merged `latest-mac.yml`. `electron-updater` 6.x selects the correct arch entry from the `files` array, so in practice this works — but any tool or older client that falls back to the top-level `path` would receive an arm64 artifact on an x64 machine, and the SHA512 verification would fail. The safer approach is to either drop those top-level fields or derive a neutral value (e.g. `version`, `releaseDate` only) that does not point to a single arch artifact.
Reviews (1): Last reviewed commit: "remove electron forge and update release..." | Re-trigger Greptile |
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/code/scripts/dev.mjs:140-155
**Watch build crashes silently abandoned**
`startWatchBuild` pushes each child into `children` but never attaches a `close` handler. If any of the three watch processes (main, preload, ws) exits unexpectedly after Electron has already started, `killAll` is never called, the Electron window keeps running against stale/missing output, and there is no feedback or cleanup. The `rendererServer` and `electron` processes both have proper `close` handlers for exactly this reason — the same guard should apply to the watch builds.
Reviews (2): Last reviewed commit: "Update vite.renderer.config.mts" | Re-trigger Greptile |
ab18bbc to
3efbf3f
Compare
There was a problem hiding this comment.
Gates denied this PR: it touches CI/CD workflows, build toolchain dependencies (migrating from electron-forge to electron-builder), and crypto/signing secrets handling — all on the deny-list. The PR is also well above size limits at 4416 lines across 30 files.
There was a problem hiding this comment.
Gates denied this PR: it touches CI/CD workflows, build toolchain dependencies (migrating from electron-forge to electron-builder), and crypto/signing secrets handling — all on the deny-list. The PR is also well above size limits at 4427 lines across 30 files.
There was a problem hiding this comment.
Gates denied this PR: it touches CI/CD workflows, build toolchain dependencies (full migration from electron-forge to electron-builder), and crypto/signing secrets handling — all on the deny-list. The PR also far exceeds size limits at 4427 lines across 30 files.

Problem
Auto-update runs on Electron's native
autoUpdater(Squirrel via update.electronjs.org) and has been a recurring source of failures we have repeatedly tried and failed to fix:It also gives us no control over the update UX: we cannot let users review and confirm an update before it downloads, show byte-level download progress, or surface "what's new" changelogs.
This migrates
apps/codefrom Electron Forge to electron-builder + electron-updater, the more stable, production-standard toolchain. It fixes the reliability issues and gives us the primitives (explicit download/install, progress events, release manifests) to build that update UX.Changes
Forge 7 to electron-builder 26, landed as small commits:
scripts/build.mjsandscripts/dev.mjs(keeps the CDP:9222debug port).electron-builder.config.cjs(mac dmg+zip, win NSIS+Squirrel, linux AppImage/deb/rpm). Abefore-pack.cjshook stages native modules from the hoisted rootnode_modules;filesdrops the deps Vite already inlines (bundle 1.7 GB to 669 MB).autoUpdatertoelectron-updater, which readslatest*.ymlfrom the GitHub release and exposes progress plus explicit download/install.code-release.ymlbuilds, signs and publishes via electron-builder (CSC_LINKsigning, per-arch macOS manifest merge, dual Windows NSIS+Squirrel for the transition).docs/UPDATES.md.How did you test this?
Locally on macOS arm64, I ran:
node scripts/build.mjsandelectron-builder build --mac --arm64(dmg + zip +latest-mac.yml).asar listconfirms native JS is in the manifest with.nodeunpacked, the bundle is 669 MB, and artifact filenames match the manifest urls.pnpm typecheck,pnpm lint, the updates unit tests (vitest run updates, 69 passing) and core's full suite (171 files). The PR's CIunit-testandbuildjobs pass.Not validated locally (needs a tagged CI run because these cannot build on macOS): real signing/notarization, the Windows and Linux builds, the finalize manifest merge plus draft promotion, and end-to-end auto-update. Recommend a staging tag (for example
v0.0.0-eb-test.1) before merge.Migration strategy
Per-platform rollout:
isSupported()is byte-identical tomain). electron-updater unlocks AppImage auto-update as a later, separate change.Windows dual installer (temporary):
win.target: ["nsis", "squirrel"]produces the NSIS installer (*.exe+latest.yml, consumed by electron-updater) and the Squirrel.Windows artifacts (*.nupkg+RELEASES+ Setup, underout/squirrel-windows/). CI uploads both to the same GitHub release.squirrelWindows.name: "PostHogCode"keeps the package identity identical to the old Forge MakerSquirrel build, soupdate.electronjs.orgkeeps serving the existing Squirrel installs."squirrel"fromwin.targetand delete thesquirrelWindowsblock inelectron-builder.config.cjs, leaving NSIS only.No UI changes in this PR; the confirm/progress/changelog UX is a follow-up on top of these primitives.
Automatic notifications