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

[ios] move local framework dependency from Mapbox.framework > RNMapbo… #535

Conversation

bartolkaruza
Copy link
Contributor

…x.framework

Fixes #232

The issue from #232 is still relevant for any apps that try to include both the Mapbox Navigation SDK and react-native-mapbox-gl/maps. The suggested rename was pending a change nitaliano/react-native-mapbox-gl#1533 that seems to never have landed on this repo.

I'm still testing this change on our end, but I wanted to open the PR to see if this approach will work for everyone.

@mfazekas
Copy link
Contributor

@bartolkaruza one issue with this approach is that after update, every ios project have to be adjusted to the new framework naming.

It also means that the apps using Navigation and RNMBGL will have two separate mapbox framework linked in.

CocoaPods/CocoaPods#6446 (comment) seems to present a CocoaPods solution for this. Isn't that something that users can implement?!

@kristfal
Copy link
Member

Agree with @mfazekas here. I'd rather we create a section in install docs for iOS that explains how to make Mapbox Navigation and rnmgl work together.

@iutin
Copy link

iutin commented Nov 30, 2019

Agree with @mfazekas here. I'd rather we create a section in install docs for iOS that explains how to make Mapbox Navigation and rnmgl work together.

It will be perfect! I have the same issue when try to use Mapbox Navigation and rnmgl together. Thank you!

@bartolkaruza
Copy link
Contributor Author

bartolkaruza commented Dec 3, 2019

@bartolkaruza one issue with this approach is that after update, every ios project have to be adjusted to the new framework naming.

Wouldn't that be resolved by a new Pod install?

It also means that the apps using Navigation and RNMBGL will have two separate mapbox framework linked in.

This isn't ideal, I agree.

CocoaPods/CocoaPods#6446 (comment) seems to present a CocoaPods solution for this. Isn't that something that users can implement?!

Hm, that relates to two targets that have the same dependency, not two dependencies with the same transient dependency. I'll try to make something work when I find the time and post back here if I find a clean workaround using Cocoapods.

@mfazekas
Copy link
Contributor

mfazekas commented Dec 4, 2019

@bartolkaruza one issue with this approach is that after update, every ios project have to be adjusted to the new framework naming.

Wouldn't that be resolved by a new Pod install?

Yep sorry you're probably right, this only affect those who did a manual install, autolink and cocoapods might just work with pod install

It also means that the apps using Navigation and RNMBGL will have two separate mapbox framework linked in.

This isn't ideal, I agree.

CocoaPods/CocoaPods#6446 (comment) seems to present a CocoaPods solution for this. Isn't that something that users can implement?!

Hm, that relates to two targets that have the same dependency, not two dependencies with the same transient dependency. I'll try to make something work when I find the time and post back here if I find a clean workaround using Cocoapods.

These just ideas and I havent thought them trough:
We could have an option with a podspec that uses s.dependency "Mapbox-iOS-SDK", "~> 5.2" instead of https://github.com/react-native-mapbox-gl/maps/blob/cc190a02f0a13cfe4a8068d76ad9734bdae00d64/react-native-mapbox-gl.podspec#L16

This could be either done by a condition on ENV. Say

if ENV["RNMBGL_USE_MB_AS_PODS"]
  s.dependency "Mapbox-iOS-SDK", "~> 5.2"
else
  s.vendored_frameworks = 'ios/Mapbox.framework'
end

or just have a different podsspec with dependency like react-native-mapbox-gl-pod.podspec.

Or we can just consider migrating to dependency for everyone, see nitaliano/react-native-mapbox-gl#1533

@bartolkaruza
Copy link
Contributor Author

I'm a fan of the approach in nitaliano/react-native-mapbox-gl#1533

Cocoapods is the default dependency management for React Native apps for a few releases now, so that would solve it for the 90% of people that don't tinker with build systems. The remaining 10% will be able to resolve issues themselves (fairly ;)) easily.

The interoperability between the Navigation SDK and RNMBGL would be trivial to set up and correctly using the shared dependency.

I'm unsure about the pain of version conflicts between the libraries in the future, what is your view on that?

@mfazekas
Copy link
Contributor

mfazekas commented Dec 4, 2019

I'm unsure about the pain of version conflicts between the libraries in the future, what is your view on that?

Well nobody sees into the future. I think it was a special situation with the pre 6.* versions and i think it will not repeat, but we'll see. I don't think we should worry about that, for now.

@kristfal
Copy link
Member

Closing due to lack of activity.

@kristfal kristfal closed this Jan 10, 2020
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.

Can't use react-native-mapbox-gl and MapboxNavigation with CocoaPods
4 participants