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

Lean Core - Focusing on thin wrapper #2723

Closed
radeno opened this issue Mar 4, 2019 · 12 comments
Closed

Lean Core - Focusing on thin wrapper #2723

radeno opened this issue Mar 4, 2019 · 12 comments
Labels

Comments

@radeno
Copy link
Contributor

radeno commented Mar 4, 2019

Based on lean core initiative in React Native which deprecating legacy code and extracting some other modules to community.

I think React Native maps pushing ahead so much legacy code even from era when AirBnb was main maintainer of this repository.

55 opened PR and 245 issues says in clear language. Very much issues are almost identical "something doesn't work by documentation".
And that is problem. RN Maps is too wide. Using Google maps in iOS was an issue when iOS Maps were too far behind. But it is no longer valid. IOS Maps and Google Maps are both very stable, very useful and very similar in same functionalities.

So what should be implemented in Google Maps should be implemented in iOS Maps and vice versa.

On this fact i propose to remove whole implementation of Google Maps for iOS. It makes more problems that positives and focusing only on making very thin wrapper between RN and Native implementation. Every other implementation makes it hard to debug, and takes more time to code it correct.

I don't have any measured data about usage of Google Maps for IOS among developers.

So when i can request about anybody about feedback.

React with:

  • rocket 🚀 if you use only iOS Maps and don't use Google Maps on iOS
  • eyes 👀 if you use Google Maps on iOS

Thank you.

@radeno
Copy link
Contributor Author

radeno commented Mar 13, 2019

I must admit i am little bit shocked how many developers uses Google Maps in iOS.

We have used Google Maps in iOS about 3-4 years ago when iOS maps was newbie and quality was questionable. But nowadays i think that differences are very very very thin between them. We are building travel apps. And yes we are choosing Apple maps for iOS and Google Maps for Android.

Yes both have different maps design and richness of some data (but that is based on requirements).

Any reasons why you choose Google Map for iOS? But very concrete arguments.

@rborn
Copy link
Collaborator

rborn commented Mar 13, 2019

@radeno few things come to mind:

  • same feel and look for both ios and android (not mandatory but a nice touch)
  • some services google offers can only be used on a google map - like routing or places
  • google map is "better" than apple map (more info, etc, please be aware of the quotes around better, that subjective 😊)

@radeno
Copy link
Contributor Author

radeno commented Mar 13, 2019

@radeno few things come to mind:

  • same feel and look for both ios and android (not mandatory but a nice touch)
  • some services google offers can only be used on a google map - like routing or places
  • google map is "better" than apple map (more info, etc, please be aware of the quotes around better, that subjective 😊)

@rborn thank you for your opinion. I really appreciate what you are doing for this community package.
Maybe my quick contra-arguments :)

  • it looks same that is right, but is it a really advantage? UX for both platforms are different, have different UI and different base apps. It is variable but for some cases it is better to be as much as close to native UI what users excepts. And for maps it is 50:50. I have iPhone and using Apple maps and Google maps. I am familiar with both, but i am not good example because i am experienced user.
  • react-native-maps doesn't allow inline routing. You are right about places that are visible in maps, but without any interaction if i am right. Or does RN-maps supports places API?
  • yes it subjective, i like both maps. Apple maps are more clear and don't show too much information. What is good for navigation, not for searching. Search or looking for something arround is Google maps better.

What my whole point is that this package is stagnates. And it is very complicate to check PR, make code reviews and closing problematic issues. RN-maps covers too much.
When something new should be added it should be coded in 3 sides. It takes a lot of time for contributor, for code reviewer and maintainer to make change in future. In our company i like and push principle "as small as possible". Small commits, small changes, small code features. It is better handled and reviewed.

You committed my first PR #2571 . From my first "work in progress" commit to finally merged commit it takes almost 20 days. And i have spent some days before PR was opened. It was lot of time because learning takes some time, find solution takes and implement it right in 3 different SDKs takes time and review takes time. Not every reviewer is familiar with every SDK, not every developer is familiar with every SDK.

Incomplete features is another problem. #2619 i have commented that some functionalities are not coded. Maybe i make PR because we want it. Then we must ask question why they missing?
Was it because original author doesn't experience with Apple Maps or because developer don't need it ? First or second response is "problematic".

I see a lot similarities with RN before version Lean initiative. They want RN as thin as possible. And that is good. Separating scopes and concerns to community modules is good move. Developers are motivated. Issues are fixed and closed, new features increases, PR are merged and deprecated code is removed step by step. Reducing "layers" and removing complexity. Result? stable and performant product.

It is just my view how to move it forward.

@rborn
Copy link
Collaborator

rborn commented Mar 13, 2019

@radeno
I totally agree the package stagnates somehow (even if we have a lot of PRs in pipe) This is mostly due to the lack of time I think (mine, @alvelig or @christopherdro).

And you are right about the complexity. It's hard to maintain and update. However if we want to split this in pieces we need to think it very well. My main concern here is that the packages would diverge too much (most of the people would create a PR in one side and we'll lose all the "cross-platform" idea. Actually this already happens so what we do is not to merge PRs that are not cross (for new features)

I'd love for @alvelig @christopherdro @Titozzz and everybody else to jump in and maybe we can create a proposal how this can be re-structured 🤗

PS.
All your counter-arguments are valid (there is no right or wrong), most of the google feature I mentioned are pure legal, they forbid you to use the routing api and draw a line on somebody's else map, then is a matter of personal preferences, and so on 😊

@Titozzz
Copy link
Contributor

Titozzz commented Mar 21, 2019

hello @rborn, so the whole question is "How can we restructure the package so that we unlock contributions // keep everything clean"?

