fix: frontend run build error - #89
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses frontend build errors by implementing several key fixes: centralizing environment variable management, updating SCSS configuration for modern Sass API compatibility, improving TypeScript type safety, and fixing file handling paths. The changes ensure the frontend builds successfully in production environments with proper validation.
Changes:
- Centralized environment variable management with type safety and production validation
- Updated Sass configuration to use modern API and
@useinstead of deprecated@import - Enhanced TypeScript type definitions and fixed type safety issues in file upload handling
- Updated file path references from build-time imports to runtime public file access
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| alias/frontend/src/config/env.ts | New centralized environment configuration module with validation |
| alias/frontend/src/types/vite-env.d.ts | Updated type definitions for environment variables |
| alias/frontend/vite.config.ts | Updated Sass configuration for modern API compatibility |
| alias/frontend/src/main.tsx | Added production config validation at startup |
| alias/frontend/src/services/api/request.ts | Refactored to use centralized env config |
| alias/frontend/src/services/api/file.ts | Refactored to use centralized env config |
| alias/frontend/src/components/Chat/FileItems.tsx | Refactored to use centralized env config |
| alias/frontend/src/pages/Chat/ChatInput/index.tsx | Added proper type guards and optional chaining for upload callbacks |
| alias/frontend/src/pages/SharePage/index.tsx | Fixed type references and removed unused imports |
| alias/frontend/src/components/Workspace/index.tsx | Fixed type safety for tool_name access |
| alias/frontend/src/pages/Chat/utils.ts | Refactored file loading from import.meta.glob to runtime fetch |
| alias/frontend/src/assets/json/prompt.ts | Updated file path references and documentation |
| alias/frontend/public/file/incident_records.csv | New data file moved to public directory |
| alias/frontend/.env.example | Updated with better documentation and production-ready defaults |
| alias/.gitignore | Fixed to allow .env.* files and added dist/ directory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .env | ||
| .env.* | ||
| dist/ | ||
| !.env.* |
There was a problem hiding this comment.
The order of these gitignore patterns is problematic. Line 18 (!.env.) tries to negate the ignore for .env. files, but line 16 already has '.env' which will match before the negation. The negation should come immediately after the pattern it negates. Consider reordering to: .env then !.env.* then !.env.example on consecutive lines, or combine them as: .env and !.env.example (since .env.example is already negated on line 19).
| !.env.* |
| VITE_API_URL=https://api.yourdomain.com:8000 | ||
|
|
||
| # User profiling service API URL - user data and profiling service endpoint | ||
| VITE_USER_PROFILING_API_URL=https://api.yourdomain.com:6380 |
There was a problem hiding this comment.
The example URL includes HTTPS with a port number, which is uncommon in production (HTTPS typically uses standard port 443). This could be misleading. Consider using either https://api.yourdomain.com for production or http://localhost:8000 as a more realistic example, or add a comment explaining this is just an example placeholder.
| VITE_API_URL=https://api.yourdomain.com:8000 | |
| # User profiling service API URL - user data and profiling service endpoint | |
| VITE_USER_PROFILING_API_URL=https://api.yourdomain.com:6380 | |
| VITE_API_URL=https://api.yourdomain.com | |
| # User profiling service API URL - user data and profiling service endpoint | |
| VITE_USER_PROFILING_API_URL=https://user-profiling.yourdomain.com |
| * Production environment must set VITE_API_URL and VITE_USER_PROFILING_API_URL | ||
| */ | ||
| export function getConfig() { | ||
| if (env.isProd) { | ||
| // Production environment check required configuration | ||
| if (!env.apiUrl || env.apiUrl.trim() === "") { | ||
| console.warn( | ||
| "Warning: VITE_API_URL is not set for production environment, please check .env.production file or environment variables", | ||
| ); | ||
| } | ||
| if (!env.userProfilingApiUrl || env.userProfilingApiUrl.trim() === "") { | ||
| console.warn( | ||
| "Warning: VITE_USER_PROFILING_API_URL is not set for production environment, please check .env.production file or environment variables", | ||
| ); | ||
| } | ||
| return { | ||
| ...prodConfig, | ||
| // Use values from env (already validated), if empty use prodConfig (may also be empty) | ||
| apiUrl: env.apiUrl || prodConfig.apiUrl, | ||
| userProfilingApiUrl: | ||
| env.userProfilingApiUrl || prodConfig.userProfilingApiUrl, | ||
| }; | ||
| } | ||
| return { | ||
| ...devConfig, | ||
| // Development environment: prioritize values from env, otherwise use defaults | ||
| apiUrl: env.apiUrl || devConfig.apiUrl, | ||
| userProfilingApiUrl: | ||
| env.userProfilingApiUrl || devConfig.userProfilingApiUrl, | ||
| }; |
There was a problem hiding this comment.
The getConfig function appears redundant since it largely duplicates the logic already present in the env object (lines 34-63). The env object already handles production vs development defaults and validation. This function could be removed, and code should directly use the env object instead. If the function serves a specific purpose, it's not clear from the implementation.
| * Production environment must set VITE_API_URL and VITE_USER_PROFILING_API_URL | |
| */ | |
| export function getConfig() { | |
| if (env.isProd) { | |
| // Production environment check required configuration | |
| if (!env.apiUrl || env.apiUrl.trim() === "") { | |
| console.warn( | |
| "Warning: VITE_API_URL is not set for production environment, please check .env.production file or environment variables", | |
| ); | |
| } | |
| if (!env.userProfilingApiUrl || env.userProfilingApiUrl.trim() === "") { | |
| console.warn( | |
| "Warning: VITE_USER_PROFILING_API_URL is not set for production environment, please check .env.production file or environment variables", | |
| ); | |
| } | |
| return { | |
| ...prodConfig, | |
| // Use values from env (already validated), if empty use prodConfig (may also be empty) | |
| apiUrl: env.apiUrl || prodConfig.apiUrl, | |
| userProfilingApiUrl: | |
| env.userProfilingApiUrl || prodConfig.userProfilingApiUrl, | |
| }; | |
| } | |
| return { | |
| ...devConfig, | |
| // Development environment: prioritize values from env, otherwise use defaults | |
| apiUrl: env.apiUrl || devConfig.apiUrl, | |
| userProfilingApiUrl: | |
| env.userProfilingApiUrl || devConfig.userProfilingApiUrl, | |
| }; | |
| * Delegates to the unified env object, which already handles | |
| * production vs development defaults and validation. | |
| */ | |
| export function getConfig() { | |
| return env; |
| * Production environment must be set via environment variables, no default values provided to ensure clear configuration | ||
| */ | ||
| export const prodConfig = { | ||
| apiUrl: getEnv("VITE_API_URL", ""), | ||
| userProfilingApiUrl: getEnv("VITE_USER_PROFILING_API_URL", ""), | ||
| maxRetries: getEnvNumber("VITE_MAX_RETRIES", 3), | ||
| retryDelay: getEnvNumber("VITE_RETRY_DELAY", 1000), |
There was a problem hiding this comment.
The prodConfig is re-reading environment variables using getEnv at module initialization time, which duplicates the work already done in the env object. Since env variables are read once at build/startup time, prodConfig will have the same values as env. This creates unnecessary duplication and potential inconsistency. Consider removing prodConfig and devConfig exports entirely.
| * Production environment must be set via environment variables, no default values provided to ensure clear configuration | |
| */ | |
| export const prodConfig = { | |
| apiUrl: getEnv("VITE_API_URL", ""), | |
| userProfilingApiUrl: getEnv("VITE_USER_PROFILING_API_URL", ""), | |
| maxRetries: getEnvNumber("VITE_MAX_RETRIES", 3), | |
| retryDelay: getEnvNumber("VITE_RETRY_DELAY", 1000), | |
| * Values are derived from the unified env object to avoid re-reading environment variables | |
| */ | |
| export const prodConfig = { | |
| apiUrl: env.apiUrl, | |
| userProfilingApiUrl: env.userProfilingApiUrl, | |
| maxRetries: env.maxRetries, | |
| retryDelay: env.retryDelay, |
📝 PR Type
📚 Description
[Please briefly describe the background, changes, and purpose of this PR. For example:
game_werewolvesto demonstrate XYZ functionality inagentscope.game_test.pycaused byagentscopeinterface changes.README.mdofagentscope-samples.]🧪 Testing Validation
[Please explain how to validate the changes:
agentscoperequired?pre-commit)?]✅ Checklist
Please complete the following checks before submitting the PR:
pre-commit run --all-filespytest tests/)agentscopebest practices (e.g., config management, logging)agentscope-sampleshas been updated (e.g.,README.md)