Add Google Sign-Up Integration to main #132
Conversation
WalkthroughAdds Google Sign-In (web + native) via Firebase: new Firebase configs, AuthContext Google flow, LoginScreen UI updates, backend token exchange call, related deps and config changes, workflow env injection, minor .gitignore formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginScreen
participant AuthContext
participant GoogleSDK as GoogleSignIn (web/native)
participant Firebase
participant BackendAPI
User->>LoginScreen: Tap "Sign in with Google"
LoginScreen->>AuthContext: signInWithGoogle()
AuthContext->>GoogleSDK: Trigger platform-specific OAuth
GoogleSDK-->>AuthContext: Return token/credential
AuthContext->>Firebase: Sign in with credential / request ID token
Firebase-->>AuthContext: Firebase user + ID token
AuthContext->>BackendAPI: POST /auth/login/google (ID token)
BackendAPI-->>AuthContext: App access token + user data
AuthContext->>LoginScreen: Update auth state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
hey @Devasy23 created the PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (11)
frontend/tsconfig.json (1)
1-4: Enable stricter TS checks for better safety (optional)Consider turning on stricter checks and preventing TS emit in an Expo/Babel setup.
Apply this diff:
-{ - "compilerOptions": {}, - "extends": "expo/tsconfig.base" -} +{ + "extends": "expo/tsconfig.base", + "compilerOptions": { + "strict": true, + "noEmit": true, + "jsx": "react-jsx", + "allowJs": true, + "noUncheckedIndexedAccess": true + } +}frontend/package.json (1)
14-14: Double‑check native Google Sign-In vs Expo AuthSession choiceYou added @react-native-google-signin/google-signin and expo-dev-client. That’s valid for native, but if you also intend to support web via a unified flow, Expo AuthSession is usually simpler to share code across platforms.
- Confirm whether you intend to use react-native-google-signin only for Android/iOS and a separate flow for web.
- If a single shared flow is desired, consider using expo-auth-session (plus expo-web-browser) for all platforms.
I can propose a minimal AuthSession-based flow that works on web and native if helpful.
Also applies to: 21-22, 26-26
frontend/eas.json (1)
14-16: Production auto-increment set—consider explicit strategy (optional)
autoIncrement: trueis fine; if you want predictable behavior across platforms, considerautoIncrement: "version"and manage platform-specific codes in app.json.Example:
- "production": { - "autoIncrement": true - } + "production": { + "autoIncrement": "version" + }frontend/app.json (1)
3-6: Confirm Google redirect URIs/deep linksWith
scheme: "splitwiser"and added browser plugin, ensure your Google OAuth client is configured with redirect URIs that include:
- splitwiser://redirect
- For web: https://auth.expo.dev or your hosting URL per your flow
I can help list exact redirect URIs once you confirm whether you use Expo AuthSession or native Google Sign-In on each platform.
Also applies to: 34-37
frontend/firebase/firebaseConfig.ts (2)
13-16: Remove debug logs; avoid leaking config/auth objectsLogging full Firebase config and auth object is noisy and risks exposing data. Gate logs in dev and avoid printing secrets/tokens.
-console.log('Firebase Config:', firebaseConfig); -const app = initializeApp(firebaseConfig); -const auth = getAuth(app); -console.log('Firebase Auth Initialized:', auth); +const app = initializeApp(firebaseConfig); +const auth = getAuth(app); +if (__DEV__) { + console.log('Firebase configured'); +}
18-18: Optional: prompt account chooser on webImprove UX by prompting the Google account chooser each time.
-const googleProvider = new GoogleAuthProvider(); +const googleProvider = new GoogleAuthProvider(); +googleProvider.setCustomParameters({ prompt: "select_account" });frontend/screens/LoginScreen.js (2)
200-207: Remove unused styles to reduce noise
googleIconandgoogleButtonTextare defined but unused.- googleIcon: { - marginRight: 12, - }, - googleButtonText: { - fontSize: 16, - color: '#475569', - fontWeight: '500', - },
88-96: Accessibility nit: add accessibility labels/rolesImprove accessibility for touch targets and buttons.
- <TouchableOpacity + <TouchableOpacity + accessibilityRole="button" + accessibilityLabel="Forgot password" style={styles.forgotPassword} onPress={() => navigation.navigate('ForgotPassword')} > @@ - <Button + <Button + accessibilityLabel="Sign in with Google" icon="google" mode="outlined" onPress={handleGoogleLogin}Also applies to: 113-122
frontend/context/AuthContext.js (3)
31-35: Include iOS client id in GoogleSignin.configureFor iOS, provide
iosClientId. Also consider scopes if you need profile/email explicitly.GoogleSignin.configure({ webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, offlineAccess: true, + iosClientId: process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID, });
250-268: Sign out from Firebase/Google regardless offirebaseUserstate
firebaseUsermay be null after reload (not restored yet). Always attempt to sign out of providers.- // Sign out from Google/Firebase if applicable - if (firebaseUser) { - if (Platform.OS === 'web') { - await auth.signOut(); - } else { - await GoogleSignin.signOut(); - await auth.signOut(); - } - } + // Sign out from Google/Firebase (best-effort) + if (Platform.OS === 'web') { + await auth.signOut(); + } else { + try { await GoogleSignin.signOut(); } catch {} + try { await auth.signOut(); } catch {} + }
75-80: Make token update listener stable (optional)If
setTokenUpdateListenerreplaces a singleton listener, re-calling it on every token/refresh change is unnecessary. Consider setting it once.-useEffect(() => { - setTokenUpdateListener(async ({ accessToken, refreshToken }) => { - if (accessToken && accessToken !== token) setToken(accessToken); - if (refreshToken && refreshToken !== refresh) setRefresh(refreshToken); - }); -}, [token, refresh]); +useEffect(() => { + setTokenUpdateListener(async ({ accessToken, refreshToken }) => { + if (accessToken) setToken(accessToken); + if (refreshToken) setRefresh(refreshToken); + }); + // eslint-disable-next-line react-hooks/exhaustive-deps +}, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
frontend/assets/adaptive-icon/background.pngis excluded by!**/*.pngfrontend/assets/adaptive-icon/foreground.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-144.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-192.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-512.pngis excluded by!**/*.pngfrontend/assets/splash.pngis excluded by!**/*.pngfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.gitignore(0 hunks)frontend/api/auth.js(1 hunks)frontend/app.json(2 hunks)frontend/context/AuthContext.js(5 hunks)frontend/eas.json(2 hunks)frontend/firebase/firebaseConfig.ts(1 hunks)frontend/package.json(1 hunks)frontend/screens/LoginScreen.js(1 hunks)frontend/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- .gitignore
🔇 Additional comments (7)
frontend/tsconfig.json (1)
1-4: Baseline TS config is fineExtending Expo’s base is a good starting point.
frontend/package.json (1)
12-34: SDK 53 Compatibility VerifiedAll key dependencies in
frontend/package.jsonalready match Expo SDK 53’s supported versions:
- react@19.0.0
- react-native@0.79.5
- react-native-screens@~4.11.1
- react-native-safe-area-context@5.4.0
- @react-native-async-storage/async-storage@2.1.2
No changes required. 🎉
frontend/eas.json (1)
3-5: CLI constraints look goodRaising the minimum CLI and using remote app versioning aligns with managed builds.
frontend/app.json (3)
9-13: App splash and UI style changes are fineAutomatic UI style and new splash asset look good.
28-28: No action needed for google-services.jsonThe
google-services.jsonfile is not tracked in git (as verified by your script), so it’s already excluded from version control and there’s nothing further to do here.
27-29: Validate Android applicationId and Firebase config alignmentUnderscores are permitted in Android package names, but please ensure the following:
- A
google-services.jsonfile exists at the path specified by"googleServicesFile"infrontend/app.json.- The
"package_name"inside that Firebase config exactly matches the"package": "splitwiser_1.android"value infrontend/app.json.- If the file lives elsewhere or is missing, add/move it or update
googleServicesFileaccordingly.frontend/screens/LoginScreen.js (1)
43-52: LGTM: Solid Google sign-in trigger and loading handlingClear loading state management and error feedback. Delegating navigation to auth state is fine.
frontend/api/auth.js
Outdated
| export const signInWithGoogle = async (id_token) => { | ||
| return apiClient.post('/auth/login/google', JSON.stringify({ id_token })); | ||
| }; |
There was a problem hiding this comment.
Don’t JSON.stringify the payload; remove unnecessary async; align naming
Axios (likely what apiClient wraps) will JSON-encode objects automatically and set proper headers. Passing a string may lead to wrong Content-Type and 415s. Also, async is unused.
Apply this diff:
-export const signInWithGoogle = async (id_token) => {
- return apiClient.post('/auth/login/google', JSON.stringify({ id_token }));
-};
+export const signInWithGoogle = (idToken) => {
+ // Keep snake_case if backend expects id_token; map locally from camelCase
+ return apiClient.post('/auth/login/google', { id_token: idToken });
+};Optional: If your backend expects camelCase, use { idToken } consistently with other endpoints.
📝 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.
| export const signInWithGoogle = async (id_token) => { | |
| return apiClient.post('/auth/login/google', JSON.stringify({ id_token })); | |
| }; | |
| export const signInWithGoogle = (idToken) => { | |
| // Keep snake_case if backend expects id_token; map locally from camelCase | |
| return apiClient.post('/auth/login/google', { id_token: idToken }); | |
| }; |
🤖 Prompt for AI Agents
In frontend/api/auth.js around lines 11 to 13, remove the unnecessary async
keyword from the signInWithGoogle function since it does not use await. Also, do
not JSON.stringify the payload; instead, pass the object { idToken } directly to
apiClient.post so Axios can handle JSON encoding and headers correctly. Rename
the parameter and payload key to idToken to maintain consistent camelCase naming
with other endpoints.
| GoogleAuthProvider, | ||
| signInWithCredential, | ||
| signInWithPopup, | ||
| User, | ||
| } from 'firebase/auth'; |
There was a problem hiding this comment.
Bug: Importing User from firebase/auth in a JS file will crash
User is a TS type, not a runtime export. This import causes a bundling/runtime error.
import {
GoogleAuthProvider,
signInWithCredential,
signInWithPopup,
- User,
} from 'firebase/auth';📝 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.
| GoogleAuthProvider, | |
| signInWithCredential, | |
| signInWithPopup, | |
| User, | |
| } from 'firebase/auth'; | |
| import { | |
| GoogleAuthProvider, | |
| signInWithCredential, | |
| signInWithPopup, | |
| } from 'firebase/auth'; |
🤖 Prompt for AI Agents
In frontend/context/AuthContext.js between lines 7 and 11, remove the import of
`User` from 'firebase/auth' because it is a TypeScript type and not available at
runtime in JavaScript, which causes bundling or runtime errors. Simply delete
`User` from the import statement to fix the issue.
| const storedToken = await AsyncStorage.getItem('auth_token'); | ||
| const storedUser = await AsyncStorage.getItem('user_data'); | ||
| const storedRefresh = await AsyncStorage.getItem("refresh_token"); | ||
| const storedUser = await AsyncStorage.getItem("user_data"); | ||
| const storedIdToken = await AsyncStorage.getItem('firebase_id_token'); | ||
| const storedFirebaseUser = await AsyncStorage.getItem('firebase_user'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Load refresh/idToken/firebaseUser; remove duplicate setUser
Currently:
setUseris called twice.refreshisn’t restored into state.- Firebase ID token and user aren’t restored at startup.
Fix all three.
- const storedToken = await AsyncStorage.getItem('auth_token');
- const storedUser = await AsyncStorage.getItem('user_data');
+ const storedToken = await AsyncStorage.getItem('auth_token');
+ const storedUser = await AsyncStorage.getItem('user_data');
const storedRefresh = await AsyncStorage.getItem("refresh_token");
const storedIdToken = await AsyncStorage.getItem('firebase_id_token');
const storedFirebaseUser = await AsyncStorage.getItem('firebase_user');
- if (storedToken && storedUser) {
- setToken(storedToken);
- setUser(JSON.parse(storedUser));
- await setAuthTokens({
- newAccessToken: storedToken,
- newRefreshToken: storedRefresh,
- });
- // Normalize user id shape: ensure `_id` exists even if API stored `id`
- const parsed = JSON.parse(storedUser);
- const normalized = parsed?._id
- ? parsed
- : parsed?.id
- ? { ...parsed, _id: parsed.id }
- : parsed;
- setUser(normalized);
- }
+ // Restore Firebase auth data (if present)
+ if (storedIdToken) setIdToken(storedIdToken);
+ if (storedFirebaseUser) {
+ try {
+ setFirebaseUser(JSON.parse(storedFirebaseUser));
+ } catch {
+ // ignore parse error; storage may contain older format
+ }
+ }
+
+ // Restore app tokens and user
+ if (storedToken && storedUser) {
+ setToken(storedToken);
+ if (storedRefresh) setRefresh(storedRefresh);
+ await setAuthTokens({
+ newAccessToken: storedToken,
+ newRefreshToken: storedRefresh,
+ });
+ // Normalize user id shape: ensure `_id` exists even if API stored `id`
+ const parsed = JSON.parse(storedUser);
+ const normalized = parsed?._id
+ ? parsed
+ : parsed?.id
+ ? { ...parsed, _id: parsed.id }
+ : parsed;
+ setUser(normalized);
+ }Also applies to: 49-63
🤖 Prompt for AI Agents
In frontend/context/AuthContext.js around lines 42 to 47 and 49 to 63, fix the
issue where setUser is called twice by removing the duplicate call, ensure the
refresh token is restored into state by setting it appropriately, and restore
the Firebase ID token and Firebase user into state at startup by retrieving and
setting them correctly. This will consolidate state initialization and avoid
redundant updates.
| // Save Firebase data to AsyncStorage | ||
| useEffect(() => { | ||
| const saveFirebaseData = async () => { | ||
| try { | ||
| if (idToken) { | ||
| await AsyncStorage.setItem('firebase_id_token', idToken); | ||
| } else { | ||
| await AsyncStorage.removeItem('firebase_id_token'); | ||
| } | ||
|
|
||
| if (firebaseUser) { | ||
| await AsyncStorage.setItem('firebase_user', JSON.stringify(firebaseUser)); | ||
| } else { | ||
| await AsyncStorage.removeItem('firebase_user'); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to save Firebase data to storage:', error); | ||
| } | ||
| }; | ||
|
|
||
| saveFirebaseData(); | ||
| }, [idToken, firebaseUser]); |
There was a problem hiding this comment.
Avoid JSON.stringify on Firebase User; store a safe subset
Firebase User contains circular references. Stringifying it will throw. Persist a minimal, serializable snapshot instead.
- if (firebaseUser) {
- await AsyncStorage.setItem('firebase_user', JSON.stringify(firebaseUser));
- } else {
+ if (firebaseUser) {
+ const snapshot = {
+ uid: firebaseUser.uid,
+ displayName: firebaseUser.displayName ?? null,
+ email: firebaseUser.email ?? null,
+ photoURL: firebaseUser.photoURL ?? null,
+ providerData: (firebaseUser.providerData || []).map(p => ({
+ providerId: p?.providerId,
+ uid: p?.uid,
+ displayName: p?.displayName ?? null,
+ email: p?.email ?? null,
+ photoURL: p?.photoURL ?? null,
+ })),
+ };
+ await AsyncStorage.setItem('firebase_user', JSON.stringify(snapshot));
+ } else {
await AsyncStorage.removeItem('firebase_user');
}📝 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.
| // Save Firebase data to AsyncStorage | |
| useEffect(() => { | |
| const saveFirebaseData = async () => { | |
| try { | |
| if (idToken) { | |
| await AsyncStorage.setItem('firebase_id_token', idToken); | |
| } else { | |
| await AsyncStorage.removeItem('firebase_id_token'); | |
| } | |
| if (firebaseUser) { | |
| await AsyncStorage.setItem('firebase_user', JSON.stringify(firebaseUser)); | |
| } else { | |
| await AsyncStorage.removeItem('firebase_user'); | |
| } | |
| } catch (error) { | |
| console.error('Failed to save Firebase data to storage:', error); | |
| } | |
| }; | |
| saveFirebaseData(); | |
| }, [idToken, firebaseUser]); | |
| // Save Firebase data to AsyncStorage | |
| useEffect(() => { | |
| const saveFirebaseData = async () => { | |
| try { | |
| if (idToken) { | |
| await AsyncStorage.setItem('firebase_id_token', idToken); | |
| } else { | |
| await AsyncStorage.removeItem('firebase_id_token'); | |
| } | |
| if (firebaseUser) { | |
| const snapshot = { | |
| uid: firebaseUser.uid, | |
| displayName: firebaseUser.displayName ?? null, | |
| email: firebaseUser.email ?? null, | |
| photoURL: firebaseUser.photoURL ?? null, | |
| providerData: (firebaseUser.providerData || []).map(p => ({ | |
| providerId: p?.providerId, | |
| uid: p?.uid, | |
| displayName: p?.displayName ?? null, | |
| email: p?.email ?? null, | |
| photoURL: p?.photoURL ?? null, | |
| })), | |
| }; | |
| await AsyncStorage.setItem('firebase_user', JSON.stringify(snapshot)); | |
| } else { | |
| await AsyncStorage.removeItem('firebase_user'); | |
| } | |
| } catch (error) { | |
| console.error('Failed to save Firebase data to storage:', error); | |
| } | |
| }; | |
| saveFirebaseData(); | |
| }, [idToken, firebaseUser]); |
🤖 Prompt for AI Agents
In frontend/context/AuthContext.js around lines 131 to 152, the code attempts to
JSON.stringify the entire Firebase user object, which contains circular
references and causes errors. To fix this, extract and store only a minimal,
serializable subset of the Firebase user data (such as uid, email, displayName)
instead of the full user object before saving it to AsyncStorage.
frontend/firebase/firebaseConfig.ts
Outdated
| import { initializeApp } from "firebase/app"; | ||
| import { getAuth, GoogleAuthProvider } from "firebase/auth"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against double-initialization and add native persistence
On Fast Refresh you can hit "Firebase App named '[DEFAULT]' already exists". Also, on React Native you should initialize auth with AsyncStorage persistence; getAuth() defaults to in-memory.
-import { initializeApp } from "firebase/app";
-import { getAuth, GoogleAuthProvider } from "firebase/auth";
+import { initializeApp, getApp, getApps } from "firebase/app";
+import { getAuth, GoogleAuthProvider, initializeAuth } from "firebase/auth";
+import { getReactNativePersistence } from "firebase/auth/react-native";
+import AsyncStorage from "@react-native-async-storage/async-storage";
+import { Platform } from "react-native";
@@
-const app = initializeApp(firebaseConfig);
-const auth = getAuth(app);
+const app = getApps().length ? getApp() : initializeApp(firebaseConfig);
+const auth =
+ Platform.OS === "web"
+ ? getAuth(app)
+ : initializeAuth(app, {
+ persistence: getReactNativePersistence(AsyncStorage),
+ });Also applies to: 14-15
🤖 Prompt for AI Agents
In frontend/firebase/firebaseConfig.ts at lines 1-2 and 14-15, guard against
double-initialization of the Firebase app by checking if an app instance already
exists before calling initializeApp. Additionally, for React Native, initialize
the auth instance with AsyncStorage persistence instead of the default in-memory
persistence by importing AsyncStorage and passing it to the getAuth() call. This
prevents errors on Fast Refresh and ensures proper auth state persistence.
| const firebaseConfig = { | ||
| apiKey: process.env.EXPO_PUBLIC_FIREBASE_API_KEY, | ||
| authDomain: process.env.EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN, | ||
| projectId: process.env.EXPO_PUBLIC_FIREBASE_PROJECT_ID, | ||
| storageBucket: process.env.EXPO_PUBLIC_FIREBASE_STORAGE_BUCKET, | ||
| messagingSenderId: process.env.EXPO_PUBLIC_FIREBASE_MESSAGING_SENDER_ID, | ||
| appId: process.env.EXPO_PUBLIC_FIREBASE_APP_ID, | ||
| measurementId: process.env.EXPO_PUBLIC_FIREBASE_MEASUREMENT_ID, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate required env vars to fail fast
If any core Firebase env var is missing, initialization will behave unpredictably. Validate and throw with a clear message (or warn in dev).
const firebaseConfig = {
apiKey: process.env.EXPO_PUBLIC_FIREBASE_API_KEY,
authDomain: process.env.EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN,
projectId: process.env.EXPO_PUBLIC_FIREBASE_PROJECT_ID,
storageBucket: process.env.EXPO_PUBLIC_FIREBASE_STORAGE_BUCKET,
messagingSenderId: process.env.EXPO_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
appId: process.env.EXPO_PUBLIC_FIREBASE_APP_ID,
measurementId: process.env.EXPO_PUBLIC_FIREBASE_MEASUREMENT_ID,
};
+const required = [
+ "EXPO_PUBLIC_FIREBASE_API_KEY",
+ "EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN",
+ "EXPO_PUBLIC_FIREBASE_PROJECT_ID",
+ "EXPO_PUBLIC_FIREBASE_APP_ID",
+];
+const missing = required.filter((k) => !process.env[k]);
+if (missing.length) {
+ const msg = `Missing Firebase env vars: ${missing.join(", ")}`;
+ if (__DEV__) console.warn(msg);
+ else throw new Error(msg);
+}🤖 Prompt for AI Agents
In frontend/firebase/firebaseConfig.ts around lines 4 to 12, the Firebase
configuration uses environment variables without validation, which can cause
unpredictable behavior if any are missing. Add validation logic to check that
all required environment variables are present before exporting the config. If
any are missing, throw an error with a clear message or log a warning in
development mode to fail fast and aid debugging.
|
Why is the expo preview build failing? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/preview.yml (2)
32-36: Add concurrency to avoid overlapping preview updates per PR/branchPrevent multiple updates from racing on the same branch when commits are pushed rapidly.
jobs: preview: runs-on: ubuntu-latest + concurrency: + group: preview-${{ github.event.pull_request.head.ref }} + cancel-in-progress: true
28-31: Use deterministic installs and caching for faster, reproducible CISwitch to npm ci (requires package-lock.json) and enable caching.
- - name: Install dependencies - run: npm install + - name: Install dependencies + run: npm ci working-directory: ./frontendAnd cache Node modules:
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 18 + cache: 'npm' + cache-dependency-path: frontend/package-lock.jsonIf the repo doesn’t use package-lock.json, stick to npm install and omit caching or migrate to a lockfile.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/preview.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: bundle-analysis
|
An Expo user account is required to proceed. |
|
I think the coderabbit has mentioned has correct reason for why expo reason was failing, It is due to external fork doesn't have access to expo token in gh action |
|
Hey @Devasy23 what can i do now |
Can you just have a small doc of what changes you made and what a dev needs to do to run, apart from the existing main branch |
|
I have just added firebase configuration Added sign in with google in login screen |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/preview.yml (1)
3-5: Fix failing previews on forked PRs: gate secrets or skip forksGitHub does not expose EXPO_TOKEN to pull_request workflows from forks. Your job currently runs unconditionally and fails on external PRs. Apply one of the safe guards below.
Option A — Skip forks (minimal, safe):
name: Create EAS Preview jobs: preview: + if: ${{ github.event.pull_request.head.repo.fork == false }} runs-on: ubuntu-latestOption B — Allow maintainers to opt-in via label (safer for forks):
- name: Create preview - uses: expo/expo-github-action/preview@v8 + uses: expo/expo-github-action/preview@v8 + if: ${{ github.event.pull_request.head.repo.fork == false || contains(github.event.pull_request.labels.*.name, 'safe-to-preview') }} with: command: eas update --auto --branch ${{ github.event.pull_request.head.ref }} working-directory: ./frontendOptional hardening:
- If you need secrets on forks, switch to pull_request_target, explicitly check out head.sha, and gate secret-using steps behind a maintainer label.
Also applies to: 26-26, 32-36
🧹 Nitpick comments (2)
.github/workflows/preview.yml (1)
36-36: Add missing newline at EOF to satisfy YAML lintingYAMLlint reports “no new line character at the end of file”. Restore a trailing newline.
- working-directory: ./frontend + working-directory: ./frontend +frontend/firebase/firebaseConfig.js (1)
18-18: Set Google provider parameters for better UXPrompt account chooser to avoid auto-selecting a cached account.
const googleProvider = new GoogleAuthProvider(); +googleProvider.setCustomParameters({ prompt: "select_account" });Optional: if you support enterprise domains, you can also set
hd: "your-domain.com".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/preview.yml(1 hunks)frontend/api/client.js(1 hunks)frontend/firebase/firebaseConfig.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/api/client.js
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/preview.yml
[error] 36-36: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: bundle-analysis
🔇 Additional comments (1)
frontend/firebase/firebaseConfig.js (1)
1-2: Ensure Firebase initialization is guarded & React Native persistence is enabledTo prevent “default app already exists” on hot reload and enable session persistence on React Native, update
frontend/firebase/firebaseConfig.js:• Import and guard
initializeAppwithgetApps()/getApp()
• Branch onPlatform.OSto use web vs. native persistence
• Callauth.useDeviceLanguage()after init
• If targeting RN, install and import@react-native-async-storage/async-storage-import { initializeApp } from "firebase/app"; -import { getAuth, GoogleAuthProvider } from "firebase/auth"; +import { initializeApp, getApps, getApp } from "firebase/app"; +import { getAuth, GoogleAuthProvider } from "firebase/auth"; +import { Platform } from "react-native"; -const app = initializeApp(firebaseConfig); -const auth = getAuth(app); +const app = getApps().length ? getApp() : initializeApp(firebaseConfig); +let auth; +if (Platform.OS === "web") { + auth = getAuth(app); +} else { + // Use native persistence on React Native + const { initializeAuth, getReactNativePersistence } = + require("firebase/auth/react-native"); + const AsyncStorage = + require("@react-native-async-storage/async-storage").default; + auth = initializeAuth(app, { + persistence: getReactNativePersistence(AsyncStorage), + }); +} +auth.useDeviceLanguage();• Add
@react-native-async-storage/async-storageto yourdependenciesif using React Native.
• Confirm only one raw call toinitializeApp(...)remains in the codebase.
| const firebaseConfig = { | ||
| apiKey: process.env.EXPO_PUBLIC_FIREBASE_API_KEY, | ||
| authDomain: process.env.EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN, | ||
| projectId: process.env.EXPO_PUBLIC_FIREBASE_PROJECT_ID, | ||
| storageBucket: process.env.EXPO_PUBLIC_FIREBASE_STORAGE_BUCKET, | ||
| messagingSenderId: process.env.EXPO_PUBLIC_FIREBASE_MESSAGING_SENDER_ID, | ||
| appId: process.env.EXPO_PUBLIC_FIREBASE_APP_ID, | ||
| measurementId: process.env.EXPO_PUBLIC_FIREBASE_MEASUREMENT_ID, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fail fast on missing required Firebase config keys
Improve DX and avoid silent misconfig by validating required keys at startup.
const firebaseConfig = {
apiKey: process.env.EXPO_PUBLIC_FIREBASE_API_KEY,
authDomain: process.env.EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN,
projectId: process.env.EXPO_PUBLIC_FIREBASE_PROJECT_ID,
storageBucket: process.env.EXPO_PUBLIC_FIREBASE_STORAGE_BUCKET,
messagingSenderId: process.env.EXPO_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
appId: process.env.EXPO_PUBLIC_FIREBASE_APP_ID,
measurementId: process.env.EXPO_PUBLIC_FIREBASE_MEASUREMENT_ID,
};
+// Minimal runtime validation (keys required for Auth flows)
+const requiredKeys = ["apiKey", "authDomain", "projectId", "appId"];
+const missing = requiredKeys.filter((k) => !firebaseConfig[k]);
+if (missing.length) {
+ const msg = `[Firebase] Missing config keys: ${missing.join(", ")}. Check EXPO_PUBLIC_FIREBASE_* env.`;
+ if (process.env.NODE_ENV !== "production") throw new Error(msg);
+ console.warn(msg);
+}I can add a README snippet documenting required EXPO_PUBLIC_FIREBASE_* vars and setup steps for Android/iOS/Web. Want me to draft it?
📝 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.
| const firebaseConfig = { | |
| apiKey: process.env.EXPO_PUBLIC_FIREBASE_API_KEY, | |
| authDomain: process.env.EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN, | |
| projectId: process.env.EXPO_PUBLIC_FIREBASE_PROJECT_ID, | |
| storageBucket: process.env.EXPO_PUBLIC_FIREBASE_STORAGE_BUCKET, | |
| messagingSenderId: process.env.EXPO_PUBLIC_FIREBASE_MESSAGING_SENDER_ID, | |
| appId: process.env.EXPO_PUBLIC_FIREBASE_APP_ID, | |
| measurementId: process.env.EXPO_PUBLIC_FIREBASE_MEASUREMENT_ID, | |
| }; | |
| const firebaseConfig = { | |
| apiKey: process.env.EXPO_PUBLIC_FIREBASE_API_KEY, | |
| authDomain: process.env.EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN, | |
| projectId: process.env.EXPO_PUBLIC_FIREBASE_PROJECT_ID, | |
| storageBucket: process.env.EXPO_PUBLIC_FIREBASE_STORAGE_BUCKET, | |
| messagingSenderId: process.env.EXPO_PUBLIC_FIREBASE_MESSAGING_SENDER_ID, | |
| appId: process.env.EXPO_PUBLIC_FIREBASE_APP_ID, | |
| measurementId: process.env.EXPO_PUBLIC_FIREBASE_MEASUREMENT_ID, | |
| }; | |
| // Minimal runtime validation (keys required for Auth flows) | |
| const requiredKeys = ["apiKey", "authDomain", "projectId", "appId"]; | |
| const missing = requiredKeys.filter((k) => !firebaseConfig[k]); | |
| if (missing.length) { | |
| const msg = `[Firebase] Missing config keys: ${missing.join(", ")}. Check EXPO_PUBLIC_FIREBASE_* env.`; | |
| if (process.env.NODE_ENV !== "production") throw new Error(msg); | |
| console.warn(msg); | |
| } |
🤖 Prompt for AI Agents
In frontend/firebase/firebaseConfig.js around lines 4 to 12, the Firebase config
object uses environment variables without validation, which can lead to silent
misconfigurations. Add a validation step at startup to check that all required
EXPO_PUBLIC_FIREBASE_* environment variables are present, and if any are
missing, throw an error or log a clear message to fail fast. This ensures
developers are immediately aware of missing config keys and improves developer
experience.
frontend/firebase/firebaseConfig.js
Outdated
| console.log('Firebase Config:', firebaseConfig); | ||
| const app = initializeApp(firebaseConfig); | ||
| const auth = getAuth(app); | ||
| console.log('Firebase Auth Initialized:', auth); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove production logs of config/auth to prevent leakage and noisy logs
Logging the full Firebase config and Auth instance can leak identifiers and clutter logs. Remove or gate to dev-only.
-console.log('Firebase Config:', firebaseConfig);
const app = initializeApp(firebaseConfig);
-const auth = getAuth(app);
-console.log('Firebase Auth Initialized:', auth);
+const auth = getAuth(app);Optional dev-only logging (non-diff, add if truly needed):
if (process.env.NODE_ENV !== 'production') {
console.debug('Firebase initialized', { projectId: firebaseConfig.projectId });
}🤖 Prompt for AI Agents
In frontend/firebase/firebaseConfig.js around lines 13 to 16, remove the
console.log statements that output the full Firebase config and Auth instance to
avoid leaking sensitive information and cluttering production logs. Instead, if
logging is necessary, wrap it in a condition that checks if the environment is
not production (e.g., process.env.NODE_ENV !== 'production') and log only
minimal, non-sensitive details like the projectId.
|
hey @Devasy23 refracted the code according to coderabbit suggestions |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
frontend/context/AuthContext.js (3)
42-70: Restore Firebase auth data at startup; remove duplicate setUser; set refresh stateCurrently:
idTokenandfirebaseUserare fetched but not restored unless app tokens exist.setUseris called twice.refreshisn’t restored into state.Apply this diff:
- const storedToken = await AsyncStorage.getItem('auth_token'); - const storedUser = await AsyncStorage.getItem('user_data'); + const storedToken = await AsyncStorage.getItem('auth_token'); + const storedUser = await AsyncStorage.getItem('user_data'); const storedRefresh = await AsyncStorage.getItem("refresh_token"); - const storedIdToken = await AsyncStorage.getItem('firebase_id_token'); - const storedFirebaseUser = await AsyncStorage.getItem('firebase_user'); + const storedIdToken = await AsyncStorage.getItem('firebase_id_token'); + const storedFirebaseUser = await AsyncStorage.getItem('firebase_user'); - if (storedToken && storedUser) { - if (storedFirebaseUser) { - try { - setFirebaseUser(JSON.parse(storedFirebaseUser)); - } catch { - // ignore parse error; storage may contain older format - } - } - setToken(storedToken); - setUser(JSON.parse(storedUser)); + // Restore Firebase auth data regardless of app token presence + if (storedIdToken) setIdToken(storedIdToken); + if (storedFirebaseUser) { + try { + setFirebaseUser(JSON.parse(storedFirebaseUser)); + } catch { + // ignore parse error; storage may contain older format + } + } + + if (storedToken && storedUser) { + setToken(storedToken); + if (storedRefresh) setRefresh(storedRefresh); await setAuthTokens({ newAccessToken: storedToken, newRefreshToken: storedRefresh, }); // Normalize user id shape: ensure `_id` exists even if API stored `id` - const parsed = JSON.parse(storedUser); - const normalized = parsed?._id + const parsed = JSON.parse(storedUser); + const normalized = parsed?._id ? parsed : parsed?.id ? { ...parsed, _id: parsed.id } : parsed; - setUser(normalized); + setUser(normalized); }
4-13: Fix imports: remove TS-onlyUser; import platform-resolved Firebase config
Useris a TypeScript type, not a runtime export; importing it in JS causes bundling/runtime errors.- Import firebase config without
.websuffix so Metro resolves.native.json native and.web.json web. Using the.webfile directly breaks native sign-in.Apply this diff:
-import { Platform } from 'react-native'; -import { GoogleSignin } from '@react-native-google-signin/google-signin'; +import { Platform } from 'react-native'; +import { GoogleSignin } from '@react-native-google-signin/google-signin'; import { GoogleAuthProvider, signInWithCredential, signInWithPopup, - User, } from 'firebase/auth'; -import { auth } from '../firebase/firebaseConfig.web'; +import { auth } from '../firebase/firebaseConfig';
138-160: Do not JSON.stringify the Firebase User object; store a safe subsetFirebase
Userhas circular references and non-serializable fields. Persist a minimal snapshot instead.Apply this diff:
- if (firebaseUser) { - await AsyncStorage.setItem('firebase_user', JSON.stringify(firebaseUser)); - } else { + if (firebaseUser) { + const snapshot = { + uid: firebaseUser.uid, + displayName: firebaseUser.displayName ?? null, + email: firebaseUser.email ?? null, + photoURL: firebaseUser.photoURL ?? null, + providerData: (firebaseUser.providerData || []).map(p => ({ + providerId: p?.providerId, + uid: p?.uid, + displayName: p?.displayName ?? null, + email: p?.email ?? null, + photoURL: p?.photoURL ?? null, + })), + }; + await AsyncStorage.setItem('firebase_user', JSON.stringify(snapshot)); + } else { await AsyncStorage.removeItem('firebase_user'); }
🧹 Nitpick comments (1)
frontend/firebase/firebaseConfig.native.js (1)
6-14: measurementId is not used on nativeOptional cleanup:
measurementIdis for web analytics. It’s harmless, but you can remove it to reduce config noise on native.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
frontend/api/auth.js(1 hunks)frontend/context/AuthContext.js(5 hunks)frontend/firebase/firebaseConfig.native.js(1 hunks)frontend/firebase/firebaseConfig.web.js(1 hunks)frontend/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/api/auth.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/firebase/firebaseConfig.web.js (1)
frontend/firebase/firebaseConfig.native.js (4)
firebaseConfig(6-14)app(16-16)auth(18-20)googleProvider(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: bundle-analysis
🔇 Additional comments (4)
frontend/package.json (2)
37-40: EAS CLI moved to devDependencies — good changeBuild-time tooling should not ship in the app bundle. This aligns with best practices.
13-33: Expo SDK 53 dependency compatibility confirmed
- React (
19.0.0) and React Native (0.79.5) match Expo SDK 53’s requirements.- You’ve upgraded react-native-safe-area-context to
5.x(5.4.0), which aligns with Expo 53 and @react-navigation’s peer dependency.- For react-native-screens, run:
to pull in the exact version Expo SDK 53 supports.npx expo install react-native-screens- Finally, execute:
to validate your full dependency matrix and catch any remaining mismatches.npx expo-doctorfrontend/firebase/firebaseConfig.web.js (1)
1-20: LGTM: Web Firebase initialization and exports are correctSingleton app init,
getAuth(app), andGoogleAuthProvidersetup look good.frontend/context/AuthContext.js (1)
204-259: Google sign-in flow looks correct and securely exchanges Firebase ID token for app tokens
- Web:
signInWithPopupthen backend exchange.- Native:
GoogleSignin.signIn()-> credential ->signInWithCredential-> backend exchange.- Tokens are injected into the API client via
setAuthTokens.
| // Configure Google Sign-In on component mount | ||
| useEffect(() => { | ||
| GoogleSignin.configure({ | ||
| webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID, | ||
| offlineAccess: true, | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding platform-specific client IDs to GoogleSignin.configure
For the native flow, many setups require iosClientId and/or androidClientId in addition to webClientId to ensure proper token issuance. Confirm your Google credentials and add as needed.
Example (only if your credentials exist):
useEffect(() => {
GoogleSignin.configure({
- webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID,
+ webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID,
+ iosClientId: process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID,
+ androidClientId: process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID,
offlineAccess: true,
});
}, []);To verify repo config includes the required plugin and env wiring, run:
🏁 Script executed:
#!/bin/bash
# Check app config and credentials presence
fd -a 'app.(json|config.*)' | xargs -I{} sh -c 'echo "--- {} ---"; cat "{}"'
rg -n "react-native-google-signin|google-services|EXPO_PUBLIC_GOOGLE_(WEB|IOS|ANDROID)_CLIENT_ID|expo-secure-store|expo-web-browser" -A 3Length of output: 5366
Add iOS and Android client IDs to GoogleSignin.configure
To ensure native flows on both platforms issue the correct tokens, include iosClientId and androidClientId alongside your existing webClientId. You’ll also want to verify that these variables are defined in your environment (e.g. in .env or via Expo’s extra), and exposed as EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID and EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID.
Locations to update:
- frontend/context/AuthContext.js (around lines 29–35)
- Your environment definition (
.env, app.config.js, or wherever you expose Expo “extra”)
Example diff:
--- a/frontend/context/AuthContext.js
+++ b/frontend/context/AuthContext.js
@@ useEffect(() => {
- GoogleSignin.configure({
- webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID,
- offlineAccess: true,
- });
+ GoogleSignin.configure({
+ webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID,
+ iosClientId: process.env.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID,
+ androidClientId: process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID,
+ offlineAccess: true,
+ });Quick environment check:
# Ensure your .env or Expo config exposes the IDs
grep -R "EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID" -n .
grep -R "EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID" -n .🤖 Prompt for AI Agents
In frontend/context/AuthContext.js around lines 29 to 35, update the
GoogleSignin.configure call to include iosClientId and androidClientId
properties, sourcing their values from environment variables
EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID and EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID
respectively. Also, verify that these environment variables are properly defined
and exposed in your .env file or Expo configuration (such as app.config.js) to
ensure native platform flows receive the correct tokens.
| // Unified logout function | ||
| const logout = async () => { | ||
| try { | ||
| // Clear stored authentication data | ||
| await AsyncStorage.removeItem('auth_token'); | ||
| await AsyncStorage.removeItem('user_data'); | ||
| await AsyncStorage.removeItem('firebase_id_token'); | ||
| await AsyncStorage.removeItem('firebase_user'); | ||
|
|
||
| // Sign out from Google/Firebase if applicable | ||
| if (firebaseUser) { | ||
| if (Platform.OS === 'web') { | ||
| await auth.signOut(); | ||
| } else { | ||
| await GoogleSignin.signOut(); | ||
| await auth.signOut(); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error("Failed to clear stored authentication:", error); | ||
| } | ||
|
|
||
| // Clear all state | ||
| setToken(null); | ||
| setRefresh(null); | ||
| setUser(null); | ||
| await clearAuthTokens(); | ||
| setFirebaseUser(null); | ||
| setIdToken(null); | ||
| }; |
There was a problem hiding this comment.
Logout should also remove the refresh token from storage
refresh_token remains in AsyncStorage after logout. Clear it to avoid stale re-auth attempts.
Apply this diff:
- await AsyncStorage.removeItem('auth_token');
- await AsyncStorage.removeItem('user_data');
- await AsyncStorage.removeItem('firebase_id_token');
- await AsyncStorage.removeItem('firebase_user');
+ await AsyncStorage.removeItem('auth_token');
+ await AsyncStorage.removeItem('refresh_token');
+ await AsyncStorage.removeItem('user_data');
+ await AsyncStorage.removeItem('firebase_id_token');
+ await AsyncStorage.removeItem('firebase_user');📝 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.
| // Unified logout function | |
| const logout = async () => { | |
| try { | |
| // Clear stored authentication data | |
| await AsyncStorage.removeItem('auth_token'); | |
| await AsyncStorage.removeItem('user_data'); | |
| await AsyncStorage.removeItem('firebase_id_token'); | |
| await AsyncStorage.removeItem('firebase_user'); | |
| // Sign out from Google/Firebase if applicable | |
| if (firebaseUser) { | |
| if (Platform.OS === 'web') { | |
| await auth.signOut(); | |
| } else { | |
| await GoogleSignin.signOut(); | |
| await auth.signOut(); | |
| } | |
| } | |
| } catch (error) { | |
| console.error("Failed to clear stored authentication:", error); | |
| } | |
| // Clear all state | |
| setToken(null); | |
| setRefresh(null); | |
| setUser(null); | |
| await clearAuthTokens(); | |
| setFirebaseUser(null); | |
| setIdToken(null); | |
| }; | |
| // Unified logout function | |
| const logout = async () => { | |
| try { | |
| // Clear stored authentication data | |
| await AsyncStorage.removeItem('auth_token'); | |
| await AsyncStorage.removeItem('refresh_token'); | |
| await AsyncStorage.removeItem('user_data'); | |
| await AsyncStorage.removeItem('firebase_id_token'); | |
| await AsyncStorage.removeItem('firebase_user'); | |
| // Sign out from Google/Firebase if applicable | |
| if (firebaseUser) { | |
| if (Platform.OS === 'web') { | |
| await auth.signOut(); | |
| } else { | |
| await GoogleSignin.signOut(); | |
| await auth.signOut(); | |
| } | |
| } | |
| } catch (error) { | |
| console.error("Failed to clear stored authentication:", error); | |
| } | |
| // Clear all state | |
| setToken(null); | |
| setRefresh(null); | |
| setUser(null); | |
| await clearAuthTokens(); | |
| setFirebaseUser(null); | |
| setIdToken(null); | |
| }; |
🤖 Prompt for AI Agents
In frontend/context/AuthContext.js between lines 260 and 289, the logout
function clears several authentication items from AsyncStorage but does not
remove the refresh token. To fix this, add an await AsyncStorage.removeItem call
for the 'refresh_token' key alongside the other removeItem calls to ensure the
refresh token is also cleared during logout.
| const auth = initializeAuth(app, { | ||
| persistence: getReactNativePersistence(AsyncStorage), | ||
| }); | ||
|
|
There was a problem hiding this comment.
Guard initializeAuth to avoid RN Auth re-initialization errors (HMR/dev reload)
Calling initializeAuth more than once throws (e.g., during fast refresh). Use a guard and fall back to getAuth after first init.
Apply this diff:
-import { initializeAuth, GoogleAuthProvider } from "firebase/auth";
+import { initializeAuth, GoogleAuthProvider, getAuth } from "firebase/auth";
@@
-const auth = initializeAuth(app, {
- persistence: getReactNativePersistence(AsyncStorage),
-});
+// Avoid double-initialization across HMR/fast refresh
+const getOrInitAuth = () => {
+ if (!globalThis.__FIREBASE_AUTH_INITIALIZED__) {
+ const inst = initializeAuth(app, {
+ persistence: getReactNativePersistence(AsyncStorage),
+ });
+ globalThis.__FIREBASE_AUTH_INITIALIZED__ = true;
+ return inst;
+ }
+ return getAuth(app);
+};
+
+const auth = getOrInitAuth();📝 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.
| const auth = initializeAuth(app, { | |
| persistence: getReactNativePersistence(AsyncStorage), | |
| }); | |
| // Import (add getAuth) | |
| import { initializeAuth, GoogleAuthProvider, getAuth } from "firebase/auth"; | |
| import { getReactNativePersistence } from "firebase/auth/react-native"; | |
| import AsyncStorage from "@react-native-async-storage/async-storage"; | |
| // …other imports… | |
| // Avoid double-initialization across HMR/fast refresh | |
| const getOrInitAuth = () => { | |
| if (!globalThis.__FIREBASE_AUTH_INITIALIZED__) { | |
| const inst = initializeAuth(app, { | |
| persistence: getReactNativePersistence(AsyncStorage), | |
| }); | |
| globalThis.__FIREBASE_AUTH_INITIALIZED__ = true; | |
| return inst; | |
| } | |
| return getAuth(app); | |
| }; | |
| const auth = getOrInitAuth(); | |
| // …rest of your config… |
🤖 Prompt for AI Agents
In frontend/firebase/firebaseConfig.native.js around lines 18 to 21, calling
initializeAuth multiple times causes errors during hot module reload or fast
refresh. To fix this, add a guard that checks if an auth instance already
exists; if it does, use getAuth(app) instead of calling initializeAuth again.
This prevents re-initialization errors by reusing the existing auth instance.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/package.json (1)
36-39: Moving EAS CLI to devDependencies is correctThanks for addressing the earlier note. Types and TS entries also look fine.
🧹 Nitpick comments (1)
frontend/package.json (1)
20-23: expo-dev-client added — align docs and local dev flow; secure token storage guidance
- Good add for testing custom native modules. Make sure docs in the PR mention running “npx expo prebuild” and installing the dev client on devices/simulators.
- For expo-secure-store: store only what’s needed (avoid long-term refresh/ID tokens), use the highest protection levels (e.g., requireAuthentication on iOS when appropriate), and provide a fallback for platforms where hardware-backed storage isn’t available.
Can you add 2–3 lines in the README/PR description:
- How to run locally with the dev client (Android/iOS).
- What is persisted (which tokens/claims) and the chosen SecureStore options.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
frontend/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: bundle-analysis
🔇 Additional comments (2)
frontend/package.json (2)
13-14: Google Sign-In native module requires app config and EAS prebuild/build steps
- Ensure app.json/app.config includes required Android/iOS config (URL schemes, REVERSED_CLIENT_ID, client IDs) and Google plist/json are wired via config plugins.
- With Expo, this module needs a custom dev client or a standalone build; it won’t work in Expo Go.
Please confirm:
- app.json has the correct Android intent filter and iOS URL types for your OAuth client.
- google-services.json and GoogleService-Info.plist are present and referenced (or added via credentials in EAS).
- You plan to use expo run or EAS Build (not Expo Go) for testing native Google Sign-In.
25-26: Firebase SDK on React Native: import paths and persistence
- With RN/Expo, ensure you import from firebase/app and firebase/auth (no custom “react-native” subpath) and set persistence using getReactNativePersistence(AsyncStorage) so sessions survive restarts.
- Double-check Web vs Native code paths for Google sign-in (web typically uses popup/redirect via auth session; native uses GoogleSignIn + signInWithCredential).
Please confirm your Auth initialization includes React Native persistence with @react-native-async-storage/async-storage and that web/native flows are split correctly.
| "react-native-safe-area-context": "5.4.0", | ||
| "react-native-screens": "~4.11.1", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify compatibility of downgraded navigation peers with Expo SDK 53
react-native-safe-area-context 5.4.0 and react-native-screens ~4.11.1 must match the versions Expo 53 and React Navigation 7 expect. Mismatches can cause runtime crashes.
If versions don’t match, run: npx expo install to auto-align peer versions.
🌐 Web query:
Which react-native-screens and react-native-safe-area-context versions are compatible with Expo SDK 53?
💡 Result:
For Expo SDK 53, the compatible versions are:
- react-native-screens: ^4.13.1 has been successfully used with SDK 53 and React Native 0.79.5[1]. Installation via
npx expo install react-native-screenswill match you with a compatible version[3]. - react-native-safe-area-context: Version 5.4.0 is the version required/expected by core navigation libraries (e.g.,
@react-navigation/bottom-tabs) starting with SDK 53[5]. Installing vianpx expo install react-native-safe-area-contextensures you get the version Expo considers compatible[8].
Key points:
- Using older versions (e.g., safe-area-context 4.12.0) with Expo SDK 53 leads to dependency conflicts and installation errors because navigation libraries expect 5.x[5].
- Newer strictness in React Native 0.79 package resolution makes correct versioning particularly important[10].
- Upgrading all Expo and related packages with
npx expo install --fixis strongly recommended after bumping Expo SDK[10].
Summary table:
| Library | Compatible Version (Expo SDK 53) | How to install |
|---|---|---|
| react-native-screens | ^4.13.1 (auto-picked by Expo) | npx expo install react-native-screens |
| react-native-safe-area-context | 5.4.0 (required by navigation libs) | npx expo install react-native-safe-area-context |
Additional Notes:
- Always run
npx expo-doctorafter upgrading to check for incompatibilities[10]. - If you encounter issues on install, check your navigation packages are up-to-date and match their peer dependency requirements[5].
These guidelines reflect best practices and real-world reports from Expo SDK 53 upgrade experiences as of August 2025.
Citations:
- 1: ReactFabric crash with Hermes after upgrading to Expo 53 / React Native 0.79.5 software-mansion/react-native-screens#3041
- 2: [Expo Go for Android] <SafeAreaView/> from "react-native-safe-area-context" error on SDK 53 expo/expo#36566
- 3: https://docs.expo.dev/versions/latest/sdk/screens/
- 4: https://expo.dev/changelog/sdk-53
- 5: react-native-safe-area-context Error expo/expo#36604
- 6: (upgrade from sdk 52 to expo sdk 53) The package at "node_modules\ws\lib\stream.js" attempted to import the Node standard library module "stream". expo/expo#36477
- 7: https://www.youtube.com/watch?v=x-nhJcKYfOo
- 8: https://docs.expo.dev/versions/latest/sdk/safe-area-context/
- 9: [docs] Upgrading to Expo 53 requires react-native-web expo/expo#36897
- 10: https://blog.logrocket.com/expo-sdk-53-checklist/
Update react-native-screens to a version compatible with Expo SDK 53
Verified that for Expo SDK 53:
- react-native-safe-area-context 5.4.0 is correct.
- react-native-screens should be at least ^4.13.1 (not ~4.11.1) to satisfy SDK 53 and React Native 0.79.5.
Please update your frontend/package.json and align with Expo’s peer deps:
• File: frontend/package.json
Line 32
Diff:
- "react-native-screens": "~4.11.1",
+ "react-native-screens": "^4.13.1",After updating, run:
npx expo install react-native-screens
npx expo-doctorto ensure all Expo dependencies resolve correctly.
🤖 Prompt for AI Agents
In frontend/package.json at line 32, update the version of react-native-screens
from "~4.11.1" to "^4.13.1" to ensure compatibility with Expo SDK 53 and React
Native 0.79.5. After making this change, run "npx expo install
react-native-screens" and "npx expo-doctor" to verify that all Expo dependencies
are correctly resolved.
|
|
Nope |
|
@meghan9908 can we setup this over a meet? |
|
Yes @Devasy23 ,what time |
|
Now would work? |
|
Yes send me the meet link |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/preview.yml (1)
40-45: Gate secret-using preview on forked PRs (no secrets in forks) or a maintainer labelForked PRs don’t get access to EXPO_TOKEN or other repo secrets; this step will fail. Gate it to avoid red X’s and potential secret exposure risks.
Apply this diff to the preview step:
- name: Create preview uses: expo/expo-github-action/preview@v8 + if: ${{ github.event.pull_request.head.repo.fork == false || contains(github.event.pull_request.labels.*.name, 'safe-to-preview') }} with: command: eas update --auto --branch ${{ github.event.pull_request.head.ref }} working-directory: ./frontendAlso consider adding the same guard to the “Setup EAS” step that uses EXPO_TOKEN (Lines 22–27) to skip configuring EAS for forked PRs.
🧹 Nitpick comments (1)
frontend/app.json (1)
27-29: Android package and googleServicesFile — validate Firebase config matches
- Confirm google-services.json’s package_name equals com.splitwiser.app.
- Ensure Firebase console has the correct SHA-1/SHAs for your keystores (debug/dev build fingerprints) to enable Google Sign-In on Android.
- For public repos, validate you’re comfortable committing google-services.json (API keys should be restricted).
I can generate a checklist/script to verify package-name and SHA-1 alignment across files if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/preview.yml(1 hunks)frontend/app.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: bundle-analysis
🔇 Additional comments (7)
.github/workflows/preview.yml (1)
32-38: Confirm these are truly EXPO_PUBLIC_ (safe-by-design) variables*Using EXPO_PUBLIC_* is correct: they’re intended to be embedded client-side and aren’t secrets. Just double-check none of these values are private server credentials.
frontend/app.json (6)
3-5: Deep link scheme added — ensure OAuth redirect URIs matchWith scheme “splitwiser”, your redirect should resemble splitwiser:/oauthredirect. Verify your Google OAuth flow (web/native) uses makeRedirectUri({ scheme: 'splitwiser' }) and that any configured URIs in Google Cloud Console align.
3-5: Renaming name/slug — verify EAS project linkage and channelsChanging name/slug is fine (EAS uses extra.eas.projectId), but confirm:
- The app remains linked to the same EAS project (projectId unchanged).
- Existing Update channels aren’t affected unintentionally if you used classic flows previously.
9-9: User interface style to automatic — LGTMAdaptive theming is a good default for modern devices.
10-10: newArchEnabled set to false — confirm intent (possible perf/regression tradeoff)If New Architecture was previously enabled, disabling it may impact performance and library compatibility. Confirm this was necessary (e.g., due to a dependency incompatibility).
12-12: Splash asset path changed — verify asset presenceEnsure ./assets/splash.png exists and matches recommended dimensions to prevent build-time failures.
34-37: Plugins added (expo-secure-store, expo-web-browser) — LGTMBoth are appropriate for OAuth and token storage. No additional config needed in app.json beyond this.
| - name: Create .env file | ||
| working-directory: ./frontend | ||
| run: | | ||
| echo "EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID }}" >> .env | ||
| echo "EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID }}" >> .env | ||
| echo "EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID }}" >> .env | ||
| echo "EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID }}" >> .env |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid relying on .env in CI; inject EXPO_PUBLIC_ via step env for EAS Update*
EAS Update doesn’t reliably read a local .env unless you wire a loader; safest is to pass vars via the step’s env so the bundler inlines EXPO_PUBLIC_* at build time.
Apply this diff to drop the .env creation:
- - name: Create .env file
- working-directory: ./frontend
- run: |
- echo "EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID }}" >> .env
- echo "EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID }}" >> .env
- echo "EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID }}" >> .env
- echo "EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID=${{ secrets.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID }}" >> .envThen, add these envs to the “Create preview” step so they’re available at bundle time:
- name: Create preview
uses: expo/expo-github-action/preview@v8
with:
command: eas update --auto --branch ${{ github.event.pull_request.head.ref }}
working-directory: ./frontend
env:
EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID: ${{ secrets.EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID }}
EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID: ${{ secrets.EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID }}
EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID: ${{ secrets.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID }}
EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID: ${{ secrets.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID }}🤖 Prompt for AI Agents
.github/workflows/preview.yml lines 32-38: the workflow writes EXPO_PUBLIC_*
values into a local .env which EAS Update may not load at bundle time; remove
the block that creates the .env and instead add the four EXPO_PUBLIC_GOOGLE_*
secrets as environment variables on the "Create preview" step so they are
present in process env during the eas update/bundle step. Specifically delete
the "Create .env file" run step and add env: entries for
EXPO_PUBLIC_GOOGLE_EXPO_CLIENT_ID, EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID,
EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID and EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID
(sourced from the respective secrets) under the "Create preview" step.
2 participants
Google Sign-In button is available on authentication screens
Users can successfully authenticate using Google OAuth
User profiles are created/updated from Google account information
OAuth tokens are securely stored and managed
Error handling covers OAuth failures and network issues
Working on both android and web seamlessly #108
Summary by CodeRabbit
New Features
Improvements
Chores
Bug Fixes