To be honest, I'm not sure removing Google Maps from iOS would be the best move, as many users might stay on an older version to keep compatibility from both.

Maybe we could change the strategy about not merging the features that are cross-platform.

If the main issue is time and not complexity, then what we need is to get new people interested in maintaining this project, maybe more communication, social media, things like this could also help.

@rborn rborn pinned this issue Mar 21, 2019
@rborn
Copy link
Collaborator

rborn commented Mar 21, 2019

Hey @Titozzz , I'm not sure I get this right: Maybe we could change the strategy about not merging the features that are cross-platform. NOT merging? when you say cross platform you mean google/apple or ios/android ?

Some other idea ( without dismissing anything from you and maybe was mentioned before and it just sank into my head) - split the module in two - one dedicated to google maps and made cross platform (ios/android/web?) and the other apple maps and made (obviously) ios only ?

@compojoom
Copy link
Contributor

Good discussion!

I'm against removing google maps for iOS - it's utopian to think that the apple maps and gmaps will have the same features. Even now there are subtle differences that just drive one crazy. That's why we've opted to use the same maps on both android and apple.

So maybe it's not a bad idea to split the project in 2 projects. One that focuses on google maps and one that focuses on apple maps only. Right now if you want to add something to google maps - you have to make sure that you add this for apple maps as well.
If we split this in 2 projects, the two could progress at different speed and maybe we'll get more people to contribute.

@radeno
Copy link
Contributor Author

radeno commented Mar 28, 2019

@rborn @compojoom i like idea of splitting. But my view of splitting is different.
I don't see any positives to embedding Google Maps into iOS in default.
It is depending on 3rd party SDK by default. This is not a way of simplicity i can imagine.

By my opinion basic/fundamental/default/,whaever other synonyms, RN-maps should depends only on native system libraries. For iOS it is Apple Maps, for Android it is Google Maps. This is cross-platform minimum which RN-Maps should offer.

And then there can be another packages which brings 3rd party providers. If user want to use Google Maps? Just install another package.

There are lot of 3rd party providers than Google Maps, i dont know why to prefer Google Maps. And again by bold 3rd party provider. Why not adding Mapbox, HERE maps, Sygic maps, Carto?
Adding another provider will bloat RN maps.

And that is point. Google Maps for iOS bloating package itself. Google Maps for iOS should be extracted as own package. On the other hand 3rd party packages should live by own and bring features what are not possible and should not be implemented in basic package.

Positives:

  • Thinner package, no need to bundle Google Maps.
  • Increase full cross-platform support for iOS and Android features. Many features today are not cross-platform but supported only for iOS system or GoogleMaps SDK
  • Decrease needs to implement feature/fix for 3 sides but only for 2.
  • Smaller learning curve to make and review pull request. Limiting only on default native implementations
  • Smaller code base is easier to maintain for future changes. Native platform changes, RN changes

RN-mas should be something like "artwork" for another maps packages. Do you want Google Maps in iOS? Okay no problem add for example "@react-native-maps/google" and you will get.

@rborn
Copy link
Collaborator

rborn commented Mar 28, 2019

@radeno I understand your point, and I think you are right, we should strive for the best solution.
What I'm afraid of is that we'll end up with a hybrid that won't be entirely cross-platform - and that because AppleMaps cannot be installed on android (or windows)

I believe that most users expect a module to work the same on both platforms, so if we try to make one that has only apple maps on ios and google maps on android, we won't get this parity (actually this is happening already) and we start to lose the whole idea of ReactNative being cross-platform. Most of the use cases are not just a simple map, they have feaures that are hard to keep in sync if the underlaying sdk is very different.

All the other providers have I think ios and android sdks that do the same thing, Apple is the only issue here.

So that's why I suggested to have Apple for iOS only, and Google maps for cross-platform.

We should try to bring more people in discussion though 😊 @alvelig @christopherdro any thoughts

@alvelig
Copy link
Contributor

alvelig commented Mar 29, 2019

I believe that most users expect a module to work the same on both platforms

This is the point.

@radeno What you say would only make sense if we made a list/roadmap of cross-platform things available in both platforms. This would be the "requirements document" for the library. And we can check how many things implemented currently are out of that scope. That would be a good base for decision making.

@radeno
Copy link
Contributor Author

radeno commented Mar 29, 2019

@rborn i really understand what you mean. I think cross-platform should not mean "look the same".
Every platform have different UI/UX. Swipe to back on iOS vs. system back button on Android. Different select boxes (and other form elements), different time pickers, different camera interface, diffrent share menu, and so on (all RN community modules :) ). I think cross-platform should also means be as most as possible to be natural for specific platform.
same behaviour != same look

I absolutely understand your concern that Apple maps are not available for other platforms than iOS and Web. Should be RN-maps as wrapper for individual map SDKs or as fundamental package for mapping support?

@alvelig
you are right. There should be document describing what is cross-platform and what is not. At current state almost all features (if not all) are multi-platform. What is necessary is to fix as many bugs and missing features. And then we can see. As you said to make decision.

Everything is about final use-case.
For users who's use maps as "source of informations" should be Google Maps better for both platforms. For users whos use maps as SDK platform to make own layers and informations it is questionable.
Examples:
AirBnB used Mapbox, than Google Maps and currently i see Apple maps in iOS app, they are experimenting a lot :)
Booking: Apple maps
Uber: Google maps
TipAdvisor: other sdk

@christopherdro christopherdro unpinned this issue Apr 26, 2019
@stale
Copy link

stale bot commented Oct 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 3, 2020
@stale stale bot closed this as completed Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants