Conversation
📝 WalkthroughWalkthroughReplaced hard-coded web and API base URLs with environment-aware constants exported from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/webview/[path].tsx (1)
19-21: Duplicate environment configuration.As noted in
services/forceupdate.ts, this duplicates theAPP_ENVconstant and URL derivation logic. Centralizing this configuration would improve maintainability and ensure consistency.♻️ Suggested import from shared config
-const APP_ENV = process.env.APP_ENV || 'production'; - -const webUrl = APP_ENV === 'development' ? 'https://stage.agit.gg' : 'https://agit.gg'; +import { webUrl } from '../../config/env';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/webview/`[path].tsx around lines 19 - 21, Replace the duplicated APP_ENV and webUrl logic in the webview component by importing the shared configuration used elsewhere (the centralized APP_ENV and URL derivation) instead of redeclaring them: remove the local const APP_ENV and const webUrl, import the shared constant or helper that derives the webUrl (the same value used in services/forceupdate.ts), and use that imported symbol wherever webUrl or APP_ENV is referenced in the component.services/forceupdate.ts (1)
11-12: Consider centralizing environment configuration.The
APP_ENVconstant and URL derivation logic are duplicated here and inapp/webview/[path].tsx. Extract this to a shared config module (e.g.,config/env.ts) to avoid duplication and ensure consistent environment handling across the codebase.Additionally, only
'development'is explicitly handled—values like'staging'or'dev'would silently fall through to production URLs. Consider using a stricter check or documenting the expected values.♻️ Suggested centralized config
Create a shared config file:
// config/env.ts const APP_ENV = process.env.APP_ENV || 'production'; export const isDevelopment = APP_ENV === 'development'; export const apiUrl = isDevelopment ? 'https://api.stage.agit.gg' : 'https://api.agit.gg'; export const webUrl = isDevelopment ? 'https://stage.agit.gg' : 'https://agit.gg';Then import where needed:
-const APP_ENV = process.env.APP_ENV || 'production'; -const apiUrl = APP_ENV === 'development' ? 'https://api.stage.agit.gg' : 'https://api.agit.gg'; +import { apiUrl } from '../config/env';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/forceupdate.ts` around lines 11 - 12, Duplicate environment logic around APP_ENV and apiUrl should be centralized: extract APP_ENV, the boolean check (e.g., isDevelopment) and apiUrl/webUrl derivation into a new shared module (e.g., config/env.ts) and replace the inline constants in functions/file-level code such as APP_ENV and apiUrl in forceupdate.ts and the similar block in app/webview/[path].tsx to import from that module; also tighten the environment check to explicit allowed values (e.g., treat only 'development' as dev and consider validating/enumerating other values like 'staging' vs 'production') so the new exports (APP_ENV or isDevelopment, apiUrl, webUrl) provide a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/webview/`[path].tsx:
- Around line 19-21: Replace the duplicated APP_ENV and webUrl logic in the
webview component by importing the shared configuration used elsewhere (the
centralized APP_ENV and URL derivation) instead of redeclaring them: remove the
local const APP_ENV and const webUrl, import the shared constant or helper that
derives the webUrl (the same value used in services/forceupdate.ts), and use
that imported symbol wherever webUrl or APP_ENV is referenced in the component.
In `@services/forceupdate.ts`:
- Around line 11-12: Duplicate environment logic around APP_ENV and apiUrl
should be centralized: extract APP_ENV, the boolean check (e.g., isDevelopment)
and apiUrl/webUrl derivation into a new shared module (e.g., config/env.ts) and
replace the inline constants in functions/file-level code such as APP_ENV and
apiUrl in forceupdate.ts and the similar block in app/webview/[path].tsx to
import from that module; also tighten the environment check to explicit allowed
values (e.g., treat only 'development' as dev and consider
validating/enumerating other values like 'staging' vs 'production') so the new
exports (APP_ENV or isDevelopment, apiUrl, webUrl) provide a single source of
truth.
ff1451
left a comment
There was a problem hiding this comment.
코드래빗 리뷰대로 하나의 파일에서 만들어두고 사용해도 괜찮을 거 같네요
우진님 생각에 따라 진행해주세요 사용되는 부분이 많지 않아서 우선 그대로 진행해도 괜찮아요!
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
constants/constants.ts (1)
3-5: Consider aligning environment value with actual environment name.The condition checks for
'development'to select stage URLs, which could be confusing since "development" typically refers to local development. Consider using'staging'or'stage'as the environment value for clarity:♻️ Optional: Use 'staging' for stage environment
-const APP_ENV = process.env.APP_ENV || 'production'; +const APP_ENV = process.env.APP_ENV || 'production'; export const apiUrl = - APP_ENV === 'development' ? 'https://api.stage.agit.gg' : 'https://api.agit.gg'; -export const webUrl = APP_ENV === 'development' ? 'https://stage.agit.gg' : 'https://agit.gg'; + APP_ENV === 'staging' ? 'https://api.stage.agit.gg' : 'https://api.agit.gg'; +export const webUrl = APP_ENV === 'staging' ? 'https://stage.agit.gg' : 'https://agit.gg';Alternatively, if
'development'is intentional and aligns with your CI/CD conventions, this is fine to keep as-is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@constants/constants.ts` around lines 3 - 5, The environment conditional for apiUrl and webUrl currently checks APP_ENV === 'development' to select staging URLs; update the condition to use a clearer env value (e.g., 'staging' or 'stage') or add support for both tokens to avoid confusion: change the checks in the apiUrl and webUrl definitions (symbols: apiUrl, webUrl) to test APP_ENV === 'staging' (or APP_ENV === 'stage') or use a set/array like ['development','staging'].includes(APP_ENV) so the correct stage URLs are selected consistently with your CI/CD naming.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@constants/constants.ts`:
- Around line 1-5: The constants file relies on process.env.APP_ENV (symbols:
APP_ENV, apiUrl, webUrl in constants/constants.ts) which won't be available in
plain React Native; update the project to use a supported RN env solution (e.g.,
install and configure react-native-config or a Babel plugin like
react-native-dotenv / transform-inline-environment-variables and add .env files)
and replace the current lookup with the configured mechanism (or use the RN
__DEV__ boolean as a safe fallback) so APP_ENV is correctly populated at
build/runtime and apiUrl/webUrl resolve as intended.
---
Nitpick comments:
In `@constants/constants.ts`:
- Around line 3-5: The environment conditional for apiUrl and webUrl currently
checks APP_ENV === 'development' to select staging URLs; update the condition to
use a clearer env value (e.g., 'staging' or 'stage') or add support for both
tokens to avoid confusion: change the checks in the apiUrl and webUrl
definitions (symbols: apiUrl, webUrl) to test APP_ENV === 'staging' (or APP_ENV
=== 'stage') or use a set/array like ['development','staging'].includes(APP_ENV)
so the correct stage URLs are selected consistently with your CI/CD naming.
| const APP_ENV = process.env.APP_ENV || 'production'; | ||
|
|
||
| export const apiUrl = | ||
| APP_ENV === 'development' ? 'https://api.stage.agit.gg' : 'https://api.agit.gg'; | ||
| export const webUrl = APP_ENV === 'development' ? 'https://stage.agit.gg' : 'https://agit.gg'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for environment variable handling configuration in babel or metro config
echo "=== Checking babel.config.js for env plugin ==="
fd -t f "babel.config" --exec cat {}
echo ""
echo "=== Checking for .env related packages in package.json ==="
fd -t f "package.json" -d 1 --exec cat {} | jq '.dependencies, .devDependencies' 2>/dev/null | grep -i -E "(dotenv|env|babel-plugin)"
echo ""
echo "=== Checking metro.config.js ==="
fd -t f "metro.config" --exec cat {}
echo ""
echo "=== Full package.json for complete dependency list ==="
cat package.json 2>/dev/null | head -100Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 2388
🏁 Script executed:
#!/bin/bash
# Check for .env files
echo "=== Checking for .env files ==="
fd -t f "\.env" || echo "No .env files found"
echo ""
echo "=== Checking if babel.config.js or metro.config.js exist as js/ts files ==="
ls -la | grep -E "(babel|metro)" || echo "No babel or metro config files"
echo ""
echo "=== Check app.json for expo configuration ==="
fd -t f "app.json" --exec cat {}Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 257
Verify process.env.APP_ENV is properly configured for React Native.
In React Native, environment variables via process.env require explicit bundler configuration (e.g., babel-plugin-transform-inline-environment-variables, react-native-dotenv, or similar). Without proper setup, process.env.APP_ENV will be undefined at runtime, always falling back to 'production'. Currently, no babel configuration, environment variable packages, or .env files are present in the project.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@constants/constants.ts` around lines 1 - 5, The constants file relies on
process.env.APP_ENV (symbols: APP_ENV, apiUrl, webUrl in constants/constants.ts)
which won't be available in plain React Native; update the project to use a
supported RN env solution (e.g., install and configure react-native-config or a
Babel plugin like react-native-dotenv / transform-inline-environment-variables
and add .env files) and replace the current lookup with the configured mechanism
(or use the RN __DEV__ boolean as a safe fallback) so APP_ENV is correctly
populated at build/runtime and apiUrl/webUrl resolve as intended.
Summary by CodeRabbit