Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The dev shell points npm’s Electron package to the Nixpkgs launcher, so the reported GUI-startup failure is not present. No actionable merge risk remains. Security Architecture Review
Pre-merge checks |
|
The flake only defines Linux systems, so the Darwin branch never evaluated. It was also wrong with the download skipped: without path.txt the npm package resolves Applications/electron, which does not exist. nixpkgs ships bin/electron on Darwin too.
|
Thanks! Pushed a small follow-up: dropped the Darwin branch. The flake only builds Linux, and without path.txt it would have resolved Applications/electron. Verified in nix: npm electron now resolves to the wrapped bin/electron. |
Summary
The Nix dev shell pointed npm's
electronpackage at${electron}/libexec/electron, the bare Electron binary.vite-plugin-electronlaunched it without the nixpkgs wrapper's GIO/GTK environment, sonpm run devbuilt but the GUI failed to open on Linux.ELECTRON_OVERRIDE_DIST_PATHnow points at${electron}/bin, the wrapped launcher.ELECTRON_SKIP_BINARY_DOWNLOAD=1stops npm from downloading its own Electron in the shell.bin/electronon Darwin as well.Related issue
None.
Type of change
Release impact
Desktop impact
Screenshots / video
N/A, dev shell only.
Testing
nix flake check --no-build --all-systemspasses.ELECTRON_OVERRIDE_DIST_PATH, the npmelectronmodule resolves toelectron-43.4.1/bin/electron, the bash wrapper, with or withoutpath.txt. The old value resolved to the unwrapped ELF. The wrapper runs:--versionprintsv43.4.1.npm run devfromnix develop; the Electron main process started and registered the global shortcut.Summary by CodeRabbit