Feedback: NAVIGATION Please prioritize a new navigation library

Thank you to the expo team for everything you do.

I want to start a thread regarding navigation in hopes that it will be improved, as it is a pain everyone deals with, but a ‘solution’ exists, so consequently it seems to be deprioritized on the expo roadmap.

options:

  1. react-navigation is not good. really. It’s a jumbled mess, redundancy everywhere, confusing APIs, massive boilerplate mass, and plentiful bugs. One option is to put lots of work into this.
  2. start from scratch with a new library.
  3. add support for the Wix navigation library, which AFAIK is not compatible with expo b/c of native code.

happy to hear feedback.
thanks again

1 Like

Hey @nthgol,

We’re currently focusing our efforts on option #1. I know @notbrent would definitely love to hear you expand on your grievances and criticisms of the library – but I’ll warn you, he is a stickler for details. Vague and ambiguous statements will not do…the more excruciatingly detailed you can be, the better.

Cheers,

Adam

1 Like

hi there,

i started working on react-navigation in january and it has made a lot of progress since then. we’ll release 2.0 soon with a lot of api improvements. if you have any concrete proposals or criticisms please let me know. for example, you could take your “redundancy everywhere” comment and elaborate on that to provide examples. you could take the “massive boilerplate mass” comment and point out where there is some unnecessary code. as for “plentiful bugs”, well, there are bugs but there are bugs with anything :stuck_out_tongue: which ones are impacting you specifically?

feel free to participate in discussions on https://github.com/react-navigation/rfcs for the evolution of the library

2 Likes

thanks for the replies. Let me start with the few minutes I have but I may come back to this, and look at the RFC page.

  1. Boilerplate: the new API update in January creating listeners to ‘focus and blur’ events is an unnecessary layer to account for the fact that this library kills access to React lifecycle methods of screens.
  2. confusing API/redundancy: The navigation prop actions (e.g. Reset, GoBack, Navigate actions) are confusing. If I want to pass subsequent actions in these, do I import NavigationActions or do I just call it from my navigation prop, like this.props.navigate()? A PR was recently accepted added more layers by passing a key = null to perform some kind of secondary reset to the state ? And why is there a routename AND a key? Can we not just use a directory-style path like Root/TabNav1/Screen1 ? What is the purpose of the key.
  3. Confusing API: If I want to return a navigator inside a general React class component,I need to mutate the router property, this seems like a workaround.
  4. Redux integration is very unclear. I don’t know anyone that can get this wired up on their first try. For example, we’ve got dispatch which is the store’s dispatch, but its not located at this.props, its at this.props.navigation. My initialState from getStateFromAction doesn’t work, and theres a github issue with 50 thumbs up recommending to use a Reset action instead of the doc recommended Navigator.router.getActionForPathAndParams.
  5. Navigation debouncing should come out of the box, with not a line of additional code needed
  6. bugs: I won’t even go through the whole list, but some include not being able to truly goBack() across tabs, navigation being dispatched multiple times if RouteNames on nested navigators not named a certain way.

thank you for the thoughtful reply!

Boilerplate: the new API update in January creating listeners to ‘focus and blur’ events is an unnecessary layer to account for the fact that this library kills access to React lifecycle methods of screens.

we need to keep the screen components mounted, so there must be another layer on top of lifecycle events. we aren’t unmounting a component when we navigate away from it to another screen in a stack and so lifecycle events don’t make sense. if you don’t want to use those events directly, use the withNavigationFocus HOC. there was a huge amount of discussion on this already in https://github.com/react-navigation/react-navigation/issues/51 and we settled on listeners. there was also a rfc. i’d love it if you have some alternative solution if you could post a rfc with the details.

