Repository navigation
perf(build): minify desktop JavaScript bundles without dropping crash context - #17527
Conversation
Greptile SummaryThe PR minifies Electron main and renderer bundles using Rolldown’s OXC minifier and generates hidden source maps for the main bundle.
Confidence Score: 5/5The PR appears safe to merge because no eligible blocking failure remains in the follow-up review scope. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| electron.vite.config.ts | Enables OXC minification for main and renderer output and hidden source-map generation for the main bundle. |
| config/electron-builder.config.cjs | Excludes generated source maps from the packaged application. |
| config/scripts/electron-vite-output-contract.test.ts | Adds contract assertions for OXC minification, hidden main source maps, and packaging exclusion. |
Reviews (3): Last reviewed commit: "perf(build): minify with rolldown's oxc ..." | Re-trigger Greptile
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe Electron Vite main and renderer builds now use Rolldown’s Merge Risk: 🔵 Low · up to Release builds now rely on hidden source maps kept outside the installed application; if the matching maps are not archived and retained, crash reports may be harder to interpret at source level. The change is mergeable with explicit owner follow-up on release-symbol storage and retention. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the goal and reports validation, but it describes the implementation as esbuild minification with keepNames, which conflicts with the actual Rolldown oxc minifier, hidden source maps, and removed keepNames settings. It also omits or does not complete several required template sections, including Linked Issue, Visual Proof/N/A, Testing, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist. Resolution Update the description to match the implemented oxc minifier, hidden main source maps, removed keepNames settings, and packaged-app source-map exclusion. Add the required template sections, including an issue reference after “Fixes #”, Visual Proof with “N/A” and a reason, Testing details and checklist status, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist.
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 |
Readiness checklist review — PASSReviewed all 8 sections. No proven P0/P1/P2 findings.
ELI5: code gets smaller, but the names and evidence that explain a crash stay attached. |
|
Debugability clarification: minification does not preserve original source line numbers. It does preserve the crash-report fields ( |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Enable esbuild minification for the Electron main and renderer bundles —
electron.vite.config.tssetsbuild.minify: 'esbuild'plusesbuild.keepNames: trueon both themainandrendererconfigs, turning on minification that electron-vite's presets otherwise default tofalse. - Add output-contract assertions — a new
config/scripts/electron-vite-output-contract.test.tscase pins the minify/keepNames settings for both environments.
Verified the mechanics independently: electron-vite 5.0.0 defaults build.minify to false for main/preload/renderer, so this is the change that actually enables minification. Vite 7.3.1 (rolldown-vite) honors esbuild.keepNames when build.minify === 'esbuild' — its vite:esbuild-transpile plugin spreads config.esbuild into the esbuild transform options — and a direct esbuild check confirmed that arrow-function components, function declarations, and classes all retain their .name under minify: true, keepNames: true. The bootstrap banners are prepended in generateBundle (after minify), so they ship unminified, and the require("electron") scan in plain-node-entry-guard still matches esbuild's minified output since string literals are preserved.
The observability tradeoff in the PR body is accurate: function/class names survive via keepNames, but minified error_stack frames lose source-level line/column fidelity since release bundles carry no source maps and there is no symbolication pipeline in src/main. That is an accepted, documented cost of this change, not a dropped crash field.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
CI status noteAll eight test shards, typecheck, package, package (Windows), macOS/Linux/Windows native smoke, and both automated code reviews pass. The remaining static-analysis failure is the unchanged baseline |
'esbuild' made rolldown disable its own minifier and re-print every chunk through esbuild, which is not a declared dependency and resolves only via pnpm's shamefullyHoist from electron-vite's tree (0.25.12 against a declared peer of ^0.27.0). Switching to rolldown's in-process 'oxc' minifier drops that second pass: main+renderer build falls 23.2s -> 11.9s and ships ~2.7MB less JavaScript. keepNames is dropped with it — it cost ~1.5MB and only recovered function names. main now builds with sourcemap:'hidden', which restores names *and* locations without emitting a sourceMappingURL. Packaging excludes out/**/*.map so app.asar is unaffected; release CI publishes the maps.
There was a problem hiding this comment.
Important
The switch from esbuild to minify: 'oxc' silently removed the keepNames guarantee this PR exists to preserve — see the inline comment. Rolldown's default minifier mangles function/class names, so crash context regresses to exactly the single-letter frames the PR title says it avoids.
Reviewed changes
- Switched the desktop minifier to rolldown's in-process oxc —
main.build.minifyandrenderer.build.minifyare now'oxc', dropping the prioresbuild.keepNames: trueoverrides (the rationale comment explains why the undeclared esbuild pass was undesirable). - Emitted hidden main-process source maps —
main.build.sourcemap: 'hidden'emits.js.mapwithout asourceMappingURL, and!out/**/*.mapinelectron-builder.config.cjsstrips them fromapp.asar. - Updated the output-contract test — the keepNames assertion was replaced with
minify: 'oxc'+esbuild-undefined checks and a hidden-sourcemap packaging check that loadselectron-builder.config.cjsviacreateRequire.
ℹ️ Nitpicks
- The comment at
electron.vite.config.ts:203says "Release CI uploads them so minified crash traces stay decodable," but this PR adds no upload step — the maps are emitted and then stripped with no consumer yet. Consider future-tense wording to match the PR body's "future release-symbol archive" framing.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
| // Why: 'esbuild' makes rolldown disable its own minifier and re-print every | ||
| // chunk through esbuild, which is undeclared here and only resolves via | ||
| // pnpm hoisting. 'oxc' is rolldown's in-process minifier. | ||
| minify: 'oxc', |
There was a problem hiding this comment.
minify: 'oxc' drops the keepNames guarantee this PR is titled around. Rolldown-vite maps minify: 'oxc' to rolldown's minify: true for non-lib builds, and that minifier mangles function/class names by default — const MyComponent = () => null becomes const e = () => null, so React componentStack and Error.stack will show single letters. The renderer config at line 309 is identical.
Technical details
# Restore function/class names under the oxc minifier
## Affected sites
- electron.vite.config.ts:200 — `main.build.minify: 'oxc'` with no `keepNames` (the `esbuild.keepNames: true` from the prior commit was removed).
- electron.vite.config.ts:309 — `renderer.build.minify: 'oxc'`, same regression.
- config/scripts/electron-vite-output-contract.test.ts:83 — the assertion now pins `esbuild` being `undefined`, so nothing guards name preservation anymore.
## Evidence
- rolldown-vite@7.3.1 `dist/node/chunks/node.js:32742`: `build.minify === 'oxc'` → `output.minify: true` for non-library builds (both main's `cjs` and renderer's `es` format).
- Empirical rolldown run: `minify: true` emits `const e=()=>null;function t(){return 42}var n=class{...}` (names mangled); `minify: true` + `output.keepNames: true` preserves them via a `__name` helper.
- Passing `minify: { compress: true, mangle: { keepNames: true } }` through `build.minify` is NOT an option here: rolldown-vite only honors `'oxc'`, `false`, `'esbuild'`, or `'terser'` at `build.minify`; any object falls through to `false` (no minification).
## Required outcome
- Shipped bundles keep function/class names readable in stack traces (parity with the `esbuild.keepNames: true` behavior the prior commit had), while remaining minified.
## Suggested approach (optional)
- Set `rollupOptions.output.keepNames: true` on both `main` and `renderer` outputs (rolldown maps `esbuildOptions.keepNames` to `output.keepNames`, so this is the direct oxc-era equivalent), and add a contract-test assertion that pins it.
## Open questions for the human (optional)
- Is mangling short-lived locals acceptable as long as declared function/class names survive, or is full name preservation required?
The macOS, Windows, and both Linux release builds all failed on "Verify
telemetry constants present in app.asar", blocking publish-release:
::error::BUILD_IDENTITY constant missing or unexpected value in
dist/mac-arm64/Orca.app/Contents/Resources/app.asar
The verifier printed no context sample, because the string
`BUILD_IDENTITY` does not occur anywhere in the shipped bundle at all.
#17527 added `minify: 'oxc'` to the main bundle, so oxc renames the
module-local `const BUILD_IDENTITY` to a short identifier.
`BUILD_IDENTITY_RE` keys off the literal name and therefore cannot match a
production bundle. `MINIFIED_TELEMETRY_RE` matches the adjacent injected
identity/key pair instead, which survives renaming.
#11019 created these patterns without that fallback, and its own comment
predicted this exact failure if minification were ever enabled on the main
bundle. The fallback was written on the v1.4.197 release branch in
35f1b0e and never merged back to main, so main has never been able to
verify a minified bundle.
Verified against a real local bundle built with the release env vars
(ORCA_BUILD_IDENTITY=stable, ORCA_POSTHOG_WRITE_KEY=phc_...): the bundle
does not contain "BUILD_IDENTITY"; both the narrowed and the widened
BUILD_IDENTITY_RE fail; MINIFIED_TELEMETRY_RE matches and recovers
identity=stable plus the write key.
`verify-telemetry-constants.mjs` keys off the literal identifiers `BUILD_IDENTITY` and `WRITE_KEY`. #17527 turned on `minify: 'oxc'` for the main bundle, so oxc renames those module-locals and neither name survives into the shipped bundle — the gate can no longer pass on any platform. Port the release-branch fix (35f1b0e, shipped in v1.4.197 and never merged back): `MINIFIED_TELEMETRY_RE` matches the adjacent injected identity/key pair instead, and the verify falls back to it when the named declarations are absent. The quote class also accepts backticks, which is what oxc emits. Proven against a local release-env build of out/main: literal "BUILD_IDENTITY" absent, named regexes miss, MINIFIED_TELEMETRY_RE matches `var xde=`stable`,Sde=`phc_TestKey_abc123`` and recovers both values. Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>
…nheriting them (#19945) * fix(release): prune optional natives before the linux arch floor check The linux-arm64 release build failed packaging, and retried three times: [verify-linux-glibc-floor] 1 bundled native binary is built for the wrong architecture (target arm64): resources/node_modules/@parcel/watcher-linux-x64-glibc/watcher.node is x64, expected arm64 (from its own path) `afterPack` ran `verifyLinuxGlibcFloor` at its top, before `prunePackagedRuntimeNodeModules`. A cross-build intentionally installs every optional native variant, so at that point the arm64 slice still carries the x64 `@parcel/watcher` package that the prune exists to drop. The check was reading a file that was never going to ship. Move the floor check below the prune so it inspects the binaries actually packed. Same ordering as 35f1b0e on the v1.4.197 release branch, which shipped green and was never merged back to main. (cherry picked from commit 6400598) (cherry picked from commit a883e17) * fix(release): restore the minified telemetry constant fallback The macOS, Windows, and both Linux release builds all failed on "Verify telemetry constants present in app.asar", blocking publish-release: ::error::BUILD_IDENTITY constant missing or unexpected value in dist/mac-arm64/Orca.app/Contents/Resources/app.asar The verifier printed no context sample, because the string `BUILD_IDENTITY` does not occur anywhere in the shipped bundle at all. #17527 added `minify: 'oxc'` to the main bundle, so oxc renames the module-local `const BUILD_IDENTITY` to a short identifier. `BUILD_IDENTITY_RE` keys off the literal name and therefore cannot match a production bundle. `MINIFIED_TELEMETRY_RE` matches the adjacent injected identity/key pair instead, which survives renaming. #11019 created these patterns without that fallback, and its own comment predicted this exact failure if minification were ever enabled on the main bundle. The fallback was written on the v1.4.197 release branch in 35f1b0e and never merged back to main, so main has never been able to verify a minified bundle. Verified against a real local bundle built with the release env vars (ORCA_BUILD_IDENTITY=stable, ORCA_POSTHOG_WRITE_KEY=phc_...): the bundle does not contain "BUILD_IDENTITY"; both the narrowed and the widened BUILD_IDENTITY_RE fail; MINIFIED_TELEMETRY_RE matches and recovers identity=stable plus the write key. (cherry picked from commit 39c3e44) (cherry picked from commit d0bbe44) * fix(skills): stop asserting readdir order in the skill root walk test The Windows skill-sharing release gate — which blocks publish-release — failed on this test, wedging the 1.4.198 cut. The implementation is correct; the assertion was not. `findSkillFiles` pushes results in `readdir` order and dedupes visited directories by realpath, so of the 32 junctions pointing at one target exactly one survives. The assertion hardcoded both the array order and which link won. `readdir` order is filesystem-dependent: APFS and ext4 return SKILL.md first, while NTFS enumerates its filename index alphabetically on the uppercased name, where "LINK00" sorts before "SKILL.MD" (L=0x4C < S=0x53). Windows therefore returned the same two paths in the opposite order. Assert the contract instead: the real file is present, exactly one link-routed path survives dedup, and nothing else does. (cherry picked from commit 9a832e9) (cherry picked from commit 505b85c) * test(release): pin ported release gate fixes * style(skills): apply pinned test formatting --------- Co-authored-by: Neil <neil@stably.ai> Co-authored-by: Merge Sim <sim@local>
The macOS, Windows, and both Linux release builds all failed on "Verify
telemetry constants present in app.asar", blocking publish-release:
::error::BUILD_IDENTITY constant missing or unexpected value in
dist/mac-arm64/Orca.app/Contents/Resources/app.asar
The verifier printed no context sample, because the string
`BUILD_IDENTITY` does not occur anywhere in the shipped bundle at all.
stablyai#17527 added `minify: 'oxc'` to the main bundle, so oxc renames the
module-local `const BUILD_IDENTITY` to a short identifier.
`BUILD_IDENTITY_RE` keys off the literal name and therefore cannot match a
production bundle. `MINIFIED_TELEMETRY_RE` matches the adjacent injected
identity/key pair instead, which survives renaming.
stablyai#11019 created these patterns without that fallback, and its own comment
predicted this exact failure if minification were ever enabled on the main
bundle. The fallback was written on the v1.4.197 release branch in
35f1b0e and never merged back to main, so main has never been able to
verify a minified bundle.
Verified against a real local bundle built with the release env vars
(ORCA_BUILD_IDENTITY=stable, ORCA_POSTHOG_WRITE_KEY=phc_...): the bundle
does not contain "BUILD_IDENTITY"; both the narrowed and the widened
BUILD_IDENTITY_RE fail; MINIFIED_TELEMETRY_RE matches and recovers
identity=stable plus the write key.
(cherry picked from commit 39c3e44)
(cherry picked from commit d0bbe44)
… context (stablyai#17527) * perf(build): minify desktop JavaScript bundles * perf(build): minify with rolldown's oxc and emit hidden main source maps 'esbuild' made rolldown disable its own minifier and re-print every chunk through esbuild, which is not a declared dependency and resolves only via pnpm's shamefullyHoist from electron-vite's tree (0.25.12 against a declared peer of ^0.27.0). Switching to rolldown's in-process 'oxc' minifier drops that second pass: main+renderer build falls 23.2s -> 11.9s and ships ~2.7MB less JavaScript. keepNames is dropped with it — it cost ~1.5MB and only recovered function names. main now builds with sourcemap:'hidden', which restores names *and* locations without emitting a sourceMappingURL. Packaging excludes out/**/*.map so app.asar is unaffected; release CI publishes the maps.
What this changes
Use esbuild minification for the Electron main and renderer bundles, while keeping function/class names (
keepNames: true). This reduces shipped JavaScript without changing the crash-report fields or the bootstrap diagnostics path.ELI5
The build currently ships JavaScript with extra spaces and repeated spelling. The minifier packs that code down.
keepNamesleaves the useful labels on errors, so crash reports still say which function failed instead of only showing anonymous letters.Measurement (macOS arm64, unpacked
dist/mac-arm64/Orca.app)app.asarInstaller compression is not included in these numbers.
Crash-report/debugability contract
Error.stackand ReactcomponentStackare still captured and persisted unchanged by the existing crash-report path.keepNames: trueis asserted byconfig/scripts/electron-vite-output-contract.test.ts.Readiness checklist
Reviewed against all eight sections:
Verdict: PASS. No proven P0/P1/P2 findings. Residual risk is limited to unsymbolicated minified source locations; function names and all existing crash evidence remain available.
Validation
pnpm exec vitest run config/scripts/electron-vite-output-contract.test.ts(and focused packaging suite): 55/55 tests passed.pnpm run tc:nodepassed.pnpm run build:unpackpassed.