fix: ensure npm run dev works after npm install on Node v24+#4212
Merged
Conversation
Move native-keymap to optionalDependencies so npm ignores its auto-gyp failure on Node v24+, then restore and rebuild it correctly via a dedicated postinstall script (scripts/postinstall.js). The script also writes path.txt for the Electron binary when install.js skips it due to a cache hit, which was causing "Error: Electron uninstall" on npm run dev. Mark native-keymap as externalized in electron-vite so Rollup does not try to bundle an optional native addon. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates install-time handling for native-keymap so development setup can recover from Node v24+ native build failures and still prepare Electron/native modules for npm run dev.
Changes:
- Moves
native-keymaptooptionalDependencies. - Adds a cross-platform
scripts/postinstall.jsto restore, patch, rebuild, download Electron, and minify locales. - Externalizes
native-keymapin the Electron main-process Vite build.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
scripts/postinstall.js |
Adds scripted postinstall flow for native-keymap restoration, Electron setup, rebuild, and locale minification. |
package.json |
Replaces inline postinstall command and moves native-keymap to optional dependencies. |
package-lock.json |
Reflects native-keymap as an optional dependency. |
electron.vite.config.js |
Ensures native-keymap is externalized during main-process bundling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yauzl v2.10.0 openReadStream callback never fires for compressed entries on Node v26+, causing extract-zip to silently exit with an incomplete dist/ (Frameworks/ entirely missing). Detect this by checking whether Electron.app/Contents/Frameworks exists after install.js runs; if it doesn't, find the cached zip in ~/Library/Caches/electron and re-extract with system unzip, which handles all entries correctly. Also expand isComplete() to require Frameworks/ on macOS, so a partial dist from a previous run is cleared and re-extracted on the next npm install. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reformat nested ternary to multiline (@stylistic/multiline-ternary, @stylistic/operator-linebreak, @stylistic/indent) and replace template literal strings with single-quoted strings (@stylistic/quotes). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
native-keymapfromdependenciestooptionalDependenciesso npm ignores its auto-gyp compile failure on Node v24+ (C++20 not declared) and continues the installscripts/postinstall.jsreplacing the fragile inline postinstall command — it restores native-keymap source (npm removes the dir on optional failure), downloads Electron, applies the C++20 patch, rebuilds native modules for Electron's ABI, and minifies localesnode_modules/electron/path.txtas a fallback whenelectron/install.jsskips it on a cache hit — fixes the "Error: Electron uninstall" crash onnpm run devinclude: ['native-keymap']toexternalizeDepsinelectron.vite.config.jsso Rollup externalizes the native addon instead of trying to bundle it (optional deps aren't auto-externalized)Test plan
npm installcompletes without error on Node v24/v26npm run devlaunches the Electron appnpm run buildproduces a distributable without errorsnpm install(no changes) is idempotent — no spurious tree mutations🤖 Generated with Claude Code