Fix Windows dev startup when ELECTRON_RUN_AS_NODE is set#8445
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 4/5
- In
desktop/windows/package.json(viascripts/dev.mjs), thedevwrapper drops CLI flags, sonpm run dev -- <flags>won’t pass options through toelectron-vite dev, which can mislead local debugging or platform-specific testing; before merging, update the wrapper to appendprocess.argv(or equivalent) when spawning the dev command.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="desktop/windows/package.json">
<violation number="1" location="desktop/windows/package.json:15">
P2: The `dev` script wrapper does not forward CLI arguments to `electron-vite dev`. Any flags passed via `npm run dev -- <flags>` will be silently ignored because `scripts/dev.mjs` hard-codes `['dev']` instead of appending `process.argv.slice(2)`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "typecheck": "npm run typecheck:node && npm run typecheck:web", | ||
| "start": "electron-vite preview", | ||
| "dev": "electron-vite dev", | ||
| "dev": "node scripts/dev.mjs", |
There was a problem hiding this comment.
P2: The dev script wrapper does not forward CLI arguments to electron-vite dev. Any flags passed via npm run dev -- <flags> will be silently ignored because scripts/dev.mjs hard-codes ['dev'] instead of appending process.argv.slice(2).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/package.json, line 15:
<comment>The `dev` script wrapper does not forward CLI arguments to `electron-vite dev`. Any flags passed via `npm run dev -- <flags>` will be silently ignored because `scripts/dev.mjs` hard-codes `['dev']` instead of appending `process.argv.slice(2)`.</comment>
<file context>
@@ -12,7 +12,7 @@
"typecheck": "npm run typecheck:node && npm run typecheck:web",
"start": "electron-vite preview",
- "dev": "electron-vite dev",
+ "dev": "node scripts/dev.mjs",
"build": "npm run typecheck && electron-vite build",
"rebuild:sqlite": "electron-rebuild -f -w better-sqlite3",
</file context>
Git-on-my-level
added
workflow-review
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the focused Windows dev-startup fix — the underlying problem makes sense, and unsetting ELECTRON_RUN_AS_NODE only for the spawned Electron app is the right direction.
I think this needs one small robustness pass before merge:
- The wrapper spawns the resolved
node_modules/.bin/electron-vitepath withshell: true. That makes the command depend on shell parsing, so checkouts whose path contains spaces can fail, and forwarded dev flags can be interpreted by the shell instead of being passed literally toelectron-vite. Sincenpm run devis a local developer entrypoint, it should preserve the old script's argument behavior as closely as possible.
Could you update the wrapper to avoid shell parsing on platforms where it is not needed, and keep arguments passed literally? For example, spawn the binary from node_modules/.bin with shell: process.platform === 'win32' only if needed for the .cmd shim, or launch via a Node/npm entrypoint with cwd: root, and add/mention a quick verification for:
- a repo path containing spaces, and
npm run dev -- <some electron-vite flag>still reachingelectron-vite dev.
After that, this looks like a good targeted fix. I’m not formally approving because this changes the Windows dev workflow/package script and should get maintainer review before merge.
|
Updated in The wrapper no longer spawns the spawn(process.execPath, [electronVite, ...args], {
cwd: root,
env,
shell: false,
stdio: 'inherit'
})This avoids shell parsing entirely while preserving forwarded args from Verified:
|
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="desktop/windows/scripts/dev.mjs">
<violation number="1" location="desktop/windows/scripts/dev.mjs:10">
P2: Hardcoding the `electron-vite` internal path (`node_modules/electron-vite/bin/electron-vite.js`) instead of using the package-manager `.bin` shim makes the launcher brittle to upstream package structure changes.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const env = { ...process.env } | ||
| const args = ['dev', ...process.argv.slice(2)] | ||
| const root = join(dirname(fileURLToPath(import.meta.url)), '..') | ||
| const electronVite = join(root, 'node_modules', 'electron-vite', 'bin', 'electron-vite.js') |
There was a problem hiding this comment.
P2: Hardcoding the electron-vite internal path (node_modules/electron-vite/bin/electron-vite.js) instead of using the package-manager .bin shim makes the launcher brittle to upstream package structure changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/windows/scripts/dev.mjs, line 10:
<comment>Hardcoding the `electron-vite` internal path (`node_modules/electron-vite/bin/electron-vite.js`) instead of using the package-manager `.bin` shim makes the launcher brittle to upstream package structure changes.</comment>
<file context>
@@ -7,12 +7,7 @@ import { fileURLToPath } from 'node:url'
- '.bin',
- process.platform === 'win32' ? 'electron-vite.cmd' : 'electron-vite'
-)
+const electronVite = join(root, 'node_modules', 'electron-vite', 'bin', 'electron-vite.js')
if (env.ELECTRON_RUN_AS_NODE) {
</file context>
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the update. The current wrapper looks like it addresses the robustness concerns I had: it forwards npm run dev -- ... arguments, avoids shell parsing by invoking the electron-vite JS bin through the current Node executable, runs from the Windows desktop package root, and only removes ELECTRON_RUN_AS_NODE from the spawned Electron dev process.
I also checked that electron-vite@5.0.0 exposes bin/electron-vite.js, and the wrapper parses cleanly with Node. This is a positive signal from my side.
I’m still not formally approving because this changes the Windows dev/package-script workflow and the PR is already marked for maintainer/workflow review. A human maintainer should make the final call before merge.
2 participants
Summary
ELECTRON_RUN_AS_NODEbefore launchingelectron-vite devnpm run devcommand unchanged for developersWhy
If
ELECTRON_RUN_AS_NODE=1leaks into the shell, Electron starts as a Node runtime instead of an Electron app. That makeselectron.appundefined and crashes startup inside@electron-toolkit/utils.Testing
npm run typechecknpx eslint scripts/dev.mjsELECTRON_RUN_AS_NODE=1 npm run devstarts without the previouselectron.app.isPackagedcrash