rnmapbox / maps

A Mapbox react native module for creating custom maps
MIT License
2.26k stars 846 forks source link

Add support for AndroidX #163

Closed sublimedatasystem closed 5 years ago

sublimedatasystem commented 5 years ago

Describe the bug

Task :@react-native-mapbox-gl_maps:compileDebugJavaWithJavac FAILED /home/sublime/Desktop/mahendra/react projects/passengerapp/node_modules/@react-native-mapbox-gl/maps/android/rctmgl/src/main/java/com/mapbox/rctmgl/events/LocationEvent.java:4: error: package android.support.annotation does not exist import android.support.annotation.NonNull;

Execution failed for task ':@react-native-mapbox-gl_maps:compileDebugJavaWithJavac'.

Compilation failed; see the compiler error output for details.

I'm getting this error on react-native run-android command run. I have migrated my project android to androidx.

Screenshots image

image

Versions (please complete the following information):

kristfal commented 5 years ago

@mfazekas we'll need to handle AndroidX as it is coming with RN60. We're still in rc stage, so what do you think about adding this as a breaking change and then move forward with an official release at 7.0.0 with AndroidX support?

Alternatively we could release 7.0.0 and then increment to 8.0.0 for AndroidX.

Here is google's recommendation about migration. This is for flutter, but it still applies to us.

mfazekas commented 5 years ago

@kristfal I'm not sure the exact changes required for AndroidX. Not sure if it means, but likely it'll mean that mapbox will be RN60+ only.

I'd vote to try to release 7.0 ASAP, and then do the AndroidX in 8.0.

sublimedatasystem commented 5 years ago

@mfazekas @kristfal I want to use mapbox in 0.59 can you guys assist me what can do to use mapbox in 0.59 version

wirrareka commented 5 years ago

we're working with androidx rctmgl edit, it's rather quick process, basically only changing few imports in rctmgl like

import android.support.annotation.NonNull =>  import androidx.annotation.NonNull
import android.support.annotation.Nullable =>    import androidx.annotation.Nullable
import android.support.v4.content.res.ResourcesCompat => import androidx.core.content.res.ResourcesCompat

https://developer.android.com/jetpack/androidx/migrate provides a good mapping list. other then that, things seem to work fine sofar, can make a PR if it helps

mfazekas commented 5 years ago

i'd vote - if possible - support androidx via jetifier npm at first. See https://github.com/mikehardy/jetifier#to-jetify--convert-node_modules-dependencies-to-androidx

As i understand this is pretty much a one way street. Once we upgrade to androidx users will be forced to upgrade to androidx project, and they might have androidx problematic dependencies.

kristfal commented 5 years ago

Yes, that would be a good approach. Suggest we do the following:

1) Release 7.0.0 make sure we support jetifier and document this properly in the readme 2) (Potentially) migrate the example app to AndroidX and use it as a poc/example/test for the jetifier 3) In a few weeks/months increment to 8.0.0 and switch to AndroidX in the main lib

Thoughts?

mfazekas commented 5 years ago

@kristfal i agree

d3vhound commented 5 years ago

As of today, using react-native init now uses RN v.0.60 and android no longer builds with this library. I think it should be in the readme that v0.60 is not supported yet.

ako977 commented 5 years ago

Yes, that would be a good approach. Suggest we do the following:

  1. Release 7.0.0 make sure we support jetifier and document this properly in the readme
  2. (Potentially) migrate the example app to AndroidX and use it as a poc/example/test for the jetifier
  3. In a few weeks/months increment to 8.0.0 and switch to AndroidX in the main lib

Thoughts?

Just checking if this suggested solution is in v 7.0.0-rc3 ?

Thanks

mfazekas commented 5 years ago

@ako977 yes please try https://www.npmjs.com/package/jetifier with react native mapbox and report any issue so we can fix them.

anfriis commented 5 years ago

@mfazekas @kristfal

Since I was unable to make this framework work when upgrading my RN project to 0.60.0, I tried creating a test project with RN 0.60.0 and v7.0.0-rc3.

Here is what I did:

react-native init androidxtest --template typescript
yarn add @react-native-mapbox-gl/maps

Got this warning when starting Metro Bundler: Loading dependency graph...warn The following packages use deprecated "rnpm" config that will stop working from next release:

@react-native-mapbox-gl/maps: https://npmjs.com/package/@react-native-mapbox-gl/maps
Please notify their maintainers about it. You can find more details at https://github.com/react-native-community/cli/blob/master/docs/configuration.md#migration-guide.

Extract from file:

     Required by:
         project :@react-native-mapbox-gl_maps
      > Cannot find a version of 'androidx.vectordrawable:vectordrawable' that satisfies the version constraints: 
           Dependency path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'com.android.support:support-vector-drawable:28.0.0' because of the following reason: ENABLE_JETIFIER is enabled
           Constraint path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'androidx.vectordrawable:vectordrawable:{strictly 1.0.0}' because of the following reason: debugRuntimeClasspath uses version 1.0.0
           Dependency path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'androidx.appcompat:appcompat:1.0.2' --> 'androidx.vectordrawable:vectordrawable:1.0.1'
           Dependency path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'androidx.appcompat:appcompat:1.0.2' --> 'androidx.vectordrawable:vectordrawable-animated:1.0.0' --> 'androidx.vectordrawable:vectordrawable:1.0.0'

   > Could not resolve com.android.support:appcompat-v7:28.0.0.
     Required by:
         project :@react-native-mapbox-gl_maps
      > Cannot find a version of 'androidx.appcompat:appcompat' that satisfies the version constraints: 
           Dependency path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'com.android.support:appcompat-v7:28.0.0' because of the following reason: ENABLE_JETIFIER is enabled
           Constraint path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'androidx.appcompat:appcompat:{strictly 1.0.0}' because of the following reason: debugRuntimeClasspath uses version 1.0.0
           Dependency path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'com.facebook.react:react-native:0.60.0' --> 'androidx.appcompat:appcompat:1.0.2'
           Dependency path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'com.mapbox.mapboxsdk:mapbox-android-plugin-localization-v7:0.9.0' --> 'androidx.appcompat:appcompat:1.0.0'
           Dependency path 'androidxtest:@react-native-mapbox-gl_maps:unspecified' --> 'com.mapbox.mapboxsdk:mapbox-android-plugin-markerview-v7:0.2.0' --> 'androidx.appcompat:appcompat:1.0.0'
yarn add -D jetifier
npx jetify

And ran Android again, but the error is the same. So it seems that there is no support for RN 0.60.0 at the moment even with jetifier

Darkilen commented 5 years ago

@anfriis Did you run jetifier like they said here (Step 3, npx jetify) ?

anfriis commented 5 years ago

@Darkilen Yes, apparently I forgot to add it here. Just updated my post

jshearer commented 5 years ago

Is there any solution to this? Even as a workaround?

jshearer commented 5 years ago

Okay I made a PR that should make things work, but like I say over there, I'm not very experienced with Java/Gradle so I'm not sure if it's the correct solution. For me, the following changes to my package.json file make things work as expected:

"scripts": {
    "postinstall": "jetify"
},
"dependencies": {
    "@react-native-mapbox-gl/maps": "jshearer/maps",
    "jetifier": "^1.6.2"
}

Then just run yarn.

Friis1978 commented 5 years ago

@jshearer any news on this issue?

Since React native must be updated to version 0.60 before August, I hope this will be solved.

ako977 commented 5 years ago

@ako977 yes please try https://www.npmjs.com/package/jetifier with react native mapbox and report any issue so we can fix them.

@mfazekas Just following up that postinstall jetify worked for 7.0.0-rc3

DavidKHahn commented 5 years ago

@sublimedatasystem @ako977 @mfazekas Hello guys, I have tried the methods using jetifier and adjusted the package.json but still receiving the 'Execution failed for task ':@react-native-mapbox-gl_maps:compileDebugJavaWithJavac'.' issue. Can you list out the steps used to fix this issue or has it not been resolved? Thanks in advance.

ice-chillios commented 5 years ago

@DavidKHahn Try using @jshearer branch. It works for me, we need to wait for merge and release for now ;)

DavidKHahn commented 5 years ago

@dsznajder thanks for the quick response. I'll give it a try! :)

DavidKHahn commented 5 years ago

Okay I made a PR that should make things work, but like I say over there, I'm not very experienced with Java/Gradle so I'm not sure if it's the correct solution. For me, the following changes to my package.json file make things work as expected:

"scripts": {
    "postinstall": "jetify"
},
"dependencies": {
    "@react-native-mapbox-gl/maps": "jshearer/maps",
    "jetifier": "^1.6.2"
}

Then just run yarn.

@dsznajder Unfortunately, this solution does not work for me. I am currently running into a mapbox issue with 'ReactTextView' when the map is being loaded onto the screen.

20190715_124021

sijad commented 5 years ago

simply changing compileOnly "com.facebook.react:react-native:+" to implementation "com.facebook.react:react-native:+" in build.gradle will enable to use jetifier.

there's a PR #251

after applying the patch follow this instruction

kelcon commented 5 years ago

It helped. Thank you 👍

danielkeithw commented 5 years ago

That fixed it, thanks.

matoscano commented 5 years ago

When will the new version with support for android x be available? Thanks

DavidKHahn commented 5 years ago

+1

On Wed, Jul 31, 2019, 1:42 AM Miguel Angel Toscano Rastrollo < notifications@github.com> wrote:

When will the new version with support for android x be available? Thanks

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/react-native-mapbox-gl/maps/issues/163?email_source=notifications&email_token=AIKYOLAMK35ONAYXYFC4KTDQCFGATA5CNFSM4H26UW72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3GRGQA#issuecomment-516756288, or mute the thread https://github.com/notifications/unsubscribe-auth/AIKYOLGQ3KMLKEV2M6776ILQCFGATANCNFSM4H26UW7Q .

muhaimincs commented 5 years ago

+1

ospfranco commented 5 years ago

any news on this?

kristfal commented 5 years ago

With React Native 60+ running jetifier by default for react-native run-android, I don't see the point of migrating this library over to AndroidX as it would force all devs to upgrade to RN60 for no apparent benefit. Please correct me if this is wrong.

Proposed path ahead:

1) Upgrade the example app to RN62 to showcase that it works just fine with this package out of the box 2) Whenever react native stops running jetifier by default, we migrate this package

If anyone has opinions against the path ahead, please flag it here. In the interim I'll close the issue.