confusing API/redundancy: The navigation prop actions (e.g. Reset, GoBack, Navigate actions) are confusing. If I want to pass subsequent actions in these, do I import NavigationActions or do I just call it from my navigation prop, like this.props.navigate()? A PR was recently accepted added more layers by passing a key = null to perform some kind of secondary reset to the state ? And why is there a routename AND a key? Can we not just use a directory-style path like Root/TabNav1/Screen1 ? What is the purpose of the key.

there are a lot of questions here. i’ll start with “And why is there a routename AND a key” - this is because you can have multiple instances of a route (eg: in a stack you can push routes as many times as you like) and there needs to be a way to uniquely identify each one. you often don’t need to think about key and i’ve further reduced the need to do this in rfcs/text/0004-less-pushy-navigate.md at master · react-navigation/rfcs · GitHub for 2.0. but in some cases you need low level control like that, and so key gives you it.

Can we not just use a directory-style path like Root/TabNav1/Screen1

what problem does this solve? if you use a unique routename then the full path doesn’t matter. not clear to me what this would look like either.

A PR was recently accepted added more layers by passing a key = null to perform some kind of secondary reset to the state

i don’t understand what you mean here. which pr is this?

Confusing API: If I want to return a navigator inside a general React class component,I need to mutate the router property, this seems like a workaround.

generally you should not need to do this. that said, we should make this more clear to people. we’ve been working on improving docs and this seems like a valuable point to make super clear! if this is your only concern re confusing api i think we’re doing quite well :stuck_out_tongue:

Redux integration is very unclear.

you shouldn’t use it. it’s super weird that so many people want to do this. why break the encapsulation of navigation state by integrating it into all of your other state? 2.0 almost entirely eliminates any reason you might want to use redux. also see Redux support: what's it for? · Issue #14 · react-navigation/rfcs · GitHub and https://twitter.com/reactnavigation/status/968522317398601728. we should add a bigger note to docs indicating this.

Navigation debouncing should come out of the box, with not a line of additional code needed

this is largely handled in 2.0 via rfcs/text/0004-less-pushy-navigate.md at master · react-navigation/rfcs · GitHub. the only case where this will still be relevant is if you explicitly use the “push” action, and we’ll explore options for improving that as well, but it’s actually more rare than you think.

bugs: I won’t even go through the whole list, but some include not being able to truly goBack() across tabs,

i don’t know what you mean about not being able to “truly goBack()” across tabs. can you explain?

navigation being dispatched multiple times if RouteNames on nested navigators not named a certain way.

i don’t understand this one either, can you explain or link to an issue?

I just wanna add my two cents on this - I’ve been using react-navigation for a while now (about a year or so) and in the beginning I had a lot of issues with it and I seriously considered switching to Wix’s solution and even considered react-router v4 which has native support.

But I decided to give react-navigation another chance, yes - I implemented a lot of workarounds and hacks to solve bugs and issues but at some point it started to mature and many issues were just gone, yes - there are still many open issues and the documentation can get better but it’s a process…

I’m glad to say that it’s improving a lot and fast recently, so I suggest to give it a second chance :slight_smile:

3 Likes

Regarding mounting/unmounting, I disagree. It can lead to inaccessible mounted screens that eat up memory.

Let’s say I have a welcome flow, signup flow, or both. Presumably you want to check whether or not the user is signed into your app in the component lifecycle of one of these screens. Maybe you have a listener.

Once you send them through to the main app, they constantly have inaccessible mounted screens, which may also have component lifecycle methods such as ‘willreceivenextprops’. It is a side-effect and memory issue to keep them mounted

re: updating - you can implement a solution to that in userspace if you want. check if a screen is focused, and if it is not and it receives some new props, then do nothing. re-render when focus changes from false to true. if you think this should be the default behavior and you’re willing to think this through deeply and tease out the implications, then please post a rfc i would be very happy to give feedback there. https://github.com/react-navigation/rfcs

re: unmounting screens. you can also implement this in a similar way to what i suggested above. again if you’re willing to think deeply about this and write up a rfc it would be much appreciated.

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.