fix: improve onboarding and handle undefined values#244
fix: improve onboarding and handle undefined values#244Benbenzhouz wants to merge 10 commits intomainfrom
Conversation
- Replace || with ?? in AuthCallbackContent.tsx - Replace || with ?? in UserInputArea.tsx - Replace || with ?? in GoogleOAuthButton.tsx - Replace || with ?? in AddressAutocomplete.tsx - Replace || with ?? in publicApiSlice.ts - Replace || with ?? in axiosBaseQuery.ts - Fix useEffect dependency in AddressAutocomplete.tsx - Remove console.log statements in AuthCallbackContent.tsx
- Create unified getApiBaseUrl() utility function - Remove hardcoded localhost fallbacks from all files - Production environment now requires API URL to be configured - Development environment allows localhost fallback with warning - Improves code quality and prevents production misconfigurations Files updated: - src/utils/api-config.ts (new) - src/services/places.ts - src/lib/axiosBaseQuery.ts - src/features/public/publicApiSlice.ts - src/components/GoogleOAuthButton.tsx - src/app/(public)/blogs/page.tsx - src/app/(public)/blogs/components/BlogList.tsx - src/app/(public)/blogs/[id]/page.tsx
- Remove unnecessary NEXT_PUBLIC_API_BASE_URL fallback - Use only NEXT_PUBLIC_API_URL as it's the actual configured variable - Simplify code and error messages
- Change from NEXT_PUBLIC_API_URL to NEXT_PUBLIC_API_BASE_URL - This matches the actual configured environment variable name
- Keep same behavior as before: NEXT_PUBLIC_API_BASE_URL ?? localhost fallback - Remove production environment check to match original usage - Environment variable name unchanged: NEXT_PUBLIC_API_BASE_URL - Function only provides centralized management, no behavior change
…upport - Restore NEXT_PUBLIC_API_BASE_URL and NEXT_PUBLIC_API_URL support - Keep function for centralized management - Match original behavior exactly
There was a problem hiding this comment.
Pull request overview
This PR centralizes API configuration management and improves undefined value handling in onboarding components to prevent runtime errors. The changes introduce a new getApiBaseUrl() utility function and update components to handle potentially undefined values more safely.
- Introduced centralized API configuration via
getApiBaseUrl()function with fallback support - Enhanced undefined/null value handling in AddressAutocomplete and UserInputArea components
- Improved address validation and formatting logic to match backend requirements
- Added validation for OAuth callback user data
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/api-config.ts | New utility function centralizing API base URL configuration with environment variable fallbacks |
| src/services/places.ts | Migrated to use centralized getApiBaseUrl() function |
| src/lib/axiosBaseQuery.ts | Replaced direct env var access with getApiBaseUrl() and fixed ESLint nullish coalescing issue |
| src/features/public/publicApiSlice.ts | Updated to use centralized API configuration |
| src/components/ui/AddressAutocomplete.tsx | Enhanced null/undefined handling, improved address formatting validation, and added external value syncing |
| src/components/GoogleOAuthButton.tsx | Migrated to use getApiBaseUrl() utility |
| src/app/onboarding/components/UserInputArea.tsx | Updated to handle undefined userInput prop with nullish coalescing operators |
| src/app/auth/callback/AuthCallbackContent.tsx | Added validation for OAuth user data and fallback values for optional fields |
| src/app/(public)/blogs/page.tsx | Replaced inline env var usage with getApiBaseUrl() |
| src/app/(public)/blogs/components/BlogList.tsx | Migrated to centralized API configuration |
| src/app/(public)/blogs/[id]/page.tsx | Updated to use getApiBaseUrl() utility |
| package.json | Explicitly set dev server port to 3000 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onClick={() => onTextSubmit(userInput)} | ||
| disabled={disabled || userInput.trim() === ''} | ||
| onClick={() => onTextSubmit(userInput ?? '')} | ||
| disabled={disabled || !userInput || userInput.trim() === ''} |
There was a problem hiding this comment.
The disabled check is redundant. The condition !userInput || userInput.trim() === '' already covers all cases where the button should be disabled. The !userInput check handles both null/undefined and empty string, so checking userInput.trim() === '' afterwards is unnecessary since it will never execute when userInput is falsy. Simplify to just disabled || !userInput?.trim().
| suggestion.description ?? | ||
| ''} |
There was a problem hiding this comment.
The null check suggestion.description ?? '' is unnecessary because the earlier check !suggestion at line 243 already ensures that suggestion is not null or undefined at this point. The optional chaining on structured_formatting is sufficient.
| suggestion.description ?? | |
| ''} | |
| suggestion.description} |
| if (value !== undefined && value !== inputValue) { | ||
| setInputValue(value ?? ''); | ||
| } | ||
| }, [value, inputValue]); |
There was a problem hiding this comment.
This useEffect creates an infinite loop. The effect depends on inputValue, and when value changes, it updates inputValue, which triggers the effect again. This will cause continuous re-renders. Consider removing inputValue from the dependency array since you only want to sync when the external value prop changes.
| }, [value, inputValue]); | |
| }, [value]); |
| lastName: parsedUser.lastName, | ||
| role: parsedUser.role, | ||
| status: parsedUser.status, | ||
| email: parsedUser.email ?? '', |
There was a problem hiding this comment.
The validation logic is incomplete. After validating that _id and email are present, the code then provides fallback values for these required fields using the nullish coalescing operator. This is inconsistent - if email is required (as checked in the validation), it shouldn't have a fallback to an empty string. Either remove the validation check for email or remove the fallback.
| email: parsedUser.email ?? '', | |
| email: parsedUser.email, |
| onClick={() => onTextSubmit(userInput)} | ||
| disabled={disabled || userInput.trim() === ''} | ||
| onClick={() => onTextSubmit(userInput ?? '')} | ||
| disabled={disabled || !userInput || userInput.trim() === ''} |
There was a problem hiding this comment.
The disabled check is redundant. The condition !userInput || userInput.trim() === '' already covers all cases where the button should be disabled. The !userInput check handles both null/undefined and empty string, so checking userInput.trim() === '' afterwards is unnecessary since it will never execute when userInput is falsy. Simplify to just disabled || !userInput?.trim().
Changes
Onboarding Improvements
Code Quality
Environment Variables
Environment Variables (No Changes Required)