-
Notifications
You must be signed in to change notification settings - Fork 548
feat: add unified dev command for backend & frontend #1040
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
base: main
Are you sure you want to change the base?
feat: add unified dev command for backend & frontend #1040
Conversation
- Add 'npm run dev' using concurrently to run both services - Add 'npm run backend' and 'npm run frontend' helper commands - Install concurrently package for reliable multi-process management - Implements proper cleanup with --kill-others-on-fail flag - Fix execute permissions on backend/run.sh Fixes AOSSIE-Org#837
📝 WalkthroughWalkthroughAdds npm scripts to run backend and frontend concurrently and platform-specific backend runners; introduces devDependencies to support cross-platform script dispatch; adds a Windows batch script to start the backend with test and normal modes and optional worker configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (CLI)
participant NPM as npm / run-script-os
participant Concurrent as concurrently
participant Front as Frontend (Tauri Dev)
participant Back as Backend Runner
Dev->>NPM: npm run dev
NPM->>run-script-os: detect platform -> choose backend script
NPM->>Concurrent: invoke concurrently with named prefixes
Concurrent->>Front: start `frontend` (tauri dev)
Concurrent->>Back: start platform-specific `backend` script
Front-->>Concurrent: frontend ready
Back-->>Concurrent: backend ready
sequenceDiagram
participant BackScript as `backend/run.bat`
participant Env as Environment
participant Hypercorn as Hypercorn Server
BackScript->>Env: read args and WORKERS
alt --test present
BackScript->>Hypercorn: start with debug, reload, access-log on 0.0.0.0:8000
else normal
alt WORKERS set
BackScript->>Hypercorn: start with WORKERS=%WORKERS%
else WORKERS not set
BackScript->>Hypercorn: start with WORKERS=1
end
end
Hypercorn-->>BackScript: running
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @package.json:
- Around line 7-9: The current "backend" npm script calls "./run.sh" which
breaks on Windows; add platform-specific npm scripts (e.g., "backend:win" that
invokes the Windows batch equivalent like "backend/run.bat" or existing
"win-dev.bat", and "backend:unix" that invokes "backend/run.sh" or
"linux-dev.sh"), create the missing Windows batch script in backend/ if it
doesn't exist, and update the main "backend" script to be a thin platform-aware
wrapper that runs the correct one (you can implement the wrapper as a small
Node-based detection or an npm script that delegates to "backend:win" vs
"backend:unix") so "npm run dev" works cross-platform.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
backend/run.shpackage.json
🔇 Additional comments (1)
package.json (1)
21-21: concurrently v9.2.1 is valid and up to date.v9.2.1 is the latest stable release on npm with no known security vulnerabilities. The caret range allows safe minor and patch updates.
- Install run-script-os package for platform detection - Create backend/run.bat for Windows support - Split backend script into platform-specific variants: - backend:win32 for Windows - backend:darwin:linux for Unix-based systems - npm run dev now works on Windows, macOS, and Linux Addresses code review feedback on cross-platform compatibility
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @backend/run.bat:
- Around line 12-20: The batch script's WORKERS handling passes an empty value
to hypercorn; update the run.bat logic around the WORKERS check so that both
undefined and empty WORKERS default to 1 before calling hypercorn (e.g., test
"%WORKERS%"=="" or set a default variable when not defined/empty) and then
invoke hypercorn main:app with the resolved worker count instead of passing a
possibly-empty %WORKERS% directly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
backend/run.batpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (2)
backend/run.bat (2)
1-7: LGTM!The script setup is correct. Using
cd /d "%~dp0"properly handles drive changes and paths with spaces.
9-11: LGTM!Test mode configuration with debug logging, live reload, and access logging is appropriate for development.
| ) else ( | ||
| REM Print the value of the WORKERS environment variable | ||
| echo WORKERS: %WORKERS% | ||
| if not defined WORKERS ( | ||
| hypercorn main:app --workers 1 --bind 0.0.0.0:8000 | ||
| ) else ( | ||
| hypercorn main:app --workers %WORKERS% --bind 0.0.0.0:8000 | ||
| ) | ||
| ) |
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.
🧩 Analysis chain
🏁 Script executed:
cat backend/run.batRepository: AOSSIE-Org/PictoPy
Length of output: 653
🏁 Script executed:
cat backend/run.shRepository: AOSSIE-Org/PictoPy
Length of output: 852
Fix inconsistent WORKERS handling between scripts.
The run.bat and run.sh scripts behave differently when WORKERS is defined but empty. The batch script will pass an empty value to hypercorn, while the bash script correctly defaults to 1 (due to ${WORKERS:-1}). Update run.bat to match bash behavior by handling the empty case:
Fix: Handle undefined and empty WORKERS
) else (
REM Print the value of the WORKERS environment variable
echo WORKERS: %WORKERS%
- if not defined WORKERS (
+ if not defined WORKERS (
+ set "WORKERS=1"
+ )
+ if "%WORKERS%"=="" (
set "WORKERS=1"
)
- ) else (
- hypercorn main:app --workers %WORKERS% --bind 0.0.0.0:8000
- )
+ hypercorn main:app --workers %WORKERS% --bind 0.0.0.0:8000
)As an optional defensive improvement, both scripts could validate that WORKERS is a positive integer, though this is not critical since the value is typically controlled by the deployment environment.
🤖 Prompt for AI Agents
In @backend/run.bat around lines 12 - 20, The batch script's WORKERS handling
passes an empty value to hypercorn; update the run.bat logic around the WORKERS
check so that both undefined and empty WORKERS default to 1 before calling
hypercorn (e.g., test "%WORKERS%"=="" or set a default variable when not
defined/empty) and then invoke hypercorn main:app with the resolved worker count
instead of passing a possibly-empty %WORKERS% directly.
Summary
Implements a single unified command to run both backend and frontend services simultaneously, resolving the "onboarding friction" issue reported in #837.
Changes Made
npm run devcommand usingconcurrentlyto run both servicesnpm run backendhelper command for running backend onlynpm run frontendhelper command for running frontend onlyconcurrentlypackage (v9.2.1) as dev dependencyUsage