-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Downgrade Java version to 1.8 and set kotlin JVM target 1.8 to fix compilation issues in newer android studios #368
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a concern 😄
android/build.gradle
Outdated
sourceCompatibility JavaVersion.VERSION_17 | ||
targetCompatibility JavaVersion.VERSION_17 | ||
} | ||
kotlinOptions { | ||
jvmTarget = "17" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we were already using 17 from the targetCompatibility
, but I think that might be too high? According to this table, Java 17 is only fully supported starting at Android 14.
I can never figure out where this might break, but my assumption is that if we accidentally use any newer API, it might compile to something incompatible on lower Android versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a valid concern... I believe it might have been like this when the plugin was created and we forgot to downgrade it...
I haven't heard of any related issues, but still might be worth to downgrade it to be safe. I will do it as part of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Downgraded and did some testing, seems to be working correctly. Lmk if you think this could have any unintended consequences @JayShortway!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I think this is safer! I don't think there should be unintended consequences. (Famous last words 😅) Thanks for being thorough with testing! 🙏
Compiling the android app in newer android studios was failing because it was trying to use jvm 21, which isn't compatible. Additionally as @JayShortway, we were using a higher version of Java that we should. We were mostly ok, but this brings the levelt to the same as our native android SDK and other hybrids.