Isolate the Electron install from the application's npm config - #150
Open
Ikromjon1998 wants to merge 1 commit into
Open
Isolate the Electron install from the application's npm config#150Ikromjon1998 wants to merge 1 commit into
Ikromjon1998 wants to merge 1 commit into
Conversation
npm reads .npmrc files upward from the working directory, so the host application's configuration governs the install performed in resources/electron. Laravel's default skeleton has shipped ignore-scripts=true since Laravel 12, which skips Electron's postinstall and leaves node_modules/electron without a runtime binary. The install exits 0 and creates the directory, so nothing surfaces the problem until the app is launched and fails on a missing Electron.app. A directory-local .npmrc takes precedence over ancestors, which keeps this decision with NativePHP rather than inheriting whatever the application sets. Install scripts are not re-enabled wholesale. Laravel adopted ignore-scripts to reduce exposure to install-time code execution, and that protection is kept here by allowing only the two packages in this tree that need a script — electron, which downloads the runtime, and esbuild, which fetches its platform binary. Everything else in the dependency graph remains unable to run code at install time, including packages added to it later. electron-chromedriver and electron-winstaller are deliberately excluded: the first is only used for driver-based testing, and the second matters to Squirrel targets while this config builds nsis. allow-scripts requires npm 11. On older npm the key is ignored and only ignore-scripts applies, which is the behaviour this directory had before Laravel began shipping an .npmrc. Closes NativePHP#145
simonhamp
approved these changes
Sep 3, 2026
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
3 participants
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.
Closes #145. Takes option 3 as suggested, with the safeguard your comment asked for.
The change
One new file,
resources/electron/.npmrc:ignore-scripts=false allow-scripts[]=electron allow-scripts[]=esbuildA directory-local
.npmrctakes precedence over ancestors, so the install here no longer inherits the host application's policy.On reopening the door
You were right to flag this, so the change deliberately does not set
ignore-scripts=falsealone.allow-scripts(npm 11) restricts install scripts to named packages. Only two in this tree need one:electronpostinstallesbuildpostinstallelectron-chromedriverandelectron-winstalleralso carry install scripts and are deliberately left out: the first is only for driver-based testing, and the second matters to Squirrel targets whileelectron-builder.mjsbuildsnsis. They stay blocked, and npm reports them as skipped rather than failing the install.The net effect is narrower than what this directory had before Laravel began shipping an
.npmrc, when every package here could run install-time code. A dependency that starts shipping an install script later is refused by default rather than silently trusted.I did not use
strict-allow-scripts, because it makes any unapproved script a hard install failure — withelectron-chromedriverpresent that fails the install outright.Verified
Reproduced the original bug and the fix with a parent
.npmrccontainingignore-scripts=true, matching the Laravel 12 skeleton:electron's postinstall is skipped,node_modules/electron/disthas no runtime andpath.txtis absent, install exits 0.electron-chromedriverremains skipped with a warning and the install still succeeds.One caveat, and a separate problem
allow-scriptsrequires npm 11. On older npm the key is ignored and onlyignore-scriptsapplies, i.e. the pre-Laravel-12 behaviour — a graceful degradation rather than a break.Worth flagging separately: with this fix in place the postinstall runs, but on a clean install the runtime still does not land, because
extract-zipresolves yauzl 2.10.0, which stops after the first entry of the Electron archive whileinstall.jsexits 0 — leaving adist/holding onlyLICENSES.chromium.html. It reproduces outside NativePHP with nothing butnpm i electron, andoverrides: { "yauzl": "^3.4.0" }fixes it. I have left that out of this PR since it would mean regeneratingresources/electron/package-lock.json. Happy to open it as its own issue or PR — just say which you prefer.