Skip to content

Conversation

@ibetitsmike
Copy link
Contributor

This PR improves Windows build support by introducing a conditional RUNNER variable in the Makefile that uses npx on Windows and bun x elsewhere.

Problem

bun x doesn't correctly pass arguments on Windows, causing issues with various build and development commands.

Solution

  • Added RUNNER variable that detects the OS and uses appropriate command runner
  • Windows: Uses npx for proper argument handling
  • Non-Windows: Uses bun x for better performance
  • Updated all commands throughout the Makefile to use $(RUNNER) instead of hardcoded bun x

Changes

  • Conditional OS detection sets RUNNER to npx on Windows, bun x elsewhere
  • All bun x commands replaced with $(RUNNER) for consistency
  • Added comments explaining why Windows needs different approach
  • Applied prettier formatting to ensure code style compliance

Testing

  • TypeScript checks pass
  • Unit tests pass
  • Formatting applied

Generated with cmux

- 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
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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".

Comment on lines 106 to 112
## 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"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@ibetitsmike
Copy link
Contributor Author

Closing in favor of #571 which has a cleaner implementation without conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant