search

segmentio / analytics-react-native

The hassle-free way to add analytics to your React-Native app.
https://segment.com/docs/sources/mobile/react-native/
MIT License
354 stars 182 forks source link

fix: resolve potential build issues with RN 0.72 #841

Closed AlexanderEggers closed 1 year ago

AlexanderEggers commented 1 year ago

Starting with RN 0.72 the JVM target will be enforced across all packages. Unfortunately in my case instead of enforcing JVM 11, I am going to have to enforce JVM 17 because that's the java version I have locally installed.

The RN team introduced a fix which updates the sourceCompatibility and targetCompatibility in all projects to make that compatible. kotlinOptions.jvmTarget is by default set by the JVM version that is installed on the machine - in my case 17. At the moment, the RN fix is not updating the kotlinOptions.jvmTarget - I hope that will still make it as part of the 0.72.0 stable relaese. Generally the JVM target of java and kotlin needs to match. If the RN team does not decide to update their fix, then I would need to patch this library manually to fix build errors on RN 0.72.

I believe dropping the suggested lines will have no impact on this project but will ensure other people with my same use-case are not running into any issues.

oscb commented 1 year ago

Hi @AlexanderEggers

Thanks for raising this up. I think it all makes sense. I truly don't think this should have any impact but we might have to manually test this change with 0.72 just to confirm before merging.

AlexanderEggers commented 1 year ago

@oscb I don't think you will have to wait for 0.72 to land. IMO there should be no reason for you to define the kotlin jvm target and segment is actually only library I know of that defines this.

oscb commented 1 year ago

I meant just testing with the current version of RN.

Just a sanity check.

oscb commented 1 year ago

I removed it from the AdvertisingId plugin and updated your PR, I haven't been able to track down why this was added, it's been a while, but seems to be working properly so I'll merge when the build finishes.

@AlexanderEggers Thanks again!

AlexanderEggers commented 1 year ago

Thanks for the update @oscb 👍

oscb commented 10 months ago

:tada: This PR is included in version @segment/analytics-react-native-v2.16.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

oscb commented 10 months ago

:tada: This PR is included in version @segment/analytics-react-native-plugin-advertising-id-v1.2.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket: