Skip to content
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

Use latest Google Play Services #731

Merged
merged 2 commits into from
Nov 1, 2016

Conversation

mlanter
Copy link
Contributor

@mlanter mlanter commented Oct 26, 2016

Switch to use the latest Google Play Services.

The pain point is that by specifying a specific version it doesn't work with other libraries which specify the latest version of Google Play Services (which most do). It causes a crash because when it is compiled it will include some of the latest version and some of 9.4.0 and then when it runs it crashes.

It's generally safe to assume users have the latest Google Play Services as the updates are pushed automatically. As Google says "Google Play services gives you the freedom to use the newest APIs for popular Google services without worrying about device support. Updates to Google Play services are distributed automatically by the Google Play Store and new versions of the client library are delivered through the Android SDK Manager."

This matches a similar decision that was made to use react-native:+ in #547

@alexHlebnikov
Copy link

If newest version of GPS will be not installed on user's device, the app will crash when user will try to interact with map.

@mlanter
Copy link
Contributor Author

mlanter commented Oct 31, 2016

It's a tradeoff between crashing for users that don't have the latest GPS versus crashing for projects that use newer versions of GPS.

I'd lean towards requiring the latest GPS, because

  • Google says it is safe to assume the latest Google Play Services
  • The react-native-maps project has tended to error on the side of supporting only the latest versions of dependencies given the rapid change of pace (based on the Compatibility part of the readme)
  • We'll be able co-exist with other react-native libraries, most of which require the latest GPS.
  • Last time this was discussed in Bumped google-play-things in build.gradle to 9.+ #533, the decision was to support the latest GPS with the tradeoff of requiring users have the latest version (which Google says is a safe assumption).

@mlanter
Copy link
Contributor Author

mlanter commented Oct 31, 2016

compile 'com.google.android.gms:play-services-maps:9.4.0'
compile "com.facebook.react:react-native:+"
compile "com.google.android.gms:play-services-base:+"
compile "com.google.android.gms:play-services-maps:+"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use version 9.6.1 explicitly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what's the advantage of explicitly setting it versus using + (similar to what's done for react-native)?

Looking at the most popular react-native libraries that use Google Play Services the rest of them use +, which makes it difficult to use them with react-native-maps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue has been vastly discussed in previous issues, this is a really bad practice that RN introduced that tends to cause lots of subtle bugs that are hard to track down.
The overall best practice is to never use + since Gradle interprets it as "hey, I depend on this library, but any version is fine, I don't really care what gets used".
This leads to many problems since you almost always want to limit your range to be "at least version x.y.z or newer". This is what Gradle will do for any explicit version, eg.: 1.2.3 means that version or anything newer than that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining, that makes sense.

I still had the issue of dealing with multiple projects requiring different versions (which causes a crash on launch because it includes part of one version and part of another). An example being react-native-device-info.

In case it helps others, what I found worked for our project was forcing a consistent version for a group of libraries.

Here's the code we added to our build.gradle-

configurations.all {
    resolutionStrategy.eachDependency { DependencyResolveDetails details ->
        if (details.getRequested().getGroup() == 'com.google.android.gms') {
            // If different projects require different versions of 
            // Google Play Services it causes a crash on run.
            // Fix by overriding version for all projects.  
            details.useVersion('9.6.1')
        }
    }
}

Copy link
Contributor

@felipecsl felipecsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use version 9.6.1 explicitly

@felipecsl felipecsl merged commit be00dfc into react-native-maps:master Nov 1, 2016
timxyz pushed a commit to 3sidedcube/react-native-maps that referenced this pull request Nov 22, 2016
* commit '3d28a7d9fd005d59219668cf61177fe574170b84': (24 commits)
  examples-setup.md: update android instructions (react-native-maps#743)
  add example for overlay overpress and docs
  iOS google maps custom tile support (react-native-maps#770)
  [Docs] Fix capitalisation of Xcode and CocoaPods (react-native-maps#749)
  Implements animateToRegion to Google Maps iOS (react-native-maps#779)
  [RN][iOS][google] Set region only when view has width&height - Fix type issue in AIRMapManager - Setup Gemfile in example/ios dir to avoid problems with different versions of cocoapods - Update examples-setup.md to use bundler - Change MapView so that we only set the native region prop when there is a width and height. GoogleMaps iOS requires the width and height to properly calculate the map zoom level.
  updates
  [marker flicker]  Fix flicker of map pins on state change (react-native-maps#728)
  add onPress for polygons and polylines on iOS and Android
  Use latest Google Play Services (react-native-maps#731)
  Update installation.md (react-native-maps#742)
  Add latest patch releases to the changelog (react-native-maps#752)
  [ios][google] implement fitToSuppliedMarkers and fitToCoordinates (react-native-maps#750)
  If we've disabled scrolling within the map, then don't capture the touch events (react-native-maps#664)
  Fix dynamic imageSrc removal, fix flicker in react-native-maps#738  (react-native-maps#737)
  Fix Anchor point on Google Maps iOS
  Added ios google maps circle support
  Added google map type only check
  Fixed typo in google maps podspec
  Added ios google maps polygon, polyline, maptype support
  ...
Exilz pushed a commit to archriss/react-native-maps that referenced this pull request Dec 9, 2016
* Use latest Google Play Services

* Use 9.6.1 explicitly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants