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

[Discussion] React Native Controllers Evolution #87

Open
drorbiran opened this issue Aug 11, 2016 · 7 comments
Open

[Discussion] React Native Controllers Evolution #87

drorbiran opened this issue Aug 11, 2016 · 7 comments

Comments

@drorbiran
Copy link
Contributor

drorbiran commented Aug 11, 2016

During the last few months we’ve been hard at work on a native navigation solution for Android. Working on both platforms in parallel introduced new challenges and helped us better understand how this entire project should be split up.

We’re currently thinking about merging react-native-controllers into react-native-navigation. This will make the codebase much simpler and unify the API between iOS and Android, while maintaining the native differences in UX between the two platforms.

This will naturally be a major version that will break some API’s that no longer make sense. In addition, there are many major features (like Screen lifecycle - onAppear, onDisappear events) that we held off until we figure out how Android fits into things. Introducing these features will require significant architecture changes, and doing these with Android around the corner did not make sense. All of these missing major features will now enter active development as the new architecture is finalized, along with all issues and pull requests.

We’d love to hear your feedback about evolving this library to the next level correctly. If you have ideas, questions or concerns please add them to the discussion in this issue.

@aksonov
Copy link

aksonov commented Aug 15, 2016

@drorbiran First of all, thanks for great component! I don't understand why it is discontinued and will became as part of something other component. I like JSX-based declarative syntax and don't like syntax of react-native-navigation.

Anyway I understand that you want to make native navigation cross-platform and hide implementation details this way. But why to invent bicycle and new syntax? As author of react-native-router-flux (RNRF) I propose you to unite our efforts. RNRF provides abstract well-defined declarative JSX-based syntax to define all your scenes with hierarchical manner (yes, mobile navigation is hierarchical with possible very deep structure, so i don't understand why react-native-navigation doesn't allow to insert drawer controller into navigation controller for example). Also RNRF has great big community and i believe many of developers will really want to use Native Navigation instead of JS one.

RNRF's v1 version is based on native Navigator, v2 is based on exNavigator component and v3 is based on new NavigationExperimental API (that based on JS navigation). I don't think that v4 version could be good idea for RNRF, so created another project for Native Navigation (RNRN): https://github.com/aksonov/react-native-router-native based on forked version of react-native-controllers (to allow nested controllers and much much more).

So we could join our efforts and make RNRF available for both iOS and Android.

@adamski
Copy link
Contributor

adamski commented Aug 15, 2016

Sounds like a good plan. I definitely support merging the two libraries.
Although I have been using react-native-navigation and don't mind the syntax, I have noticed a limitation in how things things are nested e.g. we can't have a tab controller within a modal or drawer. Its not a big deal for me at the moment, but I think more flexibility in the merged libraries would make sense.

@narychen
Copy link

I don't mind the syntax either. The performance is the only thing we care versus RN's navigator because your app's user don't care about your code style, they care about the user experience.

@winglight
Copy link

Facto, I turned here from react-native-navigation, you guys are great even better after merging projects, I guess.

@DanielZlotin DanielZlotin changed the title React Native Controllers Evolution [Discussion] React Native Controllers Evolution Aug 17, 2016
@DanielZlotin
Copy link
Contributor

We really appreciate your input and your contributions! All your inputs and concerns will be addressed and this is the exact reason we created this issue. We feel your pains.

Both of the projects (react-native-navigation and react-native-controllers) grew organically and, along with the react-native platform as a whole, was kinda "proof of concept"-ish. This includes not having tests (!), not having unified API, and in general messy at times, without well defined platform agnostic concepts. Additionally, we want to avoid maintaining multiple projects which under the hood just address the same skeletal structure of a single application.

One of the major architectural issues I want to address by making the merge is the syntax we use. In my opinion all commands flowing to the native core should be imperative.

The reasoning behind this is pretty simple: under the hood, all native code by definition is imperative. The declarative JSX syntax is and should be a wrapper around the imperative core, and should manage the deltas himself and send the appropriate commands to the core.

This is exactly the opposite of what happens here today. We have this awesome react-native-controllers library, which receives declarative JSX syntax, which is really a low-level API around iOS structured components, and sends it to be handled by the native ObjC code. On top of it we have react-native-navigation, which should hide platform specific code, receives commands imperatively and does weird voodoo to send those commands to controllers.
This is, in my opinion, completely upside down.

On top of all that, the current JSX syntax implementation (by hijacking the react import) is smelly and hard to maintain.

I think that wrapping the imperative api with something that can parse JSX, keep a "shadow skeleton", find the deltas, and convert it into react-native-navigation commands is indeed doable and encouraged (and not that hard), and we are planning to do just that in the future (using the help of this great community).

Unfortunately, this comes second to the fact that we must accomplish a few things as a top priority:

  • merge these 2 projects to a single code base (both in git and in npm)
  • clean the project up by separating the code into well defined concerns (making the code as SOLID as possible)
  • consolidate duplications and multiple intersecting pathways, while trying to stay as un-opinionated as possible
  • define general platform-agnostic concepts, which will make the code easier to reason about
  • adding tests - this is important because other than "feel good" trust of the code, helping with bug hunts and making sure the code is SOLID, this will help greatly in accepting pull requests from the community, as we will automatically run all tests on a dedicated build-server on each PR
  • Upgrading to the latest stable RN version - this will solve a lot of bugs and clean up some of the hacks
  • address some major bugs and PRs from the community and become more "community-friendly" in general

As for flexibility - I completely agree, up to a certain point. We must remember that we are after all living inside a native app and some things just do not work, and trying to force some things will cause you to "fight" with the system, and once you start fighting the system you'll never get out of it. The whole point of this project is to make our apps feel "native" to our users. Otherwise we would just hack away everything is JS.
So flexibility really should be a case-by-case examination and should be addressed by specific issues. Most of the time when things aren't supported by the platform, I guess will become some kind of a compromise. But we will always strive to be as un-opinionated as possible.

Most opinionated stuff we do right now are "quick-wins" (read - hacks) required for our production and will be generalised as part of the merge.

@DanielZlotin
Copy link
Contributor

One possible future I see for this project is to keep it in it's current state, accept some of the community as collaborators, and let you manage it freely. This then should be completely separate from our implementation that will be seated in react-native-navigation.

I think this is a good win-win scenario and will allow you to choose between the 2 projects, if you really MUST have the JSX syntax right now. Keep in mind we will still concentrate our efforts on the one true 100% native navigation solution ;)

@lc-rod
Copy link

lc-rod commented Aug 21, 2018

all of this looks good. However, where is the Android project, not everybody uses IPhones, there are a couple or more users who do use Android phones and tablets... and we have the same problems to solve....

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

No branches or pull requests

7 participants