build: pin electron exactly, so packaging works without a second declaration - #605
Conversation
…aration
`npm run build:win` failed at electron-builder in any checkout without its own
`node_modules/electron` — a git worktree, typically, where Node's resolution
walks up and finds the parent's copy but electron-builder looks only at the
project's:
Electron version "^41.2.1" is a range, not a fixed version.
electron-builder needs an exact version because it downloads the binaries for
one release. Given the range and no local install, it refuses.
Pinned rather than answered with `electronVersion` in electron-builder.json5,
because that would make the version two facts that have to agree and nothing
would notice when they stopped. The dependency declaration is the one place
the version is written; electron-builder reads it there.
The caret was already inert where it mattered: the lockfile pins 41.2.1 and
every CI job installs through .github/actions/setup, which runs `npm ci`. It
only ever applied to a bare `npm install` — and an unattended Electron bump is
what we want least here, since the packaging config carries facts held against
one release by hand (`minimumSystemVersion` against Electron 41's own
`LSMinimumSystemVersion`) and AGENTS.md records behaviour observed on 41.2.1
exactly. build-and-packaging.md already claimed the version was exact; now it
is.
A comment in electron-builder.json5 says why there is no `electronVersion`
there and names the error, so the next person to hit it adds `npm ci` rather
than the second source of truth.
Regenerating the lockfile also dropped `@mediapipe/selfie_segmentation`, which
left package.json in 6e5ffa9 when segmentation moved into the compositor and
was never regenerated out of the lock. Nothing imports it; `npm ci` was
downloading it in every job.
Verified: `npm run build:win`, unmodified, in a worktree with no
`node_modules/electron` and no override flag — installer produced.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe Electron development dependency is pinned to version ChangesElectron version resolution
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Electron packaging now consistently resolves version 41.2.1, with documentation explaining the worktree installation requirement. The manifest and lockfile are aligned, so the change is ready to merge. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides detailed context, rationale, impact, and verification results. It does not follow the required template and omits the required Summary, Related issue, Type of change, Release impact, Desktop impact, Screenshots / video, and Testing sections. Resolution Restructure the description using the repository template. Add the required headings and complete the relevant checkboxes, issue reference, testing details, release impact, and desktop or installer impact. State that screenshots are not applicable if no visual changes exist.
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
1 participant
The bug
npm run build:winfails at electron-builder in any checkout without its ownnode_modules/electron— a git worktree, typically, where Node's resolution walks up and finds the parent's copy but electron-builder looks only at the project's:Every prior step succeeds — native helpers, ffmpeg, onnxruntime, the compositor addon,
tsc,vite build. Only packaging fails.Why pin rather than set
electronVersionSetting it in
electron-builder.json5would make the version two facts that have to agree, and nothing would notice when they stopped. The dependency declaration is the one place the version is written; electron-builder reads it there when it isn't a range.The caret was already inert where it mattered:
41.2.1, and every CI job installs through.github/actions/setup, which runsnpm ci.It only ever applied to a bare
npm install, and an unattended Electron bump is what we want least here: the packaging config holds facts against one release by hand (minimumSystemVersionagainst Electron 41's ownLSMinimumSystemVersion), andAGENTS.mdrecords behaviour observed on 41.2.1 exactly.build-and-packaging.md:269already claimed the version was exact in package.json — now it is.A comment in
electron-builder.json5says why there is noelectronVersionthere and names the error, so the next person to hit it reaches fornpm cirather than the second source of truth.One unrelated line in the lockfile
Regenerating it also dropped
@mediapipe/selfie_segmentation. That left package.json in 6e5ffa9, when segmentation moved into the compositor, and was never regenerated out of the lock. Nothing imports it;npm ciwas downloading it in every job. Kept rather than hand-reverted — putting known-dead weight back to keep a diff tidy is the wrong trade.Verification
npm run build:win, unmodified, in a worktree with nonode_modules/electronand no override flag. Exit 0, installer produced.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores