-
Notifications
You must be signed in to change notification settings - Fork 32
🤖 fix: improve Windows build support with conditional RUNNER variable #570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Revert SSHRuntime/LocalRuntime/process cleanup edits - Remove platform-specific path layer and UI formatting - Restore bashExecutionService, execAsync, tests to main - Keep Windows build: Makefile bash shell + release.yml Windows job + setup-cmux action Only 3 files changed vs main: Makefile, release.yml, setup-cmux action.yml _Generated with `cmux`_.
- Add RUNNER variable that uses npx on Windows, bun x elsewhere - Windows: bun x doesn't correctly pass arguments, so use npm run/npx - Non-Windows: Continue using bun x for better performance - Update all commands to use instead of hardcoded bun x - Add comments explaining why Windows needs different approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ## Development | ||
| dev: node_modules/.installed build-main ## Start development server (Vite + tsgo watcher for 10x faster type checking) | ||
| @bun x concurrently -k \ | ||
| "bun x concurrently \"$(TSGO) -w -p tsconfig.main.json\" \"bun x tsc-alias -w -p tsconfig.main.json\"" \ | ||
| dev: node_modules/.installed build-main clean-cache ## Start development server (Vite + nodemon watcher for Windows compatibility) | ||
| @echo "Starting dev mode (2 watchers: nodemon for main process, vite for renderer)..." | ||
| @# On Windows, use npm run because bun x doesn't correctly pass arguments | ||
| @NODE_OPTIONS="--max-old-space-size=4096" $(RUNNER) concurrently -k --raw \ | ||
| "$(RUNNER) nodemon --exec node scripts/build-main-watch.js" \ | ||
| "vite" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configure nodemon to watch TypeScript sources
The new dev workflow replaces the previous tsgo -w watcher with nodemon ($(RUNNER) nodemon --exec node scripts/build-main-watch.js) but doesn’t tell nodemon which file extensions to monitor. Nodemon’s default set only includes .js, .mjs, and .json, so edits to the main process .ts files will no longer trigger a rebuild or restart, leaving the Electron backend stale. The same pattern appears in dev-server below. Please add --ext ts,tsx --watch src (or similar) so main TypeScript sources are actually rebuilt during development.
Useful? React with 👍 / 👎.
|
Closing in favor of #571 which has a cleaner implementation without conflicts |
This PR improves Windows build support by introducing a conditional
RUNNERvariable in the Makefile that usesnpxon Windows andbun xelsewhere.Problem
bun xdoesn't correctly pass arguments on Windows, causing issues with various build and development commands.Solution
RUNNERvariable that detects the OS and uses appropriate command runnernpxfor proper argument handlingbun xfor better performance$(RUNNER)instead of hardcodedbun xChanges
RUNNERtonpxon Windows,bun xelsewherebun xcommands replaced with$(RUNNER)for consistencyTesting
Generated with
cmux