Remove AirGoogleMaps from AirMaps.xcodeproj

[mikemorris]

Does any other open PR do the same thing?

No

What issue(s) is this PR fixing?

#2159, #2270

The non-Cocoapods manual install instructions appear to be gone, but IIRC it was previously required to manually link the Google Maps libraries in your Xcode project if you wished to use Google Maps on iOS - this patch restores that behaviour.

How did you test this PR?

npm link with the patch applied on master into a project where I was reproducing #2270, after using git bisect to identify the likely first bad commit.

[tuxity]

Nice! it working for me :)

[tonyhb]

+1 I think google maps shouldn't be auto-included if the build breaks by default.

[sutaka]

+1
I have the same issue

[theonetheycallneo]

I drafted this up for non-cocoapoders: https://gist.github.com/theonetheycallneo/bea215ebfbc7d7ade422458f60760df1

[alvaromb]

This should be the default behaviour of this library, thanks @mikemorris.

[jdonald]

+1. Putting

"react-native-maps": "https://github.com/mikemorris/react-native-maps.git#rm-airgooglemaps",

in my package.json avoids the problem.

[rborn]

@alvaromb @alvelig can you guys have a look at this as it seems to be related to #1954 (comment) 🐽

[alvelig]

Well, makes sense. So just to declare: we won't include GMaps by default. Developers are responsible for adding it manually themselves. Is it ok?

[alvaromb]

That's ok @alvelig, thanks.

[chrusart]

@jdonald I would use specific commit imho.
"react-native-maps": "github:mikemorris/react-native-maps#ef5c839252e427268ae2ebbf4f0cced1cb51fed6",

[rborn]

@alvelig @alvaromb so should we merge this?

[alvelig]

non-pods installation instructions are missing

[mikemorris]

@alvelig In the spirit of #1954 (comment) I wasn't attempting to add a documented or "endorsed" installation method, just removing an obstacle to upgrading for users who have chosen to install in an undocumented manner by restoring prior behavior.

I'd be happy to document a non-Cocoapods installation in a separate PR if that would be desirable.

[alvelig]

@mikemorris the problem is that it breaks current installation instruction process, doesn't it? Do we need to modify current installation instructions?

[mikemorris]

@mikemorris the problem is that it breaks current installation instruction process, doesn't it? Do we need to modify current installation instructions?

It shouldn't, but would be good to confirm. The existing Cocoapods installation instructions should still work just fine, pulling down the Google Maps SDK as a pod instead of expecting it to already exist in the project.

[alvelig]

@rborn could you check please, as you made the actual installation instructions, if they still work as they are. To have them up to date with the changes we make. TY!

[rborn]

@alvelig I'll do it after 5th of August, I'll be offline starting tomorrow :)

[alvelig]

Thanks! Happy holiday :D

[alvaromb]

@rborn we're testing this installation tomorrow. I'll ping you if it works ok.

[wbyoung]

My goal was to install and use Google Maps. I ended up on this issue & initially misunderstood what it does. For what it's worth, after installing, I started from the instructions here and am successfully running on iOS w/ Google Maps without CocoaPods & using react-native link. I know that this is not the point of the issue, but I strongly believe that using react-native link should be an acceptable installation option.

I'm not quite sure where to suggest it, but here's the changes that I made to the installation instructions:

< Make sure your GoogleMapsBase.framework, GoogleMapsCore.framdwork, and GoogleMaps.framework are all listed in the Compiled Sources.
< ** You might need to drag them into Embedded Frameworks depending your Xcode project setup too.
< - Search Xcode Build Settings for "Header Paths" and add your new framwork folder from above:
< ```
< $(SRCROOT)/Frameworks/Google-Maps-iOS-Utils
< ```
< 4. Install AirMaps React Native Wrapper
< - Open
< ```
< $(GITROOT)/node_modules/react-native-maps/lib/ios
< ```
< - Drag both folders into your Xcode project and ignore the Airmaps.xcproject
< ```
< $(GITROOT)/node_modules/react-native-maps/lib/ios/AirGoogleMaps
< $(GITROOT)/node_modules/react-native-maps/lib/ios/AirMaps
< ```
---
> Make sure your GoogleMapsBase.framework, GoogleMapsCore.framdwork, and GoogleMaps.framework are all listed in the _Link Binary With Libraries_ section.
>
> 4. Link required frameworks
>
> In the _Link Binary With Libraries_ section, click the plus button to link the following frameworks:
>
> - `GLKit.framework`
> - `CoreData.framework`
>
> Note that there are quite a few other frameworks that Google suggests linking that may be required for other uses of the framework and/or `react-native-maps`, but these should get you started. Reference the start guide from above for additional frameworks that should be linked.
42c36
< 5. Now link
---
> 5. Now link
44c38
< react-native link
---
> react-native link react-native-maps
46d39
< We run this command to link the Android side (which works fine as-is) and make sure once we can still successfully link npm updates.

An example of a library that has optional iOS dependencies that require removal in order to use them is react-native-camera, though I find the install instructions for that harder to follow. I agree that on iOS, Google Maps should be opt in.

Perhaps all of the AirMaps file could be wrapped in an #ifdef that's in an external file, defaulting to having nothing in it, then the react-native link instructions could update that include file via an npm script.

[wbyoung]

@mikemorris @alvelig @rbor I've opened #2396 which allows react-native link to be used to properly link and integrate the library without much work on the user's part and in a way that hopefully will not result in too much additional overhead in terms of support. It is my hope that #2396 will supersede this.

[alvaromb]

I can confirm this installation made by @mikemorris works perfect under iOS with react-native link /cc @rborn @alvelig.

Haven't tested @wbyoung PR, but it looks nice in order to ease the support of Google Maps for iOS.

[drewandre]

I was also able to use this library without cocoapods by using commit #ef5c839252e427268ae2ebbf4f0cced1cb51fed6. I now only run react-native link to get everything up and running. I did also have to run pod deintegrate in my /ios folder, though.

[jamesone]

Are there plans to merge this PR?

[jamesone]

@mikemorris Do you have an eta for when this PR will be merged?

[mikemorris]

@jamesone Nope, as I don't have commit access. It's working for myself, and a few others per the comments above though, so I'd very much like to see it merged and released soon!

[wbyoung]

@jamesone if you're trying out a PR to resolve this issue, please consider trying #2396 which is a solution that handles all use cases a little, but should still require no extra work if you're not planning on using Google Maps on iOS via react-native link (defaulting to not using Google Maps).

[jamesone]

I just tested this PR on my Simulator (#2310) and everything is working as expected 🥇

ENV:

• RN: 0.56
• Node: v8.6.0

[icemancast]

@jamesone did you install the google maps manually to the project and just replace the node module reference? wondering if I should clear cache too

[jamesone]

@icemancast I didn't setup Google maps. I used the default (Apple maps)

I would suggest removing node_modules, clearing the cache, reinstalling, and then restarting the dev server.

rm -rf node_modules
watchman watch-del-all
rm -rf ~/Library/Developer/Xcode/DerivedData/
rm -rf $TMPDIR/react-*
rm -rf ~/.node-gyp
yarn

[icemancast]

Thanks @jamesone , I finally got it working. I used the pr as my library until they merge it in and then installed google maps manually per google sdk docs. That and a couple back flips made it work!

[mikemorris]

Any update on getting this merged @rborn @alvelig?

[wbyoung]

@mikemorris have you had a chance to try #2396 which should solve the same problem?

There’s been some discussion over there and it may help to get your feedback to see if it also resolves your problems as well. I think/hope it likely will.

[rborn]

@wbyoung @mikemorris did't have the chance to test the last push to #2396 that fixes the markers (as far as I remember) so maybe if somebody could do it we'll push for it

[jamesone]

If anybody needs something tested, please let me know 👍Happy to help

[rborn]

@jamesone if you could test the example to work with #2396 (and maybe fix it if needed 🤪) would be amazing 🤗

[mikemorris]

@rborn @wbyoung I'm kinda philosophically opposed to the approach of leaving a vendored library committed in this repo. The changes in #2396 look good, but I'd kinda like to see that happen in addition to removing this dependency and recommending Cocoapods to manage the dependency if there's a desire to use Google Maps on iOS.

[rborn]

@mikemorris maybe you can work with @wbyoung toward this? 🤗

[wbyoung]

@mikemorris the changes in #2396 don’t add the vendored Google frameworks/libraries to the repository. They leave that as the user’s responsibility during install (only if not using CocoaPods). The choice of committing those files or managing them in another way is entirely deferred to the user.

What #2396 does is basically remove all of the AirGoogleMaps files but with preprocessor conditionals. So it’s essentially exactly the same as this PR in the default case. The subtle difference is that compiler will still see the files, but won’t produce any instructions as a result. And when desired, that conditional can be flipped on so the files do get compiled.

Personally, I have the desire to avoid using CocoaPods for the project that I’m integrating this into while needing to use Google Maps as well. I’d imagine others will at some point have the same preferences and/or needs. While I understand the superiority of CocoaPods for this specific case (especially since some dependencies aren’t pre-built & since many in the RN community don’t know Xcode well), I think it’s still important to have the option.

[mikemorris]

@wbyoung Ack, yea okay seeing that now - for some reason I was thinking that GoogleMaps/GoogleMaps.h itself got committed at some point.

Your approach looks good, I'll test it this weekend.

[mikemorris]

Closing in favor of #2396