feat(frontend): add configurable dev server port and DevTools #448
Conversation
|
All contributors have signed the CLA. Thank you! |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughLoads environment variables at startup, adds example env entries for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)apps/frontend/src/**/*.{ts,tsx,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/**/*.{ts,tsx}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
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 |
|
|
Summary of ChangesHello @steven-pribilinskiy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the frontend development workflow by introducing new configuration options via environment variables. Developers now have the flexibility to specify a custom port for the Vite development server and control the automatic opening of DevTools, streamlining their local setup and debugging processes. These changes aim to provide a more adaptable and user-friendly development environment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
|
I have read the CLA Document and I hereby sign the CLA |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces configurable environment variables for the Vite dev server port and auto-opening DevTools, which is a great addition for development flexibility. The implementation is mostly solid, but I've found a potential issue where providing a non-numeric value for the port could crash the dev server. I've left a comment with a suggestion for a more robust implementation. The other changes, including the documentation and the logic for DevTools, look good.
steven-pribilinskiy
force-pushed
the
feat/configurable-devtools-and-port
branch
from
f216c06 to
4aee086
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/frontend/.env.exampleapps/frontend/electron.vite.config.tsapps/frontend/src/main/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/electron.vite.config.tsapps/frontend/src/main/index.ts
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/index.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/index.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/.env.example
🧬 Code graph analysis (1)
apps/frontend/src/main/index.ts (2)
apps/frontend/src/main/ipc-handlers/index.ts (1)
setupIpcHandlers(43-110)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(690-690)
🔇 Additional comments (3)
apps/frontend/.env.example (1)
45-50: LGTM!The documentation clearly describes the new environment variables with appropriate defaults and usage instructions. The values match the implementation in the config and main files.
apps/frontend/src/main/index.ts (2)
96-98: LGTM!The DevTools opening condition now correctly respects the
OPEN_DEVTOOLSenvironment variable while maintaining the development mode check. The string comparison with'false'aligns with the documented behavior in.env.example.
267-274: Verify that usage monitor initialization belongs in this PR.This code initializes and starts the usage monitor after window creation, which is new functionality not mentioned in the PR objectives (which focus on configurable dev server port and DevTools). While the implementation looks correct, it appears to be scope creep.
Was this change intentionally included, or should it be in a separate PR?
steven-pribilinskiy
force-pushed
the
feat/configurable-devtools-and-port
branch
from
4aee086 to
eab8054
Compare
steven-pribilinskiy
force-pushed
the
feat/configurable-devtools-and-port
branch
2 times, most recently
from
c38bf87 to
5a6cdc5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/frontend/electron.vite.config.ts (2)
4-7: Missingdotenvdependency will cause runtime error.The code imports and invokes
dotenv.config(), butdotenvis not listed inapps/frontend/package.jsonor the rootpackage.json(as confirmed by previous verification). This will cause a "Cannot find module 'dotenv'" error when the config loads.Add
dotenvtodevDependenciesinapps/frontend/package.json:"devDependencies": { "dotenv": "^16.0.0" }
62-62: Add port range validation.While
Number()correctly handles non-numeric values via the||fallback, it doesn't validate that the port is within the valid range (1-65535). SettingVITE_DEV_PORT=99999will cause Vite to fail with a cryptic error.🔎 Proposed fix with validation
- port: Number(process.env.VITE_DEV_PORT) || 5173, + port: (() => { + const port = Number(process.env.VITE_DEV_PORT); + if (isNaN(port) || port < 1 || port > 65535) { + if (process.env.VITE_DEV_PORT) { + console.warn(`Invalid VITE_DEV_PORT: ${process.env.VITE_DEV_PORT}, using default 5173`); + } + return 5173; + } + return port; + })(),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/frontend/.env.exampleapps/frontend/electron.vite.config.tsapps/frontend/src/main/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/index.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/index.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/index.tsapps/frontend/electron.vite.config.ts
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/.env.example
| # Vite dev server port (default: 5173) | ||
| # VITE_DEV_PORT=5173 | ||
|
|
||
| # Auto-open Chrome DevTools in dev mode (default: true) | ||
| # OPEN_DEVTOOLS=false |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Documentation is clear and matches implementation.
The environment variable documentation accurately describes the defaults and usage. The examples correctly show how to configure the dev server port and disable DevTools.
Optional enhancement: Consider adding a note that OPEN_DEVTOOLS is case-sensitive (only lowercase 'false' disables DevTools):
# Auto-open Chrome DevTools in dev mode (default: true)
+# Note: Must be exactly 'false' (lowercase) to disable
# OPEN_DEVTOOLS=false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Vite dev server port (default: 5173) | |
| # VITE_DEV_PORT=5173 | |
| # Auto-open Chrome DevTools in dev mode (default: true) | |
| # OPEN_DEVTOOLS=false | |
| # Vite dev server port (default: 5173) | |
| # VITE_DEV_PORT=5173 | |
| # Auto-open Chrome DevTools in dev mode (default: true) | |
| # Note: Must be exactly 'false' (lowercase) to disable | |
| # OPEN_DEVTOOLS=false |
🤖 Prompt for AI Agents
In apps/frontend/.env.example around lines 46 to 50, add a short note that the
OPEN_DEVTOOLS value is case-sensitive and only the lowercase string 'false' will
disable DevTools; update the comment above the example to explicitly mention
this behavior so users know to use exactly "false" (lowercase) rather than
"False" or "FALSE".
|
|
||
| // Open DevTools in development | ||
| if (is.dev) { | ||
| if (is.dev && process.env.OPEN_DEVTOOLS !== 'false') { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider case-insensitive and additional falsy value support.
The current check process.env.OPEN_DEVTOOLS !== 'false' is case-sensitive and only recognizes the exact string 'false'. Common variations like 'False', 'FALSE', '0', or 'no' will still open DevTools, which may be unexpected.
🔎 Optional improvement for more flexible handling
- if (is.dev && process.env.OPEN_DEVTOOLS !== 'false') {
+ if (is.dev && !['false', '0', 'no'].includes(process.env.OPEN_DEVTOOLS?.toLowerCase() ?? '')) {
mainWindow.webContents.openDevTools({ mode: 'right' });
}This handles false, False, FALSE, 0, no, No, NO as falsy values.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (is.dev && process.env.OPEN_DEVTOOLS !== 'false') { | |
| if (is.dev && !['false', '0', 'no'].includes(process.env.OPEN_DEVTOOLS?.toLowerCase() ?? '')) { |
🤖 Prompt for AI Agents
In apps/frontend/src/main/index.ts at line 96, the environment check
process.env.OPEN_DEVTOOLS !== 'false' is case-sensitive and only treats the
exact string 'false' as falsy; update it to normalize the env value (e.g.,
toLowerCase and trim) and compare against a set of falsy tokens like 'false',
'0', 'no', '' (or undefined) so values such as 'False', 'FALSE', '0', or 'no'
are treated as falsy; implement by reading the env into a variable, normalizing,
and checking membership in a predefined falsy array before deciding to open
DevTools.
5a6cdc5 to
313baed
Compare
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
1 issue(s) must be addressed (1 required, 0 recommended)
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Low | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- High: 1 issue(s)
- Low: 4 issue(s)
Generated by Auto Claude PR Review
Findings (5 selected of 5 total)
🟠 [HIGH] dotenv imported but not declared in package.json
📁 apps/frontend/electron.vite.config.ts:4
The PR imports { config } from 'dotenv' but dotenv is not listed in package.json dependencies or devDependencies. It only exists as a transitive dependency (via electron-vite). Importing transitive dependencies directly is fragile - they can disappear when parent packages update. Additionally, Vite/electron-vite already handles .env file loading automatically via Vite's built-in env support, making this import potentially unnecessary.
Suggested fix:
Option 1: Remove the dotenv import entirely - Vite automatically loads .env files and variables prefixed with VITE_ are available via process.env. Option 2: If dotenv is truly needed, add it to devDependencies: `npm install --save-dev dotenv`
🔵 [LOW] Case-sensitive OPEN_DEVTOOLS comparison
📁 apps/frontend/src/main/index.ts:96
The check process.env.OPEN_DEVTOOLS !== 'false' is case-sensitive, so 'FALSE', 'False', or 'NO' would not disable DevTools. This differs from common conventions where boolean environment variables are often case-insensitive.
Suggested fix:
Consider using case-insensitive comparison: `process.env.OPEN_DEVTOOLS?.toLowerCase() !== 'false'`
🔵 [LOW] Inconsistent boolean env var pattern
📁 apps/frontend/src/main/index.ts:96
The proposed !== 'false' pattern differs from the established codebase pattern. Other boolean env vars in this file use === 'true' pattern (e.g., DEBUG, DEBUG_UPDATER). The current approach is actually more user-friendly (opt-out vs opt-in) but deviates from project conventions.
Suggested fix:
Document the intentional difference in .env.example, or for consistency use: `process.env.SKIP_DEVTOOLS !== 'true'` with inverted naming
🔵 [LOW] AI false positive: parseInt radix concern does not apply
📁 apps/frontend/electron.vite.config.ts:62
CodeRabbit flagged 'Add radix parameter to parseInt' as critical, but the code uses Number() not parseInt(). The Number() function doesn't take a radix parameter. This AI comment should be dismissed.
Suggested fix:
Dismiss this AI comment - it misread the code. Number(x) || fallback is the correct pattern used.
🔵 [LOW] AI false positive: Non-numeric string parsing concern
📁 apps/frontend/electron.vite.config.ts:62
Gemini flagged port parsing issues with non-numeric strings as high priority, but this is a false positive. Number('abc') returns NaN, and NaN || 5173 correctly falls back to the default. The pattern handles invalid input safely.
Suggested fix:
Dismiss this AI comment - the Number() || fallback pattern correctly handles non-numeric strings.
This review was generated by Auto Claude.
"Does Vite automatically load .env files for use in vite.config.ts? Can you use process.env in the config file without manual dotenv loading?"According to the electron-vite and Vite documentation. Tool Call: Fetch https://electron-vite.org/guide/env-and-mode electron-vite Environment Variable HandlingBased on the documentation, yes, electron-vite automatically loads .env files from the project root, similar to Vite's behavior. Key Mechanisms:Prefix-based Scoping: electron-vite uses different prefixes to control where variables are accessible:
Variables without these prefixes aren't exposed to any process. Customization: You can modify the default prefix through the Config vs. Runtime Access:The documentation indicates that environment variables are accessed at runtime through The configuration file itself uses these same environment variables through the standard access pattern, making the handling consistent across your build process. Tool Call: Fetch https://vite.dev/guide/env-and-mode.html Based on the documentation provided, Vite does not automatically load .env files for use in vite.config.ts. The documentation explicitly states: "It also allows you to read This clarifies that environment variables from The automatic AssistantThe Vite documentation confirms it: Vite does NOT automatically load .env files for use in vite.config.ts. The So the |
MikeeBuilds
added
feature
|
@AndyMik90 something is wrong with the CLA Assistant check, I've already wrote "I have read the CLA Document and I hereby sign the CLA" |
|
I have read the CLA Document and I hereby sign the CLA |
- Add VITE_DEV_PORT env var to customize Vite dev server port - Add OPEN_DEVTOOLS env var to control auto-opening DevTools in dev mode - Document both variables in .env.example - Load dotenv in electron.vite.config.ts for env var support
steven-pribilinskiy
force-pushed
the
feat/configurable-devtools-and-port
branch
from
313baed to
eaa6b1d
Compare
|
It is a bit pathetic that for a trivial change you have to run
I would suggest to add a high-level PR agent reviewer that can decide whether an army of AI LLM reviewers is even necessary |
5 participants
Summary
Add configurable environment variables for frontend development flexibility.
Changes
Files Modified
apps/frontend/.env.example- Document new env varsapps/frontend/electron.vite.config.ts- Load dotenv and use VITE_DEV_PORTapps/frontend/src/main/index.ts- Respect OPEN_DEVTOOLS settingSummary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.