tweak: RN-1770: target Android API Level 35 (Android 15)#6555
tweak: RN-1770: target Android API Level 35 (Android 15)#6555
Conversation
Summary of ChangesHello @jaskfla, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the application's target Android API level to 35. This change is crucial for maintaining compatibility with the latest Android operating system (Android 15), ensuring the application can utilize new platform features, and adhering to the most recent security and privacy standards mandated by Google Play. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to update the Android target API level to 35 (Android 15). While updating targetSdkVersion is a good step, the compileSdkVersion has not been updated in tandem. It's crucial to align compileSdkVersion with targetSdkVersion to ensure the app is built and tested against the new platform's APIs and behavioral changes. I've added a comment to address this oversight.
| minSdkVersion = 21 | ||
| compileSdkVersion = 34 | ||
| targetSdkVersion = 34 | ||
| targetSdkVersion = 35 |
There was a problem hiding this comment.
While updating targetSdkVersion to 35 is correct, the compileSdkVersion on line 7 should also be updated to 35. It is a best practice to keep compileSdkVersion and targetSdkVersion at the same value. This ensures your application is compiled against the APIs of the version it targets, allowing you to handle new platform behaviors and requirements correctly. Without this change, you might encounter unexpected runtime issues on Android 15 devices.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
jaskfla
force-pushed
the
rn-1770-android-api-35
branch
from
61dc322 to
39bf35c
Compare
| * Returns the name of the main component registered from JavaScript. This is used to schedule | ||
| * rendering of the component. | ||
| */ | ||
| override fun getMainComponentName(): String = "MediTrak" |
There was a problem hiding this comment.
Bug: Android component name doesn't match JavaScript registration
getMainComponentName() returns "MediTrak" but the JavaScript code in main.jsx registers the app component as 'TupaiaMediTrak'. iOS (AppDelegate.swift) correctly uses "TupaiaMediTrak". This mismatch will cause the Android app to crash on startup because React Native won't find a component with the name "MediTrak".
| <key>UIRequiredDeviceCapabilities</key> | ||
| <array> | ||
| <string>armv7</string> | ||
| <string>armv64</string> |
There was a problem hiding this comment.
Bug: Invalid iOS device capability architecture value typo
The UIRequiredDeviceCapabilities key specifies armv64 which is not a valid iOS device capability. The correct value is arm64. This typo could cause App Store submission rejection or unexpected device compatibility issues.
Applied all Android-related changes and some iOS-related changes
jaskfla
force-pushed
the
rn-1770-android-api-35
branch
from
dc483fd to
3287a26
Compare
| <key>NSAllowsArbitraryLoads</key> | ||
| <false/> | ||
| <key>NSAllowsLocalNetworking</key> | ||
| <true/> |
There was a problem hiding this comment.
ATS keys misplaced inside NSExceptionDomains dictionary
Medium Severity
NSAllowsArbitraryLoads and NSAllowsLocalNetworking are placed inside the NSExceptionDomains dictionary, but Apple's documentation requires these keys to be direct children of the NSAppTransportSecurity dictionary. Nested inside NSExceptionDomains, they will be ignored by the system, so the intended NSAllowsLocalNetworking exception for debug builds won't take effect.
| // If you opted-in for the New Architecture, we load the native entry point for this app. | ||
| load() | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing Bugsnag initialization in Android MainApplication
High Severity
The Java-to-Kotlin conversion of MainApplication dropped the Bugsnag.start(this) call that was in the old MainApplication.java's onCreate(). The Android manifest still declares the Bugsnag API key, but without the explicit start() call, crash reporting will be silently disabled on Android.
| @@ -0,0 +1,44 @@ | |||
| package com.tupaia.meditrak | |||
There was a problem hiding this comment.
Kotlin package name mismatches Android namespace causing crash
High Severity
Both new Kotlin files declare package com.tupaia.meditrak, but the build.gradle namespace is "com.tupaiameditrak". The AndroidManifest uses shorthand .MainApplication and .MainActivity, which Android resolves relative to the namespace — so it will look for com.tupaiameditrak.MainApplication, but the actual class is com.tupaia.meditrak.MainApplication. This causes a ClassNotFoundException at app startup. The old Java files correctly used package com.tupaiameditrak.
Additional Locations (1)
| self.initialProps = [:] | ||
|
|
||
| return super.application(application, didFinishLaunchingWithOptions: launchOptions) | ||
| } |
There was a problem hiding this comment.
Missing Bugsnag and AppCenter initialization on iOS
High Severity
The Objective-C to Swift conversion of AppDelegate dropped the [Bugsnag start], [AppCenterReactNative register], and [AppCenterReactNativeAnalytics registerWithInitiallyEnabled:true] calls that existed in the old AppDelegate.mm. Crash reporting and analytics will be silently disabled on iOS.
# Conflicts: # packages/meditrak-app/metro.config.js # yarn.lock
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| * Returns the name of the main component registered from JavaScript. This is used to schedule | ||
| * rendering of the component. | ||
| */ | ||
| override fun getMainComponentName(): String = "MediTrak" |
There was a problem hiding this comment.
Android component name doesn't match JS registration
High Severity
getMainComponentName() returns "MediTrak", but the JavaScript side registers the component as "TupaiaMediTrak" via AppRegistry.registerComponent('TupaiaMediTrak', () => App) in main.jsx. The iOS AppDelegate.swift correctly uses "TupaiaMediTrak". This mismatch will cause the Android app to fail to find and render the root React component.
| <key>UIRequiredDeviceCapabilities</key> | ||
| <array> | ||
| <string>armv7</string> | ||
| <string>armv64</string> |
There was a problem hiding this comment.
Invalid device capability armv64 instead of arm64
High Severity
UIRequiredDeviceCapabilities specifies armv64, which is not a valid value. Apple's documentation lists the valid architecture capabilities as armv6, armv7, arm64, and arm64e. This typo could prevent the app from being installed on devices or cause App Store rejection.
1 participant
RN-1770