RDS Mobile app migration from CLI to Expo #486
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
These enhancements deliver a smoother sign-in process and a more intuitive, visually appealing interface. WalkthroughThis update introduces a comprehensive set of new configuration files and source code additions. It adds gitignore files, Android build configurations (Gradle, wrapper, properties, settings), Android manifests, and Kotlin classes for the main activity and application. Several Android resource files (drawable, mipmap, colors, strings, and styles) are provided to support theming and UI. New Expo and TypeScript configuration files are included alongside multiple React Native components for authentication, tab navigation, and profile management. Additionally, constants for API endpoints and color schemes have been defined, and the previous Metro configuration file has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant AS as AuthScreen
participant AT as AsyncStorage
participant R as Router
U->>AS: Launch App / Open AuthScreen
AS->>AT: Check for token (github_token)
AT-->>AS: Return token (or null)
alt Token exists
AS->>R: Redirect to HomeScreen
else No token
U->>AS: Tap "GitHub Sign In"
AS->>AS: Build GitHub auth URL
AS->>U: Open Browser with auth URL
U->>AS: Return via deep link with token
AS->>AT: Store github_token
AS->>R: Redirect to HomeScreen
end
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
🔭 Outside diff range comments (4)
RDSExpoApp/app/(tabs)/HomeScreen.tsx (1)
22-23: 🧹 Nitpick (assertive)Remove trailing space
There's an unnecessary trailing space at the end of the file.
}); -23 +RDSExpoApp/app/(tabs)/NotifyScreen.tsx (1)
22-23: 🧹 Nitpick (assertive)Remove trailing space
There's an unnecessary trailing space at the end of the file.
}); -23 +RDSExpoApp/constants/Colors.ts (1)
19-20: 🧹 Nitpick (assertive)Remove trailing space
There's an unnecessary trailing space at the end of the file.
}; -20 +RDSExpoApp/app/(tabs)/index.tsx (1)
1-148: 🛠️ Refactor suggestionConsider implementing comprehensive error handling and loading states
The authentication flow would benefit from:
- Loading indicators during async operations
- User-facing error messages
- Retry mechanisms for failed authentication attempts
- More comprehensive validation of the received tokens
These additions would significantly improve the reliability and user experience of the authentication process.
🧰 Tools
🪛 GitHub Check: build (18.x)
[warning] 100-100:
Inline style: { borderRadius: 12, marginTop: 20 }
[warning] 92-92:
Inline style: { borderRadius: 12 }
[failure] 73-73:
React Hook useEffect has missing dependencies: 'checkUserSession' and 'handleTokenFromUrl'. Either include them or remove the dependency array🪛 GitHub Actions: Continous-Integeration
[error] 73-73: React Hook useEffect has missing dependencies: 'checkUserSession' and 'handleTokenFromUrl'. Either include them or remove the dependency array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (15)
RDSExpoApp/.yarn/install-state.gzis excluded by!**/.yarn/**,!**/*.gzRDSExpoApp/android/app/src/main/res/drawable-hdpi/splashscreen_logo.pngis excluded by!**/*.pngRDSExpoApp/android/app/src/main/res/drawable-mdpi/splashscreen_logo.pngis excluded by!**/*.pngRDSExpoApp/android/app/src/main/res/drawable-xhdpi/splashscreen_logo.pngis excluded by!**/*.pngRDSExpoApp/android/app/src/main/res/drawable-xxhdpi/splashscreen_logo.pngis excluded by!**/*.pngRDSExpoApp/android/app/src/main/res/drawable-xxxhdpi/splashscreen_logo.pngis excluded by!**/*.pngRDSExpoApp/android/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarRDSExpoApp/assets/fonts/SpaceMono-Regular.ttfis excluded by!**/*.ttfRDSExpoApp/assets/images/adaptive-icon.pngis excluded by!**/*.pngRDSExpoApp/assets/images/favicon.pngis excluded by!**/*.pngRDSExpoApp/assets/images/icon.pngis excluded by!**/*.pngRDSExpoApp/assets/images/rdsLogo.pngis excluded by!**/*.pngRDSExpoApp/assets/images/splash-icon.pngis excluded by!**/*.pngRDSExpoApp/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
RDSExpoApp/.gitignore(1 hunks)RDSExpoApp/android/.gitignore(1 hunks)RDSExpoApp/android/app/build.gradle(1 hunks)RDSExpoApp/android/app/proguard-rules.pro(1 hunks)RDSExpoApp/android/app/src/debug/AndroidManifest.xml(1 hunks)RDSExpoApp/android/app/src/main/AndroidManifest.xml(1 hunks)RDSExpoApp/android/app/src/main/java/com/abstrctdatyp/RDSExpoApp/MainActivity.kt(1 hunks)RDSExpoApp/android/app/src/main/java/com/abstrctdatyp/RDSExpoApp/MainApplication.kt(1 hunks)RDSExpoApp/android/app/src/main/res/drawable/ic_launcher_background.xml(1 hunks)RDSExpoApp/android/app/src/main/res/drawable/rn_edit_text_material.xml(1 hunks)RDSExpoApp/android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml(1 hunks)RDSExpoApp/android/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml(1 hunks)RDSExpoApp/android/app/src/main/res/values-night/colors.xml(1 hunks)RDSExpoApp/android/app/src/main/res/values/colors.xml(1 hunks)RDSExpoApp/android/app/src/main/res/values/strings.xml(1 hunks)RDSExpoApp/android/app/src/main/res/values/styles.xml(1 hunks)RDSExpoApp/android/build.gradle(1 hunks)RDSExpoApp/android/gradle.properties(1 hunks)RDSExpoApp/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)RDSExpoApp/android/gradlew(1 hunks)RDSExpoApp/android/gradlew.bat(1 hunks)RDSExpoApp/android/settings.gradle(1 hunks)RDSExpoApp/app.json(1 hunks)RDSExpoApp/app/(tabs)/HomeScreen.tsx(1 hunks)RDSExpoApp/app/(tabs)/NotifyScreen.tsx(1 hunks)RDSExpoApp/app/(tabs)/ProfileScreen.tsx(1 hunks)RDSExpoApp/app/(tabs)/_layout.tsx(1 hunks)RDSExpoApp/app/(tabs)/index.tsx(1 hunks)RDSExpoApp/app/_layout.tsx(1 hunks)RDSExpoApp/assets/svgs/github_logo.js(1 hunks)RDSExpoApp/constants/Colors.ts(1 hunks)RDSExpoApp/constants/apiConstant/AuthApi.ts(1 hunks)RDSExpoApp/constants/apiConstant/BaseUrl.ts(1 hunks)RDSExpoApp/package.json(1 hunks)RDSExpoApp/tsconfig.json(1 hunks)metro.config.js(0 hunks)
💤 Files with no reviewable changes (1)
- metro.config.js
🧰 Additional context used
🧬 Code Definitions (1)
RDSExpoApp/constants/apiConstant/AuthApi.ts (1)
RDSExpoApp/constants/apiConstant/BaseUrl.ts (1)
STAGING_BASE_URL(3-3)
🪛 GitHub Check: build (18.x)
RDSExpoApp/app/(tabs)/_layout.tsx
[warning] 52-52:
Inline style: { marginBottom: -3 }
[warning] 47-47:
Do not define components during render. React will see a new component type on every render and destroy the entire subtree’s DOM nodes and state (https://reactjs.org/docs/reconciliation.html#elements-of-different-types). Instead, move this component definition out of the parent component “TabLayout” and pass data as props. If you want to allow component creation in props, set allowAsProps option to true
[warning] 36-36:
Inline style: { marginBottom: -3 }
[warning] 31-31:
Do not define components during render. React will see a new component type on every render and destroy the entire subtree’s DOM nodes and state (https://reactjs.org/docs/reconciliation.html#elements-of-different-types). Instead, move this component definition out of the parent component “TabLayout” and pass data as props. If you want to allow component creation in props, set allowAsProps option to true
RDSExpoApp/app/(tabs)/index.tsx
[warning] 100-100:
Inline style: { borderRadius: 12, marginTop: 20 }
[warning] 92-92:
Inline style: { borderRadius: 12 }
[failure] 73-73:
React Hook useEffect has missing dependencies: 'checkUserSession' and 'handleTokenFromUrl'. Either include them or remove the dependency array
🪛 GitHub Actions: Continous-Integeration
RDSExpoApp/app/(tabs)/_layout.tsx
[warning] 31-31: Do not define components during render. React will see a new component type on every render and destroy the entire subtree’s DOM nodes and state. Instead, move this component definition out of the parent component 'TabLayout' and pass data as props.
[warning] 36-36: Inline style: { marginBottom: -3 }
[warning] 47-47: Do not define components during render. React will see a new component type on every render and destroy the entire subtree’s DOM nodes and state. Instead, move this component definition out of the parent component 'TabLayout' and pass data as props.
[warning] 52-52: Inline style: { marginBottom: -3 }
[warning] 63-63: Do not define components during render. React will see a new component type on every render and destroy the entire subtree’s DOM nodes and state. Instead, move this component definition out of the parent component 'TabLayout' and pass data as props.
[warning] 68-68: Inline style: { marginBottom: -3 }
RDSExpoApp/app/(tabs)/index.tsx
[error] 73-73: React Hook useEffect has missing dependencies: 'checkUserSession' and 'handleTokenFromUrl'. Either include them or remove the dependency array.
🔇 Additional comments (65)
RDSExpoApp/assets/svgs/github_logo.js (2)
1-2: Good choice of using react-native-svg for SVG rendering.Using
react-native-svgis the recommended approach for rendering SVGs in React Native applications, which aligns well with the Expo migration objective.
20-20: LGTM - Clean export pattern.The default export of the component follows the standard React pattern and makes the component easy to import.
RDSExpoApp/android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml (1)
1-5: Adaptive Icon XML is Configured Correctly: The adaptive icon is well defined with clear background and foreground references. Please ensure that the resource names (@color/iconBackgroundand@mipmap/ic_launcher_foreground) are defined elsewhere in the project.RDSExpoApp/android/app/src/main/res/drawable/ic_launcher_background.xml (1)
1-6: Layer-list Drawable Defined Properly: The file correctly creates a composite drawable by combining a solid color from@color/splashscreen_backgroundwith a centered bitmap (@drawable/splashscreen_logo). Verify that these referenced resources exist and meet the design requirements.RDSExpoApp/android/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml (1)
1-5: Round Adaptive Icon XML is Correct: The round adaptive icon mirrors the standard adaptive icon structure, correctly referencing the background and foreground resources. Confirm that@color/iconBackgroundand@mipmap/ic_launcher_foregroundare available in your project resources.RDSExpoApp/android/.gitignore (1)
1-17: Comprehensive .gitignore for Android: The.gitignorefile appropriately excludes common OS-specific files, Android/IntelliJ build artifacts, local configuration files, and bundle artifacts. This should help keep the repository clean from unnecessary files.RDSExpoApp/.gitignore (1)
1-37: Well-Structured .gitignore File
The file is comprehensive and clearly lists the files, directories, and file patterns that should be ignored. It covers essential dependency folders (likenode_modules/), Expo artifacts, native build files, debugging logs, macOS system files, and TypeScript build outputs. This will help keep the repository clean and reduce clutter.RDSExpoApp/tsconfig.json (1)
1-20: Solid TypeScript Configuration
Thetsconfig.jsoncorrectly extends Expo’s base configuration and sets strict compiler options along with an appropriate path alias (@/*). The inclusion of all relevant TypeScript and type definition files ensures that the project is well-covered during compilation.RDSExpoApp/android/build.gradle (1)
1-42: Robust Android Gradle Configuration
The Gradle file is well-structured and aligns with the requirements for a React Native Expo project on Android. It correctly establishes thebuildscriptenvironment with externalized properties and includes the necessary repositories and dependencies. A couple of points for consideration:
- Versioning: The dependency declarations for the Android Gradle plugin, React Native Gradle plugin, and Kotlin plugin do not include explicit version numbers. If these versions are managed elsewhere (e.g., via a gradle wrapper or external properties), this approach is acceptable; otherwise, adding version numbers might help ensure build consistency.
- Dynamic Node Resolution: The use of dynamic resolution (via Node commands) in the repository URLs is clever but can be fragile in environments where Node isn’t available or configured in the system PATH. Please verify this works consistently across all build environments.
RDSExpoApp/android/app/proguard-rules.pro (1)
1-15: Effective ProGuard Rules Setup
This ProGuard configuration file appropriately preserves the necessary classes forreact-native-reanimatedand React Native TurboModules, ensuring that these critical packages are not obfuscated or stripped during the build process. This setup is in line with the requirements for a React Native Expo Android project. If additional libraries are integrated in the future, remember to review and potentially extend these rules accordingly.RDSExpoApp/android/app/src/debug/AndroidManifest.xml (1)
1-8: Review Note: Validate Debug Manifest Configuration
The manifest correctly declares the SYSTEM_ALERT_WINDOW permission and sets cleartext traffic for the debug build. Verify that the use of cleartext traffic (viaandroid:usesCleartextTraffic="true") is intended only for development and that a more secure configuration is enforced in production.RDSExpoApp/android/app/src/main/res/values/styles.xml (1)
1-19: Review Note: Ensure Consistent Theming
The styles resource defines key themes includingAppTheme,ResetEditText, andTheme.App.SplashScreenconsistently. Please double-check that hard-coded values (e.g.,#fffffffor the status bar) fit your design guidelines, including potential support for dark mode if needed.RDSExpoApp/android/gradle/wrapper/gradle-wrapper.properties (1)
1-7: Review Note: Gradle Wrapper Settings
The gradle-wrapper properties are set to use Gradle version 8.10.2 with a 10-second network timeout. Ensure that this version is compatible with your React Native/Expo toolchain and that these settings meet your build environment requirements.RDSExpoApp/android/app/src/main/res/values/colors.xml (1)
1-6: Review Note: Color Resource Definitions
The colors resource file neatly centralizes key color definitions. Confirm that the chosen colors (e.g., using white forsplashscreen_backgroundandcolorPrimaryDark) align with your app’s design requirements.RDSExpoApp/android/settings.gradle (1)
1-39:Details
❓ Verification inconclusive
Review Note: Dynamic Gradle Configuration
This settings file leverages dynamic Node.js command executions to resolve paths for the React Native Gradle plugin and Expo modules. While this approach streamlines configuration, please ensure:
- These commands consistently execute across different environments (local, CI, etc.).
- Consideration for error handling or fallback strategies if Node is unavailable or if resolution fails.
Additionally, verify that the autolinking commands and dependency resolution management integrate seamlessly with the rest of the Android build process.
Action Required: Verify Robustness of Dynamic Node Execution in Gradle Settings
The settings file employs dynamic Node.js command executions to resolve paths for the React Native Gradle plugin and Expo modules. Please verify the following:
- That these Node-based commands execute consistently across environments (local, CI, etc.).
- That any potential failures (e.g., if Node isn’t available or a resolution fails) are gracefully handled—consider adding explicit error handling or fallback strategies.
- That the autolinking command and dependency resolution management integrate correctly within the overall Android build process.
RDSExpoApp/android/app/src/main/AndroidManifest.xml (10)
1-7: Permissions and Manifest Declaration are Correct.
All the necessary permissions (INTERNET, external storage, etc.) are declared properly.
8-14: Queries Block is Implemented as Expected.
The intent query for the HTTPS scheme is correctly specified. Verify later if additional hosts might be needed.
16-23: Application Element is Well Configured.
The application element includes appropriate meta-data (Expo update flags), theme, icon, and RTL support.
29-36: MainActivity Declaration is Comprehensive.
Configuration settings like launch mode, configChanges, windowSoftInputMode, and portrait orientation properly address the requirements of a React Native/Expo app.
38-41: Launcher Intent Filter is Standard.
Declaring ACTION_MAIN with CATEGORY_LAUNCHER ensures proper app startup.
43-49: OAuth Redirect Filter Review.
The intent filter intended for OAuth redirection uses the custom scheme "rdsapp" and host "ProfileScreen". Please verify that this matches the deep linking and routing setup in your Expo configuration.
51-57: Additional Deep-Link Filter for 'rdsapp' Scheme.
The intent filter capturing all "rdsapp" links (without host restrictions) supports development scenarios.
70-78: AutoVerify Set to False for 'realdevsquad.com'.
Intent filter for "realdevsquad.com" is configured with autoVerify="false". Confirm that this behavior is desired in your migration context.
80-88: AutoVerify Intent Filter for 'rdsapp.netlify.app' is Configured Properly.
Enabling autoVerify for "rdsapp.netlify.app" seems to be in line with domain verification strategies.
91-93: Manifest Structure Closes Correctly.
The closing tags are properly formatted and the overall structure adheres to standard Android practices.RDSExpoApp/android/app/src/main/res/drawable/rn_edit_text_material.xml (4)
1-15: XML Declaration and License Block are Standard.
The file begins with the XML declaration and includes the complete Apache 2.0 license header.
16-21: Inset Element is Defined Correctly.
The inset element uses dimension resources appropriately to set padding values.
23-36: Selector Configuration Addresses Known Issue.
The commented-out item explains the NullPointerException issue well. The included items (for disabled and default states) should cover the drawable state requirements.
37-38: XML Document Closes Properly.
The closing tag maintains the document’s integrity.RDSExpoApp/android/app/src/main/res/values/strings.xml (1)
1-6: String Resources are Appropriately Defined.
The strings for the application name and splash screen configurations (including non-translatable attributes) are correctly set.RDSExpoApp/android/app/build.gradle (6)
1-4: Plugins are Applied Correctly.
The Android, Kotlin, and React Native plugins are included as required.
5-5: Project Root Calculation is Sound.
The computation of the project root using Groovy’s file APIs seems robust for the intended directory structure.
11-63: Comprehensive React Native Configuration Block.
The use of node commands to resolve paths (entry file, reactNativeDir, hermesCommand, and codegenDir) reflects common patterns in Expo projects. Consider adding error handling if any of these command executions fail.
83-127: Android Block is Configured Standardly.
SDK versions, namespace, applicationId, and build types are defined as expected.
Note: The release build currently uses the debug signing configuration (line 112). Make sure to update this before production.
129-147: Dynamic Packaging Options are Creatively Handled.
The logic for splitting and trimming packaging options from Gradle properties is clever. Please verify that it behaves reliably across different environments and configurations.
149-177: Dependencies Block Manages Optional Features Well.
Conditional inclusion for GIF and WebP support—and the choice between Hermes and JavaScriptCore—are handled properly based on project properties.RDSExpoApp/android/gradlew (9)
1-20: Gradle Wrapper Script Header is Standard.
The shebang, licensing information, and SPDX identifier are in line with the Gradle wrapper conventions.
67-83: Robust APP_HOME Resolution Logic.
The loop correctly handles daisy-chained symbolic links to set the application path reliably.
85-91: APP_HOME Canonicalization is Handled Properly.
Usingcd -Pand capturing the present working directory ensures a correct absolute path; this is a standard and dependable technique.
95-104: Error Handling Functions are Well Defined.
The customwarnanddiefunctions provide clear and user-friendly error messages, which is good for shell scripting.
121-144: Java Command Resolution is Handled Carefully.
The script checks for the presence ofJAVA_HOMEand correctly distinguishes IBM’s JDK layout. This fallback mechanism is standard practice.
146-163: File Descriptor Limit Adjustment is Logical.
Usingulimitto query and set file descriptor limits is a good practice. Ensure that the chosen MAX_FD value is appropriate on all target platforms.
214-218: Java Command Argument Assembly is Clear and Secure.
The aggregation of JVM options, classpath, main class, and command-line arguments is well done, ensuring proper quoting and sequence.
220-224: xargs Availability Check is Prudent.
Confirming thatxargsis available helps avoid runtime errors during argument parsing.
252-253: Final Execution is Standard.
The script’s final exec call to invoke the resolved Java command with all arguments is standard for Gradle wrapper scripts.RDSExpoApp/package.json (2)
1-46: Well-configured package.json for Expo React Native app.The package.json is properly configured with appropriate dependencies for an Expo React Native application. The use of specific versioning for packages helps ensure compatibility.
16-34:Details
✅ Verification successful
Verify AsyncStorage version compatibility.
The AsyncStorage version (1.23.1) is quite recent. Make sure it's fully compatible with the rest of your dependencies.
🏁 Script executed:
#!/bin/bash # Check compatibility of AsyncStorage version with the React Native version echo "Checking compatibility of @react-native-async-storage/async-storage 1.23.1 with React Native 0.76.7" npm view @react-native-async-storage/async-storage@1.23.1 peerDependenciesLength of output: 331
AsyncStorage Compatibility Confirmed
The output shows that@react-native-async-storage/async-storage@1.23.1specifies a peer dependency constraint of'react-native': '^0.0.0-0 || >=0.60 <1.0'. Since the current React Native version is 0.76.7—well within the supported range—no further action is needed for this dependency.RDSExpoApp/android/gradlew.bat (1)
1-95: Standard Gradle wrapper file is correctly included.This is a standard Gradle wrapper batch script for Windows environments. It's properly configured with adequate error handling and execution setup.
RDSExpoApp/android/app/src/main/java/com/abstrctdatyp/RDSExpoApp/MainActivity.kt (3)
36-45: New Architecture configuration is correct.The ReactActivityDelegate is properly configured with the new architecture flag and fabricEnabled for Fabric renderer support.
52-64: Good back button behavior implementation.The back button behavior is correctly customized to match Android S (API 31+) behavior on older versions, maintaining consistent UX across different Android versions.
14-24:Details
❓ Verification inconclusive
Check commented theme configuration.
There's a commented-out theme configuration on line 19. Verify if this is intentional or if the theme should be properly set for the splash screen.
🏁 Script executed:
#!/bin/bash # Check if AppTheme is defined in the styles.xml file if [ -f "RDSExpoApp/android/app/src/main/res/values/styles.xml" ]; then echo "Checking for AppTheme in styles.xml" cat "RDSExpoApp/android/app/src/main/res/values/styles.xml" | grep -A 10 "AppTheme" else echo "styles.xml not found to verify AppTheme" fiLength of output: 955
Action: Verify the theme configuration in MainActivity
We confirmed that the AppTheme is properly defined in styles.xml. However, the call to
setTheme(R.style.AppTheme)remains commented out in MainActivity.kt. Please verify if this was an intentional decision—perhaps because the expo-splash-screen integration now handles the theme automatically—or if the theme needs to be explicitly applied during onCreate. If explicit theming is required for proper splash screen behavior, consider uncommenting the line. Otherwise, removing the commented code could help avoid confusion.RDSExpoApp/android/gradle.properties (4)
10-13: Appropriate memory settings for Gradle daemon.The JVM memory settings are properly increased for better build performance, which is especially important for larger React Native projects.
40-42: Hermes is properly enabled.Hermes JS engine is enabled, which is good for performance and memory usage in React Native applications.
44-50: Image format support is well configured.Support for GIF and static WebP is enabled, while animated WebP is disabled (which is appropriate due to iOS limitations).
33-38:Details
❓ Verification inconclusive
New Architecture is enabled by default.
The new architecture is enabled by default, which is forward-looking but may introduce some compatibility issues with certain native modules that haven't been updated yet.
🏁 Script executed:
#!/bin/bash # Check for dependencies that might have issues with the new architecture if [ -f "RDSExpoApp/package.json" ]; then echo "Checking native dependencies that might need verification with new architecture:" cat "RDSExpoApp/package.json" | grep -A 20 "dependencies" | grep "react-native-" | cut -d'"' -f2 else echo "package.json not found to verify native dependencies" fiLength of output: 435
Action Required: Review Native Module Compatibility for New Architecture
The configuration in
RDSExpoApp/android/gradle.propertiesnow setsnewArchEnabled=trueby default to support TurboModules and Fabric rendering. This forward-looking change is great for enabling future features, but it may lead to compatibility issues with certain native modules. Notably, yourpackage.jsonshows dependencies such as:
@react-native-async-storage/async-storagereact-native-reanimatedreact-native-safe-area-contextreact-native-screensreact-native-webPlease verify that these dependencies have been tested and confirmed to function correctly under the new architecture. If any of these native modules exhibit issues, you might need to delay enabling this setting or apply patch updates where available.
RDSExpoApp/android/app/src/main/java/com/abstrctdatyp/RDSExpoApp/MainApplication.kt (5)
21-38: LGTM: ReactNativeHost implementation is correctly configured for Expo.The implementation uses
ReactNativeHostWrapperfrom Expo modules and properly configures the JavaScript entry point to.expo/.virtual-metro-entry, which is required for Expo applications. The packages setup also allows for manual additions if needed.
40-41: LGTM: ReactHost implementation uses the recommended approach.The implementation correctly uses
ReactNativeHostWrapper.createReactHost()to create a React host from thereactNativeHost, which is the proper pattern for Expo applications.
43-51: LGTM: Application lifecycle initialization is properly implemented.The
onCreate()method correctly initializes SoLoader, conditionally loads the native entry point for the new architecture, and properly notifies Expo's lifecycle dispatcher.
53-56: LGTM: Configuration changes are handled appropriately.The
onConfigurationChanged()method properly calls the superclass implementation and notifies Expo's lifecycle dispatcher, which is necessary for Expo to handle configuration changes correctly.
31-31:Details
❓ Verification inconclusive
Verify that the JS main module path is correct.
The JS main module name is set to
.expo/.virtual-metro-entry. Confirm that this path aligns with the Expo configuration in your project, as incorrect paths can lead to app launch failures.
🏁 Script executed:
#!/bin/bash # Check if the entry file exists in the project if [ -d ".expo" ]; then echo "Checking .expo directory structure..." find .expo -name ".virtual-metro-entry*" -type f | sort else echo ".expo directory not found at the root level" # Try to find it elsewhere in the project find . -name ".expo" -type d | xargs -I{} find {} -name ".virtual-metro-entry*" -type f fi # Check the app.json or app.config.js for related configuration if [ -f "app.json" ]; then echo "Checking app.json for entryPoint configuration..." grep -A 3 "entryPoint" app.json fi if [ -f "app.config.js" ]; then echo "Checking app.config.js for entryPoint configuration..." grep -A 3 "entryPoint" app.config.js fi # Look for metro.config.js if [ -f "metro.config.js" ]; then echo "Checking metro.config.js for relevant configuration..." cat metro.config.js fiLength of output: 426
Action: Confirm the JS main module path configuration.
After reviewing the project structure and configuration, the script output indicates that the
.expodirectory wasn’t found at the repository root and no.virtual-metro-entryfile was detected. Additionally, the Expo configuration (e.g. viaentryPointinapp.json) doesn’t appear to override the default. Please verify that your project either generates or relocates the expected.expo/.virtual-metro-entryfile at runtime. If your Expo setup uses a custom configuration or build process that handles this differently, ensure that the configuration aligns with the expected module path.
- Confirm that the build process generates the
.expofolder (and the.virtual-metro-entryfile) in the correct location.- Double-check the Expo configuration (e.g. in
app.json) to ensure there’s no conflictingentryPointsetting.RDSExpoApp/app/_layout.tsx (1)
19-36: LGTM: Well-structured component with proper font loading and error handlingThe RootLayout component has good error handling for font loading and properly manages the splash screen. The use of useEffect hooks to handle errors and hide the splash screen is well implemented.
RDSExpoApp/app/(tabs)/index.tsx (4)
15-20: LGTM - Well-implemented URL builder utilityThe
buildUrlfunction correctly handles URL construction with query parameters, properly encoding parameter values.
75-79: LGTM - handleSignIn function implementationThe implementation correctly opens the GitHub authentication URL and handles potential errors.
106-147: LGTM - Well-organized StyleSheetThe StyleSheet is well-organized with clear naming conventions. Consider adding the missing styles for the buttons as mentioned in the previous comment.
91-94:Details
❓ Verification inconclusive
Differentiate between GitHub and Web sign-in functionalities
Both buttons currently use the same
handleSignInfunction, which only opens the GitHub authentication URL. If they're meant to have different behaviors, they should use different handler functions.Also, move inline styles to the StyleSheet for consistency:
- <TouchableOpacity style={[styles.button, { borderRadius: 12 }]} onPress={handleSignIn}> + <TouchableOpacity style={[styles.button, styles.githubButton]} onPress={handleSignIn}> <FontAwesome name="github" size={24} color="#000" style={styles.icon} /> <Text style={styles.buttonText}>GitHub SignIn</Text> </TouchableOpacity> - <TouchableOpacity style={[styles.button, { borderRadius: 12, marginTop: 20 }]} onPress={handleSignIn}> + <TouchableOpacity style={[styles.button, styles.webButton]} onPress={handleWebSignIn}> <FontAwesome name="globe" size={24} color="#000" style={styles.icon} /> <Text style={styles.buttonText}>Web SignIn</Text> </TouchableOpacity> // And in the StyleSheet: + githubButton: { + borderRadius: 12, + }, + webButton: { + borderRadius: 12, + marginTop: 20, + },Also applies to: 96-99
🏁 Script executed:
#!/bin/bash # Check if there are different sign-in methods implemented echo "Checking for web sign-in implementation..." rg -A 5 "webSignIn" RDSExpoApp/Length of output: 126
Action Required: Confirm Distinct Sign-In Behavior & Style Consistency
The current implementation for the GitHub and Web sign-in buttons still uses the same
handleSignInfunction. Our search did not locate a separate handler (e.g.,handleWebSignIn) or any alternative implementation for the Web sign-in functionality, which may indicate that differentiation hasn't been implemented yet. If distinct behaviors for GitHub and Web sign-in are intended, please ensure that:
Behavior:
- The GitHub button continues to use
handleSignIn(or an appropriately named GitHub-specific function).- A separate handler, such as
handleWebSignIn, is implemented and assigned to the Web sign-in button.Styling:
- Move the inline style properties (e.g.,
borderRadius,marginTop) into the component’s StyleSheet (as shown in the provided diff snippet) to maintain consistency across the codebase.Please manually verify whether the Web sign-in functionality should have a unique handler and update the implementation accordingly.
🧰 Tools
🪛 GitHub Check: build (18.x)
[warning] 92-92:
Inline style: { borderRadius: 12 }
| const GithubSvg = (props) => ( | ||
| <Svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| xmlSpace="preserve" | ||
| viewBox="0 0 32 32" | ||
| {...props} | ||
| > | ||
| <Path | ||
| fill="#1A1718" | ||
| d="M16 3.006C8.835 3.006 3.007 8.835 3.007 16S8.836 28.994 16 28.994 28.994 23.165 28.994 16 23.165 3.006 16 3.006zm1.953 15.825c.319.276.566 1.251.566 1.699v3.964h-4.527s.006-1.344 0-2.265c-3.099.667-3.964-1.699-3.964-1.699-.566-1.132-1.132-1.699-1.132-1.699-1.132-.673 0-.566 0-.566 1.132 0 1.699 1.132 1.699 1.132.994 1.687 2.762 1.416 3.397 1.132 0-.566.248-1.422.566-1.699-2.473-.279-4.532-1.699-4.532-4.53 0-2.831.568-3.397 1.135-3.964-.114-.279-.588-1.311.018-2.831 0 0 1.113 0 2.245 1.699.561-.561 2.265-.566 2.832-.566.565 0 2.269.005 2.83.566 1.132-1.699 2.248-1.699 2.248-1.699.605 1.52.132 2.552.017 2.831.566.566 1.132 1.132 1.132 3.964s-2.056 4.253-4.53 4.531z" | ||
| /> | ||
| <Path | ||
| fill="#1A1718" | ||
| d="M16 0C7.163 0 0 7.163 0 16s7.164 16 16 16c8.837 0 16-7.163 16-16S24.837 0 16 0zm0 29.493C8.548 29.493 2.507 23.452 2.507 16S8.548 2.506 16 2.506c7.453 0 13.494 6.041 13.494 13.493 0 7.453-6.042 13.494-13.494 13.494z" | ||
| /> | ||
| </Svg> | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enhancing the SVG component with accessibility and customization options.
The GitHub logo SVG implementation is clean and follows React functional component patterns. However, consider the following improvements:
- Add accessibility attributes for screen readers
- Allow customization of the fill color through props instead of hardcoding
-const GithubSvg = (props) => (
+const GithubSvg = ({ fill = '#1A1718', ...props }) => (
<Svg
xmlns="http://www.w3.org/2000/svg"
xmlSpace="preserve"
viewBox="0 0 32 32"
+ aria-label="GitHub logo"
+ role="img"
{...props}
>
<Path
- fill="#1A1718"
+ fill={fill}
d="M16 3.006C8.835 3.006 3.007 8.835 3.007 16S8.836 28.994 16 28.994 28.994 23.165 28.994 16 23.165 3.006 16 3.006zm1.953 15.825c.319.276.566 1.251.566 1.699v3.964h-4.527s.006-1.344 0-2.265c-3.099.667-3.964-1.699-3.964-1.699-.566-1.132-1.132-1.699-1.132-1.699-1.132-.673 0-.566 0-.566 1.132 0 1.699 1.132 1.699 1.132.994 1.687 2.762 1.416 3.397 1.132 0-.566.248-1.422.566-1.699-2.473-.279-4.532-1.699-4.532-4.53 0-2.831.568-3.397 1.135-3.964-.114-.279-.588-1.311.018-2.831 0 0 1.113 0 2.245 1.699.561-.561 2.265-.566 2.832-.566.565 0 2.269.005 2.83.566 1.132-1.699 2.248-1.699 2.248-1.699.605 1.52.132 2.552.017 2.831.566.566 1.132 1.132 1.132 3.964s-2.056 4.253-4.53 4.531z"
/>
<Path
- fill="#1A1718"
+ fill={fill}
d="M16 0C7.163 0 0 7.163 0 16s7.164 16 16 16c8.837 0 16-7.163 16-16S24.837 0 16 0zm0 29.493C8.548 29.493 2.507 23.452 2.507 16S8.548 2.506 16 2.506c7.453 0 13.494 6.041 13.494 13.493 0 7.453-6.042 13.494-13.494 13.494z"
/>
</Svg>
);📝 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 GithubSvg = (props) => ( | |
| <Svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| xmlSpace="preserve" | |
| viewBox="0 0 32 32" | |
| {...props} | |
| > | |
| <Path | |
| fill="#1A1718" | |
| d="M16 3.006C8.835 3.006 3.007 8.835 3.007 16S8.836 28.994 16 28.994 28.994 23.165 28.994 16 23.165 3.006 16 3.006zm1.953 15.825c.319.276.566 1.251.566 1.699v3.964h-4.527s.006-1.344 0-2.265c-3.099.667-3.964-1.699-3.964-1.699-.566-1.132-1.132-1.699-1.132-1.699-1.132-.673 0-.566 0-.566 1.132 0 1.699 1.132 1.699 1.132.994 1.687 2.762 1.416 3.397 1.132 0-.566.248-1.422.566-1.699-2.473-.279-4.532-1.699-4.532-4.53 0-2.831.568-3.397 1.135-3.964-.114-.279-.588-1.311.018-2.831 0 0 1.113 0 2.245 1.699.561-.561 2.265-.566 2.832-.566.565 0 2.269.005 2.83.566 1.132-1.699 2.248-1.699 2.248-1.699.605 1.52.132 2.552.017 2.831.566.566 1.132 1.132 1.132 3.964s-2.056 4.253-4.53 4.531z" | |
| /> | |
| <Path | |
| fill="#1A1718" | |
| d="M16 0C7.163 0 0 7.163 0 16s7.164 16 16 16c8.837 0 16-7.163 16-16S24.837 0 16 0zm0 29.493C8.548 29.493 2.507 23.452 2.507 16S8.548 2.506 16 2.506c7.453 0 13.494 6.041 13.494 13.493 0 7.453-6.042 13.494-13.494 13.494z" | |
| /> | |
| </Svg> | |
| ); | |
| const GithubSvg = ({ fill = '#1A1718', ...props }) => ( | |
| <Svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| xmlSpace="preserve" | |
| viewBox="0 0 32 32" | |
| aria-label="GitHub logo" | |
| role="img" | |
| {...props} | |
| > | |
| <Path | |
| fill={fill} | |
| d="M16 3.006C8.835 3.006 3.007 8.835 3.007 16S8.836 28.994 16 28.994 28.994 23.165 28.994 16 23.165 3.006 16 3.006zm1.953 15.825c.319.276.566 1.251.566 1.699v3.964h-4.527s.006-1.344 0-2.265c-3.099.667-3.964-1.699-3.964-1.699-.566-1.132-1.132-1.699-1.132-1.699-1.132-.673 0-.566 0-.566 1.132 0 1.699 1.132 1.699 1.132.994 1.687 2.762 1.416 3.397 1.132 0-.566.248-1.422.566-1.699-2.473-.279-4.532-1.699-4.532-4.53 0-2.831.568-3.397 1.135-3.964-.114-.279-.588-1.311.018-2.831 0 0 1.113 0 2.245 1.699.561-.561 2.265-.566 2.832-.566.565 0 2.269.005 2.83.566 1.132-1.699 2.248-1.699 2.248-1.699.605 1.52.132 2.552.017 2.831.566.566 1.132 1.132 1.132 3.964s-2.056 4.253-4.53 4.531z" | |
| /> | |
| <Path | |
| fill={fill} | |
| d="M16 0C7.163 0 0 7.163 0 16s7.164 16 16 16c8.837 0 16-7.163 16-16S24.837 0 16 0zm0 29.493C8.548 29.493 2.507 23.452 2.507 16S8.548 2.506 16 2.506c7.453 0 13.494 6.041 13.494 13.493 0 7.453-6.042 13.494-13.494 13.494z" | |
| /> | |
| </Svg> | |
| ); |
| @@ -0,0 +1 @@ | |||
| <resources/> No newline at end of file | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Empty Night Mode Colors Resource: This file currently only contains an empty <resources/> element. If the intention is to provide a placeholder for dark theme colors, it might be useful to add a comment inside the resource tag or placeholders indicating future additions.
| { | ||
| "expo": { | ||
| "name": "RDSExpoApp", | ||
| "slug": "RDSExpoApp", | ||
| "version": "1.0.0", | ||
| "orientation": "portrait", | ||
| "icon": "./assets/images/icon.png", | ||
| "scheme": "rdsapp", | ||
| "userInterfaceStyle": "automatic", | ||
| "newArchEnabled": true, | ||
| "splash": { | ||
| "image": "./assets/images/splash-icon.png", | ||
| "resizeMode": "contain", | ||
| "backgroundColor": "#ffffff" | ||
| }, | ||
| "ios": { | ||
| "supportsTablet": true | ||
| }, | ||
| "android": { | ||
| "adaptiveIcon": { | ||
| "foregroundImage": "./assets/images/adaptive-icon.png", | ||
| "backgroundColor": "#ffffff" | ||
| }, | ||
| "package": "com.abstrctdatyp.RDSExpoApp" | ||
| }, | ||
| "web": { | ||
| "bundler": "metro", | ||
| "output": "static", | ||
| "favicon": "./assets/images/favicon.png" | ||
| }, | ||
| "plugins": [ | ||
| "expo-router" | ||
| ], | ||
| "experiments": { | ||
| "typedRoutes": true | ||
| }, | ||
| "extra": { | ||
| "GITHUB_CLIENT_ID": "Ov23lidvROsX75m5UoNm", | ||
| "GITHUB_AUTHORIZATION_CALLBACK_URL": "http://localhost:3000/auth/github/callback" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Comprehensive Expo App Configuration
The app.json file is well-constructed, encapsulating all crucial aspects of the Expo application configuration. It specifies the app’s name, schema, splash screen, platform-specific settings, and even experimental features like typed routes. One suggestion: consider externalizing sensitive or environment-specific values (such as GITHUB_CLIENT_ID and callback URLs) into environment variables to enhance security and flexibility across different deployment contexts.
| <intent-filter > | ||
| <action android:name="android.intent.action.VIEW" /> | ||
|
|
||
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <category android:name="android.intent.category.BROWSABLE" /> | ||
|
|
||
| <data | ||
| android:scheme="https" | ||
| android:host="www.realdevsquad.com/" /> | ||
| </intent-filter> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Host Value Format Observation.
The intent filter for the HTTPS scheme uses android:host="www.realdevsquad.com/" (line 67) with a trailing slash. Typically, host values should omit the trailing slash. Please confirm if this is intentional.
| eval "set -- $( | ||
| printf '%s\n' "$DEFAULT_JVM_OPTS $JAVA_OPTS $GRADLE_OPTS" | | ||
| xargs -n1 | | ||
| sed ' s~[^-[:alnum:]+,./:=@_]~\\&~g; ' | | ||
| tr '\n' ' ' | ||
| )" '"$@"' | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Advanced Argument Parsing via eval and sed.
The use of eval on processed arguments addresses shell metacharacter concerns. While functional, this approach can be fragile on edge cases—thorough testing is advisable.
| const handleLogout = async () => { | ||
| try { | ||
| await AsyncStorage.removeItem('github_token'); | ||
| console.log('User logged out'); | ||
| router.replace('/'); | ||
| } catch (error) { | ||
| console.error('Error logging out', error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve user feedback during logout.
Currently, there's no visual feedback to the user during the logout process, and errors are only logged to the console.
+import { StyleSheet, Text, TouchableOpacity, View, Alert, ActivityIndicator } from 'react-native';
export default function ProfileScreen() {
const router = useRouter();
+ const [isLoggingOut, setIsLoggingOut] = useState(false);
const handleLogout = async () => {
+ setIsLoggingOut(true);
try {
- await AsyncStorage.removeItem('github_token');
- console.log('User logged out');
- router.replace('/');
+ // Consider adding a confirmation dialog
+ const confirmLogout = await new Promise((resolve) => {
+ Alert.alert(
+ "Confirm Logout",
+ "Are you sure you want to log out?",
+ [
+ { text: "Cancel", onPress: () => resolve(false), style: "cancel" },
+ { text: "Logout", onPress: () => resolve(true) }
+ ]
+ );
+ });
+
+ if (confirmLogout) {
+ await AsyncStorage.removeItem('github_token');
+ console.log('User logged out');
+ router.replace('/');
+ }
} catch (error) {
console.error('Error logging out', error);
+ Alert.alert("Logout Failed", "There was a problem logging you out. Please try again.");
+ } finally {
+ setIsLoggingOut(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.
| const handleLogout = async () => { | |
| try { | |
| await AsyncStorage.removeItem('github_token'); | |
| console.log('User logged out'); | |
| router.replace('/'); | |
| } catch (error) { | |
| console.error('Error logging out', error); | |
| } | |
| }; | |
| import React, { useState } from 'react'; | |
| import { StyleSheet, Text, TouchableOpacity, View, Alert, ActivityIndicator } from 'react-native'; | |
| import AsyncStorage from '@react-native-async-storage/async-storage'; | |
| import { useRouter } from 'expo-router'; | |
| export default function ProfileScreen() { | |
| const router = useRouter(); | |
| const [isLoggingOut, setIsLoggingOut] = useState(false); | |
| const handleLogout = async () => { | |
| setIsLoggingOut(true); | |
| try { | |
| // Consider adding a confirmation dialog | |
| const confirmLogout = await new Promise((resolve) => { | |
| Alert.alert( | |
| "Confirm Logout", | |
| "Are you sure you want to log out?", | |
| [ | |
| { text: "Cancel", onPress: () => resolve(false), style: "cancel" }, | |
| { text: "Logout", onPress: () => resolve(true) } | |
| ] | |
| ); | |
| }); | |
| if (confirmLogout) { | |
| await AsyncStorage.removeItem('github_token'); | |
| console.log('User logged out'); | |
| router.replace('/'); | |
| } | |
| } catch (error) { | |
| console.error('Error logging out', error); | |
| Alert.alert("Logout Failed", "There was a problem logging you out. Please try again."); | |
| } finally { | |
| setIsLoggingOut(false); | |
| } | |
| }; | |
| return ( | |
| <View style={styles.container}> | |
| {isLoggingOut ? ( | |
| <ActivityIndicator size="large" color="#0000ff" /> | |
| ) : ( | |
| <TouchableOpacity onPress={handleLogout} style={styles.button}> | |
| <Text style={styles.buttonText}>Logout</Text> | |
| </TouchableOpacity> | |
| )} | |
| </View> | |
| ); | |
| } | |
| const styles = StyleSheet.create({ | |
| container: { | |
| flex: 1, | |
| justifyContent: 'center', | |
| alignItems: 'center', | |
| }, | |
| button: { | |
| backgroundColor: '#2089dc', | |
| padding: 12, | |
| borderRadius: 8, | |
| }, | |
| buttonText: { | |
| color: '#ffffff', | |
| fontWeight: 'bold', | |
| }, | |
| }); |
| const queryParams = { | ||
| sourceUtm: 'rds-mobile-app', | ||
| redirectURL: 'rdsapp://auth', | ||
| }; | ||
|
|
||
| const githubAuthUrl = buildUrl(AuthApi.GITHUB_AUTH_API, queryParams); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider more descriptive variable naming
While the implementation is correct, consider renaming sourceUtm to something more descriptive like source or utmSource. Also, it might be helpful to add a comment explaining the purpose of the redirect URL format rdsapp://auth.
| const handleTokenFromUrl = async (url: string) => { | ||
| try { | ||
| const urlObj = new URL(url); | ||
| const token = urlObj.searchParams.get('token'); | ||
| if (token) { | ||
| await AsyncStorage.setItem('github_token', token); | ||
| console.log('Token stored:', token); | ||
| router.replace('/HomeScreen'); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error processing deep link', error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Avoid logging sensitive data and improve error handling
The token is being logged to the console, which could be a security concern. Also, consider adding more specific error handling and user feedback.
const handleTokenFromUrl = async (url: string) => {
try {
const urlObj = new URL(url);
const token = urlObj.searchParams.get('token');
if (token) {
await AsyncStorage.setItem('github_token', token);
- console.log('Token stored:', token);
+ console.log('Token stored successfully');
router.replace('/HomeScreen');
+ } else {
+ console.error('No token found in URL');
+ // Consider showing an error message to the user
}
} catch (error) {
console.error('Error processing deep link', error);
+ // Consider showing an error message to the 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.
| const handleTokenFromUrl = async (url: string) => { | |
| try { | |
| const urlObj = new URL(url); | |
| const token = urlObj.searchParams.get('token'); | |
| if (token) { | |
| await AsyncStorage.setItem('github_token', token); | |
| console.log('Token stored:', token); | |
| router.replace('/HomeScreen'); | |
| } | |
| } catch (error) { | |
| console.error('Error processing deep link', error); | |
| } | |
| }; | |
| const handleTokenFromUrl = async (url: string) => { | |
| try { | |
| const urlObj = new URL(url); | |
| const token = urlObj.searchParams.get('token'); | |
| if (token) { | |
| await AsyncStorage.setItem('github_token', token); | |
| console.log('Token stored successfully'); | |
| router.replace('/HomeScreen'); | |
| } else { | |
| console.error('No token found in URL'); | |
| // Consider showing an error message to the user | |
| } | |
| } catch (error) { | |
| console.error('Error processing deep link', error); | |
| // Consider showing an error message to the user | |
| } | |
| }; |
| useEffect(() => { | ||
| checkUserSession(); | ||
|
|
||
| (async () => { | ||
| const initialUrl = await Linking.getInitialURL(); | ||
| console.log('Initial URL received:', initialUrl); | ||
| if (initialUrl) { | ||
| handleTokenFromUrl(initialUrl); | ||
| } | ||
| })(); | ||
|
|
||
| const subscription = Linking.addEventListener('url', (event) => { | ||
| console.log('Deep link event received:', event.url); | ||
| handleTokenFromUrl(event.url); | ||
| }); | ||
|
|
||
| return () => { | ||
| subscription.remove(); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Fix missing dependencies in useEffect
The current implementation has missing dependencies in the useEffect hook, which can lead to stale closures and unexpected behavior.
- useEffect(() => {
+ useEffect(() => {
checkUserSession();
(async () => {
const initialUrl = await Linking.getInitialURL();
console.log('Initial URL received:', initialUrl);
if (initialUrl) {
handleTokenFromUrl(initialUrl);
}
})();
const subscription = Linking.addEventListener('url', (event) => {
console.log('Deep link event received:', event.url);
handleTokenFromUrl(event.url);
});
return () => {
subscription.remove();
};
- }, []);
+ }, [checkUserSession, handleTokenFromUrl]);📝 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.
| useEffect(() => { | |
| checkUserSession(); | |
| (async () => { | |
| const initialUrl = await Linking.getInitialURL(); | |
| console.log('Initial URL received:', initialUrl); | |
| if (initialUrl) { | |
| handleTokenFromUrl(initialUrl); | |
| } | |
| })(); | |
| const subscription = Linking.addEventListener('url', (event) => { | |
| console.log('Deep link event received:', event.url); | |
| handleTokenFromUrl(event.url); | |
| }); | |
| return () => { | |
| subscription.remove(); | |
| }; | |
| }, []); | |
| useEffect(() => { | |
| checkUserSession(); | |
| (async () => { | |
| const initialUrl = await Linking.getInitialURL(); | |
| console.log('Initial URL received:', initialUrl); | |
| if (initialUrl) { | |
| handleTokenFromUrl(initialUrl); | |
| } | |
| })(); | |
| const subscription = Linking.addEventListener('url', (event) => { | |
| console.log('Deep link event received:', event.url); | |
| handleTokenFromUrl(event.url); | |
| }); | |
| return () => { | |
| subscription.remove(); | |
| }; | |
| }, [checkUserSession, handleTokenFromUrl]); |
🧰 Tools
🪛 GitHub Check: build (18.x)
[failure] 73-73:
React Hook useEffect has missing dependencies: 'checkUserSession' and 'handleTokenFromUrl'. Either include them or remove the dependency array
🪛 GitHub Actions: Continous-Integeration
[error] 73-73: React Hook useEffect has missing dependencies: 'checkUserSession' and 'handleTokenFromUrl'. Either include them or remove the dependency array.
| const checkUserSession = async () => { | ||
| const token = await AsyncStorage.getItem('github_token'); | ||
| if (token) { | ||
| console.log('User already logged in, redirecting to HomeScreen'); | ||
| router.replace('/HomeScreen'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding loading state during user session check
The function correctly checks for an existing token, but consider adding a loading state to improve user experience while the check is in progress.
const AuthScreen = () => {
const router = useRouter();
+ const [isLoading, setIsLoading] = useState(false);
// ... other code
const checkUserSession = async () => {
+ setIsLoading(true);
const token = await AsyncStorage.getItem('github_token');
if (token) {
console.log('User already logged in, redirecting to HomeScreen');
router.replace('/HomeScreen');
}
+ setIsLoading(false);
};
// ... and then use isLoading in your return JSX to show a loading indicator📝 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 checkUserSession = async () => { | |
| const token = await AsyncStorage.getItem('github_token'); | |
| if (token) { | |
| console.log('User already logged in, redirecting to HomeScreen'); | |
| router.replace('/HomeScreen'); | |
| } | |
| }; | |
| const AuthScreen = () => { | |
| const router = useRouter(); | |
| const [isLoading, setIsLoading] = useState(false); | |
| // ... other code | |
| const checkUserSession = async () => { | |
| setIsLoading(true); | |
| const token = await AsyncStorage.getItem('github_token'); | |
| if (token) { | |
| console.log('User already logged in, redirecting to HomeScreen'); | |
| router.replace('/HomeScreen'); | |
| } | |
| setIsLoading(false); | |
| }; | |
| // ... and then use isLoading in your return JSX to show a loading indicator | |
| return ( | |
| <View> | |
| {isLoading ? ( | |
| <ActivityIndicator size="large" color="#0000ff" /> | |
| ) : ( | |
| // ... other JSX content | |
| )} | |
| </View> | |
| ); | |
| }; |
1 participant
Date: 27-03-2025
Developer Name: Aditi Shinde
Issue Ticket Number
#480
Description
This PR contains the code of both CLI and Expo app in the same repo.
CLI is the old app and expo is the recently created app which is the replica of the CLI app but in React-Native Expo.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes